<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 9, 2015 at 7:59 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:54 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
><br>
><br>
> On Mon, Mar 9, 2015 at 7:48 PM, Connor Abbott <<a href="mailto:cwabbott0@gmail.com">cwabbott0@gmail.com</a>> wrote:<br>
>><br>
>> 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>><br>
>> >> 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<br>
>> >>> 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<br>
>> >>> *before)<br>
>> >>>             exec_node_data(__type, (__list)->head, __field),<br>
>> >>> \<br>
>> >>>                 * __next =<br>
>> >>> \<br>
>> >>>             exec_node_data(__type, (__node)->__field.next, __field);<br>
>> >>> \<br>
>> >>> -        __next != NULL;<br>
>> >>> \<br>
>> >>> +        &__next->__field != NULL;<br>
>> >>> \<br>
>> >><br>
>> >> I'm not understanding now the address of __next->__field can ever be<br>
>> >> 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<br>
>> > "exec_node_is_tail_sentinel(&__next->__field)"<br>
>><br>
>> 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,<br>
><br>
><br>
> No, we can't.  The whole point of the *_safe versions is that we never touch<br>
> the current node after we've let the client execute code.  We stash off the<br>
> next one so that, even if the delete the current one, we still have<br>
> something to give them next iteration.<br>
> --Jason<br>
<br>
</div></div>Ah, right. My only concern is that doing pointer arithmetic on NULL<br>
isn't defined, and given that what we're doing involves underflowing<br>
the pointer so it wraps around to a large address (when subtracting in<br>
exec_node_get_data()) and then overflowing it back to 0 (when doing<br>
&__next->field), it's likely that some compiler might come along and<br>
"optimize" this.<br></blockquote><div><br></div><div>We could cast everything through uintptr_t but that's gonna get messy...<br></div><div> </div><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>
><br>
>><br>
>> 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>
>><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>
><br>
><br>
</div></div></blockquote></div><br></div></div>