Re: [PATCH v4 04/79] drm_mode.h: use __u32 and __u64 from linux/types.h

[Emil Velikov]

On 21 October 2015 at 17:27, Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
> On Wed, Oct 21, 2015 at 12:21 PM, Emil Velikov <emil.l.velikov@xxxxxxxxx> wrote:
>> On 21 October 2015 at 16:18, Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
>>> On Wed, Oct 21, 2015 at 11:09 AM, Emil Velikov <emil.l.velikov@xxxxxxxxx> wrote:
>>>> Hi Alex,
>>>>
>>>> On 15 October 2015 at 14:48, Mikko Rapeli <mikko.rapeli@xxxxxx> wrote:
>>>>> On Thu, Oct 15, 2015 at 09:32:10AM -0400, Alex Deucher wrote:
>>>>>> On Thu, Oct 15, 2015 at 1:55 AM, Mikko Rapeli <mikko.rapeli@xxxxxx> wrote:
>>>>>> > Fixes userspace compilation error:
>>>>>> >
>>>>>> > drm/drm_mode.h:472:2: error: unknown type name âuint32_tâ
>>>>>> >
>>>>>> > Signed-off-by: Mikko Rapeli <mikko.rapeli@xxxxxx>
>>>>>>
>>>>>> NACK on all these type conversions.  This has not been a problem for
>>>>>> years and years and the result looks terrible.
>>>>>
>>>>> Documentation/CodingStyle, section 5
>>>>>
>>>>>  (e) Types safe for use in userspace.
>>>>>
>>>>>      In certain structures which are visible to userspace, we cannot
>>>>>      require C99 types and cannot use the 'u32' form above. Thus, we
>>>>>      use __u32 and similar types in all structures which are shared
>>>>>      with userspace.
>>>>>
>>>>> I have only been looking at kernel headers from userspace occationally in
>>>>> the past 10 years and had a several cases where the provided headers did
>>>>> not compile when included into trivial programs trying to use the structs
>>>>> for an ioctl() for example. This long lasting problem triggered me to write
>>>>> a test for this and provide these fixes too. In previous reviews usage
>>>>> of <stdint.h> and its types in kernel headers was already NACK'ed
>>>>> so I changed several places from uint32_t's to __u32.
>>>>>
>>>>> With these changes it is btw trivial now to add a grep test the there
>>>>> are no uint32_t's in include/uapi/ anymore, thus enforcing that coding style
>>>>> rule.
>>>>>
>>>> Based of the reply from Mikko, can you please elaborate your concern ?
>>>> Are you thinking about some corner case where this may cause breakage,
>>>> or it's solely on stylistic point of view ?
>>>
>>> Style.
>>>
>> In that case shouldn't the kernel coding style (not to mention the
>> issues with stdint.h types Linus has brought a while back) be
>> considered/prevail ?
>
> I don't really have a strong opinion one way or another at this point.
> The types just look ugly.
>
Fwiw I don't find them too appealing either. But the alternative did
not fare so well :(

>>
>>>>
>>>> Over the last few years we've been doing some ad-hoc 'synchronisation'
>>>> with the headers in libdrm, and this will get us one step closer to
>>>> doing things properly.
>>>
>>> How does this affect libdrm one way or another?
>>>
>> In a strange way. Some (most?) distributions do not ship the uapi
>> headers from the kernel, but rely on the ones in libdrm. Due to
>> various issues (as addressed in the series) we have not synchronised
>> the headers, thus users such as the gallium radeon winsys has ended up
>> redefining/duplicating internally.
>
> The problem is that you generally need the latest headers to support
> the latest features in the user mode stacks, so if the user tries to
> build against an older kernel, they will be missing definitions.
>
Pretty much that's why we have the loose release criteria for libdrm.
Get the functionality upstream (mainline or -next), pull the changes
(sync) and release. As the sync has been busted for a while, people
are forced to do unpleasant things.

-Emil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/