<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 9, 2015 at 7:48 PM, Connor Abbott <span dir="ltr"><<a href="mailto:cwabbott0@gmail.com" target="_blank">cwabbott0@gmail.com</a>></span> wrote:<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">On Mon, Mar 9, 2015 at 10:35 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>
> On Mon, Mar 9, 2015 at 7:24 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>
>> On Mon, Mar 9, 2015 at 6:36 PM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote:<br>
>>> 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>
>> I'm not understanding now the address of __next->__field can ever be NULL.<br>
>><br>
>> __next is something with an embedded struct exec_node, so don't we<br>
>> want "__next->__field != NULL" without the address-of operator?<br>
><br>
> Sorry, that should have been "exec_node_is_tail_sentinel(&__next->__field)"<br>
<br>
</div></div>No, that won't work. We want to bail out if the *current* node is the<br>
tail sentinel, not the next one. If the next node is the tail<br>
sentinel, then the current one is the last element, so we have to go<br>
through the loop once more. We could use exec_node_is_tail_sentinel()<br>
on the current node,</blockquote><div><br></div><div>No, we can't.  The whole point of the *_safe versions is that we never touch the current node after we've let the client execute code.  We stash off the next one so that, even if the delete the current one, we still have something to give them next iteration.<br></div><div>--Jason<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> but we've already dereferenced the pointer<br>
earlier when getting the next node so it would be less efficient to<br>
dereference it again.<br>
<div class="HOEnZb"><div class="h5"><br>
> _______________________________________________<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>
_______________________________________________<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>