Re: [PATCH 00/19] Enable -Wshadow=local for kernel/sched

From: Ingo Molnar
Date: Wed Apr 17 2024 - 07:23:32 EST



* Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:

> On Tue, Apr 16, 2024 at 05:29:02PM -0700, Linus Torvalds wrote:
> > On Tue, 16 Apr 2024 at 14:15, Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> > >
> > > I was looking at -Wshadow=local again, and remembered this series. It
> > > sounded like things were close, but a tweak was needed. What would be
> > > next to get this working?
> >
> > So what is the solution to
> >
> > #define MAX(a,b) ({ \
> > typeof(a) __a = (a); \
> > typeof(b) __b = (b); \
> > __a > __b ? __a : __b; \
> > })
>
> #define __MAX(a, __a, b, __b) ({ \
> typeof(a) __a = (a); \
> typeof(b) __b = (b); \
> __a > __b ? __a : __b; \
> })
>
> #define MAX(a, b) __MAX(a, UNIQUE_ID(a), b, UNIQUE_ID(b))
>
> At least, I think that was the plan. This was two years ago and I've
> mostly forgotten.

I think as long as we can keep any additional complexity inside macros it
would be acceptable, at least from the scheduler's POV. A UNIQUE_ID() layer
of indirection for names doesn't sound look a too high price.

I had good reasults with -Wshadow in user-space projects: once the false
positives got ironed out, the vast percentage of new warnings was for
genuinely problematic new code. But they rarely used block-nested macros
like the kernel does.

Thanks,

Ingo