On 03/10/2015 08:04 PM, Connor Abbott wrote: > On Tue, Mar 10, 2015 at 10:54 PM, Ian Romanick <i...@freedesktop.org> wrote: >> On 03/09/2015 06:36 PM, Kenneth Graunke 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. >> >> Wait... what? That is 100% wrong. They are pointers to the exec_node, >> or every assumption of the list structure (like checking for the end of >> the list) breaks. > > No, this is the foreach_list_safe code so __next is the user-specified > variable pointing to the structure containing the next exec_node. Look > at the code below the change, where we say "__next = > exec_node_data(__type, (__next)->__field.next, __field))"
Which doesn't completely show up in the diff context. Okay. But in that case... >>> 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; >>> \ I agree with Matt that this looks really insane. Maybe just add another variable that points to the exec_node? I guess you'd need to be able to declare variables with multiple types in the initializer of the for-statement. Also... I'm not a fan of these ever-growing macros. They're a lot like alligators. They're pretty cute when they're small, but when they grow up they drown you in a river and eat you. I feel like the users of this macro should just use foreach_list_safe, and use exec_node_data by hand. That would simplify things a lot by hiding less from the programmer. >>> __node = __next, __next = >>> \ >>> exec_node_data(__type, (__next)->__field.next, __field)) >>> >>> @@ -693,7 +693,7 @@ inline void exec_node::insert_before(exec_list *before) >>> exec_node_data(__type, (__list)->tail_pred, __field), >>> \ >>> * __prev = >>> \ >>> exec_node_data(__type, (__node)->__field.prev, __field); >>> \ >>> - __prev != NULL; >>> \ >>> + &__prev->__field != NULL; >>> \ >>> __node = __prev, __prev = >>> \ >>> exec_node_data(__type, (__prev)->__field.prev, __field)) >>> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev