[Mesa-dev] [PATCH] glsl/list: make nodes safe for double removal, etc.

[Rob Clark]

On Mon, Apr 4, 2016 at 10:34 AM, Iago Toral <itoral at igalia.com> wrote:
> On Sat, 2016-04-02 at 17:09 -0400, Rob Clark wrote:
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> 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.

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.

BR,
-R

> Iago
>
>> Signed-off-by: Rob Clark <robclark at freedesktop.org>
>> Dared-by: Jason Ekstrand <jason at jlekstrand.net>
>> ---
>>  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
>
>