libdrm/amdgpu - Fixup typedef not to hide pointer type

Christian König deathsimple at vodafone.de
Fri Sep 16 10:49:44 UTC 2016


Well thinking more about that what you probably could do is changing the 
typedefs to put the pointer into a single element structure.

That should break only when somebody really abused the API and shouldn't 
change the ABI.

Regards,
Christian.

Am 16.09.2016 um 11:12 schrieb Christian König:
> Am 16.09.2016 um 11:02 schrieb Edward O'Callaghan:
>> Hi Christian,
>>
>> On 09/16/2016 06:49 PM, Christian König wrote:
>>> NAK, that is clearly an API breakage.
>> It should have never been typedef'ed in the first place. Does that mean
>> we would have to bump version for API change? What is the procedure 
>> there?
>
> Keep it as it is, you can't change it even if we bump the version 
> number you would break existing userspace.
>
>>
>>> BTW: Why would we want to stop hiding the type?
>> Quite a few reasons, I'll start with to justify the change:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/CodingStyle?id=refs/tags/v4.8-rc6#n276 
>>
>
> This doesn't apply here, this is a user space library interface and 
> not the kernel.
>
> The structure types are not defined outside of the library and so you 
> can't dereference or NULL check them.
>
> Regards,
> Christian.
>
>>
>> Suppose you have 'amdgpu_semaphore_handle' as typedef of a pointer type
>> and you had some function 'void f(amdgpu_semaphore_handle * h) {}'.
>> Suppose now, within 'f()' we deference 'h' and use the result in some
>> way. The issue is that it is not directly obvious that we have a double
>> pointer type and so while it maybe the case that '(*h != NULL)' it could
>> well be the case that '(h == NULL)'.
>>
>> Kind Regards,
>> Edward.
>>
>>> Christian.
>>>
>>> Am 16.09.2016 um 10:46 schrieb Edward O'Callaghan:
>>>> Oops, turns out I mailed to dri-devel by mistake so resending here.
>>>>
>>>> The following series fixes up libdrm/amdgpu such that to not hide
>>>> a pointer type behind a typedef.
>>>>
>>>> Please Review,
>>>>
>>>> Edward O'Callaghan (3):
>>>>    [PATCH 1/3] amdgpu: Fix amdgpu_va_handle typedef not to hide 
>>>> pointer
>>>>    [PATCH 2/3] amdgpu: Fix amdgpu_semaphore_handle typedef not to hide
>>>>    [PATCH 3/3] amdgpu: Fix amdgpu_bo_list_handle typedef not to hide
>>>> _______________________________________________
>>>> amd-gfx mailing list
>>>> amd-gfx at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>
>



More information about the amd-gfx mailing list