<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 9, 2015 at 11:06 PM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon, Mar 9, 2015 at 6:36 PM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote:<br>
</span><span class="">> From: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>><br>
><br>
> __next and __prev are pointers to the structure containing the exec_node<br>
> link, not the embedded exec_node.  NULL checks would fail unless the<br>
> embedded exec_node happened to be at offset 0 in the parent struct.<br>
><br>
> Signed-off-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>><br>
> Reviewed-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
> ---<br>
>  src/glsl/list.h | 4 ++--<br>
>  1 file changed, 2 insertions(+), 2 deletions(-)<br>
><br>
> diff --git a/src/glsl/list.h b/src/glsl/list.h<br>
> index ddb98f7..680e963 100644<br>
> --- a/src/glsl/list.h<br>
> +++ b/src/glsl/list.h<br>
> @@ -684,7 +684,7 @@ inline void exec_node::insert_before(exec_list *before)<br>
>             exec_node_data(__type, (__list)->head, __field),                \<br>
>                 * __next =                                                  \<br>
>             exec_node_data(__type, (__node)->__field.next, __field);        \<br>
> -        __next != NULL;                                                    \<br>
> +        &__next->__field != NULL;                                          \<br>
<br>
</span>Wow. Okay, so this apparently relies on exec_node_data (which is<br>
pretty confusingly named, I think) returning a negative pointer value<br>
if passed NULL as its node parameter, such that &...->__field<br>
effectively adds back the offset of the field in the struct, giving a<br>
NULL pointer?<br>
<br>
That's sufficiently confusing to warrant a lengthy comment at the very least.<br>
<br>
Testing that the address of a field in a struct is NULL... just looks insane.<br></blockquote><br></div><div class="gmail_quote">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?<br></div><div class="gmail_quote">--Jason<br></div><div class="gmail_quote"><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</div></div></blockquote></div><br></div></div>