Received-SPF: pass (google.com: domain of linux-kernel+bounces-128390-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Date: Tue, 2 Apr 2024 09:48:02 -0700
From: Jakub Kicinski <kuba@kernel.org>
To: Alexander Lobakin <aleksander.lobakin@intel.com>
Cc: Eric Dumazet <edumazet@google.com>, "David S. Miller"
 <davem@davemloft.net>, Paolo Abeni <pabeni@redhat.com>, Dmitry Safonov
 <0x7f454c46@gmail.com>, Heiner Kallweit <hkallweit1@gmail.com>,
 <nex.sw.ncis.osdt.itp.upstreaming@intel.com>, <netdev@vger.kernel.org>,
 <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net-next 2/2] netdev_queues: fix -Wshadow / Sparse
 shadow warnings throughout the file
Message-ID: <20240402094802.6fb25869@kernel.org>
In-Reply-To: <22021664-6630-4663-ac28-c0df4187d8b6@intel.com>
References: <20240329170000.3241460-1-aleksander.lobakin@intel.com>
	<20240329170000.3241460-3-aleksander.lobakin@intel.com>
	<20240329131857.730c6528@kernel.org>
	<20240329135344.1a310f31@kernel.org>
	<912d1f9f-a88d-4751-8d91-b82a75f82a32@intel.com>
	<CANn89iL+goDgitdic97+um6D-PZDw2xYf=2eYgnNYEYU-Nws0Q@mail.gmail.com>
	<22021664-6630-4663-ac28-c0df4187d8b6@intel.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

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
?