On Mon, Apr 2, 2012 at 7:40 AM, Tolga Dalman <tolga.dal...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
