[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:       Ingo Molnar <mingo () kernel ! org>
Date:       2024-04-17 11:23:18
Message-ID: Zh+xJpjaHjF2qvmV () gmail ! com
[Download RAW message or body]


* Matthew Wilcox <willy@infradead.org> 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@chromium.org> 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

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