On Tue, May 5, 2015 at 11:13 AM, Neil Roberts <[email protected]> wrote: > Jason Ekstrand <[email protected]> writes: > >> +#define list_for_each_entry(type, pos, head, member) \ >> + for (type *pos = container_of((head)->next, pos, member); \ >> + &pos->member != (head); \ >> + pos = container_of(pos->member.next, pos, member)) > > I think this is technically invalid. The container_of macro dereferences > the pos pointer but at this point it is undefined so I think the > compiler is allowed to do whatever it wants here. There is a comment > above the container_of macro saying exactly that.
Right. I think I've come across that too. We also have a LIST_ELEM (or maybe ITEM) macro in the file that does the type-based thing. I'll fix it up to use that instead. --Jason > Some quick testing with clang shows that it actually takes advantage of > this undefined behaviour and doesn't add the offset to pos to get the > container and it all falls apart. > > A while ago we added the wayland linked-list implementation to Cogl and > we ran into a similar problem. We actually ended up changing the > container_of macro to take a type instead of a sample pointer because > this is such an easy trap to fall into. > > https://git.gnome.org/browse/cogl/commit/?id=1efed1e0a2bce706eb490197 > > Regards, > - Neil _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
