[Mesa-dev] [PATCH 1/9] util: fix undefined behavior

[nobled]

On Mon, Apr 2, 2012 at 7:40 AM, Tolga Dalman
<tolga.dalman at googlemail.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Hi,
>
>
> On 02.04.2012 00:24, nobled wrote:
>>  #define LIST_FOR_EACH_ENTRY(pos, head, member)                               \
>> -   for (pos = container_of((head)->next, pos, member);                       \
>> +   for (pos = NULL, pos = container_of((head)->next, pos, member);   \
>>       &pos->member != (head);                                         \
>>       pos = container_of(pos->member.next, pos, member))
>
> The container_of macro is obviously error-prone and leads to future
> problems. Your patch works around that buggy API instead of
> really fixing it.
>
> container_of could be defined using the offsetof macro, i.e.,
>
> #define container_of(ptr, type, member) \
>  (void *)((char *)(ptr) - offsetof(type, member))
>
> As far as I can see, container_of is only used sparsely in
> mesa, so applying my proposal should be possible quite easily.
Not really. container_of() is only used *directly* in this one header;
but changing it to require a type parameter requires also changing all
the macros that use it in turn, which actually are used widely in
mesa. Adding the type name to every use of the LIST_FOR_EACH_* macros
also introduces redundancy and the possibility of error, e.g. if the
containing ever changes but not every use of the macro is updated,
there's no compile error but it'll go wrong at runtime. That can't
happen with the current version of it.

container_of() just has one basic caveat that we can easily satisfy,
since like you said, it isn't directly used in many places, and I
don't think future problems are actually likely--any misuse will
trigger uninitialized warnings like the above so they're easily
caught, unlike possible typename mistakes.
>
> Best regards
> Tolga Dalman
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2.0.19 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
>
> iQEcBAEBAgAGBQJPeZAkAAoJEI0vwg8AaIlUzlQH/3dCLXcdoFvE9M7KtC/BEmhT
> Qw+sYrG77VvQaMhB5uM6DpNkEZUkXiZVqObarISwQvup0Wwarvr9QLQu5b2LYzRI
> NqHPEth16ndUnYDAuMeK0x7hc2/Vu7B6D48Q8n9EOs8gezQN3HG9fbswV+u40sl4
> qj3HV7bP9IsYWe+fQOzx5k3NzaJQUjqPX5abfvjav4awV3SwHqm+EEaCEOQp6DYx
> p0snUq9JnVY3HzA+6bu7aOT6p4J8RvchscPCb/6YjxNa35YhtpeYn5Lgg6AnXYQu
> suIil4b8+/eDh9rHOB7DuFHRGoRVUP2NhVko0DbT7yLbKbJRpX9CVtKVbhc7qZo=
> =60Yn
> -----END PGP SIGNATURE-----
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev