[Mesa-dev] [PATCH 1/2] util: Fix foreach_list_typed_safe when exec_node is not at offset 0.

[Matt Turner]

On Mon, Mar 9, 2015 at 7:32 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>
> On Mon, Mar 9, 2015 at 7:24 PM, Matt Turner <mattst88 at gmail.com> wrote:
>>
>> On Mon, Mar 9, 2015 at 6:36 PM, Kenneth Graunke <kenneth at whitecape.org>
>> wrote:
>> > From: Jason Ekstrand <jason.ekstrand at intel.com>
>> >
>> > __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 <jason.ekstrand at intel.com>
>> > Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>> > ---
>> >  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.

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?