Received-SPF: pass (google.com: domain of linux-kernel+bounces-147810-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20220302043451.2441320-1-willy@infradead.org> <202404161413.8B4810C5@keescook>
 <CAHk-=wg7+dfpEvDnTnB-eR9QRZ2VySbxOEHbYqL3Ai5wwkon5Q@mail.gmail.com>
In-Reply-To: <CAHk-=wg7+dfpEvDnTnB-eR9QRZ2VySbxOEHbYqL3Ai5wwkon5Q@mail.gmail.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Tue, 16 Apr 2024 17:50:36 -0700
Message-ID: <CAHk-=wizMKBPZeOy7UiQjNR5Jm103WNzL4sHq4FpZihk7yBsfQ@mail.gmail.com>
Subject: Re: [PATCH 00/19] Enable -Wshadow=local for kernel/sched
To: Kees Cook <keescook@chromium.org>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>, Ingo Molnar <mingo@redhat.com>, 
	Peter Zijlstra <peterz@infradead.org>, Juri Lelli <juri.lelli@redhat.com>, 
	Vincent Guittot <vincent.guittot@linaro.org>, Dietmar Eggemann <dietmar.eggemann@arm.com>, 
	Steven Rostedt <rostedt@goodmis.org>, Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>, 
	Daniel Bristot de Oliveira <bristot@redhat.com>, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"

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;
    }