On Mon, Mar 9, 2015 at 11:06 PM, Matt Turner <matts...@gmail.com> wrote:

> On Mon, Mar 9, 2015 at 6:36 PM, Kenneth Graunke <kenn...@whitecape.org>
> wrote:
> > From: Jason Ekstrand <jason.ekstr...@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.ekstr...@intel.com>
> > Reviewed-by: Kenneth Graunke <kenn...@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;
>     \
>
> Wow. Okay, so this apparently relies on exec_node_data (which is
> pretty confusingly named, I think) returning a negative pointer value
> if passed NULL as its node parameter, such that &...->__field
> effectively adds back the offset of the field in the struct, giving a
> NULL pointer?
>
> That's sufficiently confusing to warrant a lengthy comment at the very
> least.
>
> Testing that the address of a field in a struct is NULL... just looks
> insane.
>

How about we do things slightly differently and check "(__node)->field.next
!= NULL" just like we do on regular versions.  Since the check happens
between the increment step and running the user's code, __node is valid for
every invocation of the checking condition.  Would that make you feel
better about it?
--Jason

_______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to