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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Tue, 2 Apr 2024 09:48:02 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: Eric Dumazet <edumazet@...gle.com>, "David S. Miller"
 <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, Dmitry Safonov
 <0x7f454c46@...il.com>, Heiner Kallweit <hkallweit1@...il.com>,
 <nex.sw.ncis.osdt.itp.upstreaming@...el.com>, <netdev@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 2/2] netdev_queues: fix -Wshadow / Sparse
 shadow warnings throughout the file

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
?