<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Mon, Mar 9, 2015 at 7:58 PM, Matt Turner <<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Mar 9, 2015 at 7:32 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > > > On Mon, Mar 9, 2015 at 7:24 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote: >> >> On Mon, Mar 9, 2015 at 6:36 PM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> >> wrote: >> > From: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> >> > >> > __next and __prev are pointers to the structure containing the exec_node >> > link, not the embedded exec_node. NULL checks would fail unless the >> > embedded exec_node happened to be at offset 0 in the parent struct. >> > >> > Signed-off-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> >> > Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> >> > --- >> > src/glsl/list.h | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/src/glsl/list.h b/src/glsl/list.h >> > index ddb98f7..680e963 100644 >> > --- a/src/glsl/list.h >> > +++ b/src/glsl/list.h >> > @@ -684,7 +684,7 @@ inline void exec_node::insert_before(exec_list >> > *before) >> > exec_node_data(__type, (__list)->head, __field), >> > \ >> > * __next = >> > \ >> > exec_node_data(__type, (__node)->__field.next, __field); >> > \ >> > - __next != NULL; >> > \ >> > + &__next->__field != NULL; >> > \ >> >> I'm not understanding now the address of __next->__field can ever be NULL. >> >> __next is something with an embedded struct exec_node, so don't we >> want "__next->__field != NULL" without the address-of operator? > > > No, "__field" is the name of the exec_node field embedded in the __type > struct. So if I have > > struct foo { > /* stuff */ > struct exec_node bar; > } > > as my type, __type is "struct foo", __next and __node are both of type > "__type *", and __field is "bar". So, in order to get form __next to an > exec_node, you have to do &__next->__field because we need the actual > exec_node back. </div></div>More concretely, for something like: foreach_list_typed_safe (bblock_link, successor, link, &predecessor->block->children) I think this macro expands to (after this patch) for (bblock_link * successor = exec_node_data(bblock_link, (&predecessor->block->children)->head, link), * __next = exec_node_data(bblock_link, (successor)->link.next, link); &__next->link != NULL; successor = __next, __next = exec_node_data(bblock_link, (__next)->link.next, link)) How can the address of a field in a struct (e.g., __next->link) be NULL? </blockquote></div> </div><div class="gmail_extra">If the address of the struct is (void *)-8 and the link field is 8 bytes into the struct. In this case, assuming unsigned overflow (I think that's reasonably safe), the address of the link will be (void *)0 </div><div class="gmail_extra">--Jason </div></div>