Re: [PATCH net-next 2/2] netdev_queues: fix -Wshadow / Sparse shadow warnings throughout the file

[Jakub Kicinski]

On Tue, 2 Apr 2024 17:53:08 +0200 Alexander Lobakin wrote:
> >> But what if there's a function which calls one of these functions and
> >> already has _res or __res or something? I know renaming is enough for
> >> the warnings I mentioned, but without __UNIQUE_ID() anything can happen
> >> anytime, so I wanted to fix that once and for all :z
> >>
> >> I already saw some macros which have a layer of indirection for
> >> __UNIQUE_ID(), but previously they didn't and then there were fixes
> >> which added underscores, renamed variables etc etc...
> >>  
> > 
> > We have hundreds of macros in include/ directory which use local names without
> > __UNIQUE_ID()  
> 
> Most of them were added before __UNIQUE_ID() became norm, weren't they?
> Lots of them were switched to __UNIQUE_ID() because of issues, weren't they?

Lots of ugly code gets into the kernel. Just look at your patch and
then look at mine.

I understand __UNIQUE_ID() may be useful for libraries or global
macros in the kernel, but within a subsystem, for macros which are
rarely used, we can just patch the macro var names. Sprinkling
__UNIQUE_ID() is in bad taste.

> > What is the plan ? Hundreds of patches obfuscating them more than they are ?  
> 
> Only those which flood the console when building with W=12 C=1 to
> recheck that my new code is fine.

I have never seen this warning be useful in the context of a macro.
Sure if you shadow inside a function that may be pernicious.
But well written macro will not be a problem.
I guess that it may be really hard for the compiler to understand that
something was a macro but perhaps we should either:
 - ignore the warning if the shadowing happens inside a compound
   statement
 - add a declaration attribute to turn the warning off
?