[Mesa-dev] [PATCH 2/2] panfrost: Implement custom block/instruction iterators

Boris Brezillon boris.brezillon at collabora.com
Tue Aug 13 22:35:20 UTC 2019


On Tue, 13 Aug 2019 22:25:49 +0200
Boris Brezillon <boris.brezillon at collabora.com> wrote:

> On Tue, 13 Aug 2019 12:59:03 -0700
> Alyssa Rosenzweig <alyssa.rosenzweig at collabora.com> wrote:
> 
> > Could you explain a little bit more why this is neded? What do these
> > helpers do that the generic list.h helpers can't?  
> 
> The _safe attribute in generic helpers is just about handling the case
> where the current item is removed from the list (or a new one is
> inserted and you want to skip the new entry in your iteration). Some
> users of the mir_foreach_xxx() iterators break this rule: they remove
> entries beyond the current one. That's okay if the removed entry is
> after the one stored in the temporary 'storage' param (also called
> next/prev sometimes), but when one actually removes the next/prev
> entry, the safe iterator ends up manipulating an element that's been
> removed from the list (->{prev,next} might be invalid) or even worse,
> something that's been released (use-after-free).
> 
> > Would it be possible
> > to extend the shared list.h helpers (might this contain functionality
> > interesting to other drivers as well)?  
> 
> Hm, not without making them more complex I'm afraid. And since
> most users are happy with the current implementation I'm not entirely
> sure there's a need for such a feature.

Taking back what I said previously. Looks like we can add 2 simple macros
to do that.

--->8---
diff --git a/src/util/list.h b/src/util/list.h
index 96e2f24695c2..979e68fd806a 100644
--- a/src/util/list.h
+++ b/src/util/list.h
@@ -231,6 +231,11 @@ static inline void list_validate(const struct list_head *list)
        pos = __next,                                                   \
        __next = LIST_ENTRY(type, __next->member.next, member))
 
+#define list_del_safe(type, item, member)                               \
+        if (__next == item)                                             \
+                __next = LIST_ENTRY(type, __next->member.next, member); \
+        list_del(&item->member)
+
 #define list_for_each_entry_rev(type, pos, head, member)                \
    for (type *pos = LIST_ENTRY(type, (head)->prev, member),             \
             *__prev = LIST_ENTRY(type, pos->member.prev, member);      \
@@ -246,6 +251,11 @@ static inline void list_validate(const struct list_head *list)
        pos = __prev,                                                   \
         __prev = LIST_ENTRY(type, __prev->member.prev, member))
 
+#define list_del_safe_rev(type, item, member)                           \
+        if (__prev == item)                                             \
+                __prev = LIST_ENTRY(type, __prev->member.prev, member); \
+        list_del(&item->member)
+
 #define list_for_each_entry_from(type, pos, start, head, member)        \
    for (type *pos = LIST_ENTRY(type, (start), member);                  \
        &pos->member != (head);                                         \


More information about the mesa-dev mailing list