On Mon, 2016-04-04 at 11:04 -0400, Rob Clark wrote: > On Mon, Apr 4, 2016 at 10:34 AM, Iago Toral <[email protected]> wrote: > > On Sat, 2016-04-02 at 17:09 -0400, Rob Clark wrote: > >> From: Rob Clark <[email protected]> > >> > >> It's no extra overhead to do a _self_link() and it eliminates a class of > >> potential problems. > > > > it can also hide actual programming mistakes that would otherwise be > > immediately visible... does this actually help something specific? > > Well, basically it avoids needing to explicitly do a _self_link() > after removing a node in cases where you know (for example) that you > might end up removing multiple times. The kernel list implementation > does have separate list_del() and list_del_init(), which would be a > different possible way to go. > > But in my experience the programming mistakes that this would hide are > simply cases where you wanted to do list_del_init() instead of > list_del(), so I'm curious about which other cases you are worried > about.
I was thinking about incorrect double deletions of the same node. If we are never supposed to do that this would be hiding a programming mistake. In any case, I don't have a strong preference myself, I was just curious about the reasons for the change. > Anyways, this patch doesn't solve something in particular, it is > mostly just a response to a comment Jason made about my usage of > immediate _self_link() after removal on another patch. I think having two functions for this, one that sets the pointers to NULL and one that self links is a better solution since you can choose the one that makes sense for what you are actually trying to do, but I don't have a strong preference, if others like this approach I am fine with it too. Iago > > > >> Signed-off-by: Rob Clark <[email protected]> > >> Dared-by: Jason Ekstrand <[email protected]> > >> --- > >> src/compiler/glsl/list.h | 15 +++++++-------- > >> 1 file changed, 7 insertions(+), 8 deletions(-) > >> > >> diff --git a/src/compiler/glsl/list.h b/src/compiler/glsl/list.h > >> index a1c4d82..77e1f67 100644 > >> --- a/src/compiler/glsl/list.h > >> +++ b/src/compiler/glsl/list.h > >> @@ -165,19 +165,18 @@ exec_node_get_prev(struct exec_node *n) > >> } > >> > >> static inline void > >> -exec_node_remove(struct exec_node *n) > >> +exec_node_self_link(struct exec_node *n) > >> { > >> - n->next->prev = n->prev; > >> - n->prev->next = n->next; > >> - n->next = NULL; > >> - n->prev = NULL; > >> + n->next = n; > >> + n->prev = n; > >> } > >> > >> static inline void > >> -exec_node_self_link(struct exec_node *n) > >> +exec_node_remove(struct exec_node *n) > >> { > >> - n->next = n; > >> - n->prev = n; > >> + n->next->prev = n->prev; > >> + n->prev->next = n->next; > >> + exec_node_self_link(n); > >> } > >> > >> static inline void > > > > > _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
