[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-kernel
Subject:    Re: [PATCH 00/19] Enable -Wshadow=local for kernel/sched
From:       Linus Torvalds <torvalds () linux-foundation ! org>
Date:       2024-04-17 0:50:36
Message-ID: CAHk-=wizMKBPZeOy7UiQjNR5Jm103WNzL4sHq4FpZihk7yBsfQ () mail ! gmail ! com
[Download RAW message or body]

On Tue, 16 Apr 2024 at 17:29, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So what is the solution to
>
>     #define MAX(a,b) ({ \

Side note: do not focus on the macro name. I'm not interested in "the
solution is MAX3()" kinds of answers.

And the macro doesn't have to be physically nested like that.

The macro could be a list traversal thing.  Appended is an example
list traversal macro that is type-safe and simple to use, and would
absolutely improve on our current "list_for_each_entry()" in many
ways.

Imagine now traversing a list within an entry that happens while
traversing an outer one. Which is not at all an odd thing, IOW, you'd
have

        traverse(bus_list, bus) {
                traverse(&bus->devices, device) {
                        .. do something with the device ..
                }
        }

this kind of macro use that has internal variables that will
inevitably shadow each other when used in some kind of nested model is
pretty fundamental.

So no. The answer is *NOT* some kind of "don't do that then".

             Linus

PS. The list trraversal thing below worked at some point. It's an old
snippet of mine, it might not work any more. It depends on the kernel
'list_head' definitions, it's not a standalone example.

---

    #define list_traversal_head(type, name, member) union {     \
        struct list_head name;                          \
        struct type *name##_traversal_type;             \
        struct type##_##name##_##member##_traversal_struct
*name##_traversal_info; \
    }

    #define list_traversal_node(name) union {           \
        struct list_head name;                          \
        int name##_traversal_node;                      \
    }

    #define DEFINE_TRAVERSAL(from, name, to, member)            \
    struct to##_##name##_##member##_traversal_struct {          \
        char dummy[offsetof(struct to, member##_traversal_node)]; \
        struct list_head node;                          \
    }

    #define __traverse_type(head, ext) typeof(head##ext)
    #define traverse_type(head, ext) __traverse_type(head, ext)

    #define traverse_offset(head) \
        offsetof(traverse_type(*head,_traversal_info), node)

    #define traverse_is_head(head,  raw) \
        ((void *)(raw) == (void *)(head))

    /*
     * Very annoying. We want 'node' to be of the right type, and __raw to be
     * the underlying "struct list_head". But while we can declare multiple
     * variables in a for-loop in C99, we can't declare multiple _types_.
     *
     * So __raw has the wrong type, making everything pointlessly uglier.
     */
    #define traverse(head, node) \
        for (typeof(*head##_traversal_type) __raw = (void
*)(head)->next, node; \
                node = (void *)__raw + traverse_offset(*head),
!traverse_is_head(head, __raw); \
                __raw = (void *) ((struct list_head *)__raw)->next)

    struct first_struct {
        int offset[6];
        list_traversal_head(second_struct, head, entry);
    };

    struct second_struct {
        int hash;
        int offset[17];
        list_traversal_node(entry);
    };

    DEFINE_TRAVERSAL(first_struct, head, second_struct, entry);

    struct second_struct *find(struct first_struct *p)
    {
        traverse(&p->head, node) {
                if (node->hash == 1234)
                        return node;
        }
        return NULL;
    }

[prev in list] [next in list] [prev in thread] [next in thread]