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

[Jason Ekstrand]

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.

Put differently, &__next->field undoes the pointer arithmatic that
exec_node_data(__type, ptr, __field) does.  Ideallly, we would like __next
to be a pointer of type "struct exec_node" and do the conversion to "__type
*" later.  Unfortunately, C doesn't let us do that inside the for loop.  So
we settle for extra pointer arithmetic.

The other option, of course, would be to use a struct but then we couldn't
make a variable named __node on behalf of the caller.
--Jason


>
> >          __node = __next, __next =
>     \
> >             exec_node_data(__type, (__next)->__field.next, __field))
> >
> > @@ -693,7 +693,7 @@ inline void exec_node::insert_before(exec_list
> *before)
> >             exec_node_data(__type, (__list)->tail_pred, __field),
>    \
> >                 * __prev =
>     \
> >             exec_node_data(__type, (__node)->__field.prev, __field);
>     \
> > -        __prev != NULL;
>     \
> > +        &__prev->__field != NULL;
>     \
> >          __node = __prev, __prev =
>     \
> >             exec_node_data(__type, (__prev)->__field.prev, __field))
> >
> > --
> > 2.2.2
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150309/82f6d2a6/attachment-0001.html>