[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:58 PM, Matt Turner <mattst88 at gmail.com> wrote:

> 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?
>

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
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150309/15f76722/attachment.html>