[Mesa-dev] question about container_of

[Nicolai Hähnle]

On 20.04.2017 18:32, Emil Velikov wrote:
> On 18 April 2017 at 13:55, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>> On Mon, 27 Feb 2017 13:26:11 +0000
>> Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>
>>> Hi Julien,
>>>
>>> On 27 February 2017 at 12:08, Julien Isorce <julien.isorce at gmail.com> wrote:
>>>> Hi,
>>>>
>>>> Since 2012 commit ccff74971203b533bf16b46b49a9e61753f75e6c it is said:
>>>> "sample must be initialized, or else the result is undefined" in the
>>>> description of mesa/src/util/list.h::container_of .
>>>>
>>>> But I can find a few places where it is used without initializing that
>>>> second parameter, i.e. like:
>>>>
>>>> struct A a;
>>>> container_of(ptr, a, member);
>>>>
>>>> Then I can add the "= NULL" but should it be just
>>>> container_of(ptr, struct A, member);
>>>> like in the kernel and some other places in mesa ?
>>>>
>>> Strictly peaking these are toolchain (ASAN iirc) bugs, since there is
>>> no pointer deref, as we're doing pointer arithmetic.
>>
>> Hi Emil,
>>
>> that's what people would usually think. It used to work with GCC. Then
>> came Clang.
>>
>>> Afaict the general decision was to to merge the patch(es) since they
>>> will make actual bugs stand out amongst the noise. In the long run,
>>> it's better to fix the tool (ASAN/other) than trying to "fix" all the
>>> cases in mesa and dozens of other projects. But until then patches are
>>> safe enough ;-)
>>>
>>> That's my take on it, at least.
>>
>> It depends on how container_of() has been defined. For more details,
>> see e.g.:
>> https://cgit.freedesktop.org/wayland/wayland/commit/?id=a18e34417ba3fefeb81d891235e8ebf394a20a74
>>
>> The comment in the code in Mesa is correct for the fallback
>> implementation it has. Maybe things rely on the fallback implementation
>> never being used when compiling with Clang.
>>
>> Julien, some alternatives for container_of use a pointer, others expect
>> a type instead. I believe one must use the correct form.
>>
> Thanks for the correction Pekka - yes, the issue (to deref or not) is
> implementation specific.
> A 'minor' detail that I've missed.
>
> Seems like we use the more prone one - having the second argument
> being a pointer as, opposed to a type.
> Perhaps we should reconsider and update mesa, sooner rather than later.

Can we use typeof in Mesa? It's not standard, but I think both GCC and 
Clang have it, and it makes for a more convenient version of 
container_of that is still correct. The big question is whether it works 
on other compilers.

If it doesn't work on all compilers we need to support, I agree 
switching to the macro with the type argument makes sense.

Cheers,
Nicolai


>
> Thanks again!
> Emil
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.