[Mesa-dev] question about container_of

[Emil Velikov]

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.

Thanks again!
Emil