Re: Wshadow: Better name for 'optarg'?

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Oct 04, 2023 at 07:56:35PM +0200, Thomas Huth wrote:
> On 04/10/2023 19.43, Philippe Mathieu-Daudé wrote:
> > On 4/10/23 19:35, Thomas Huth wrote:
> > > On 04/10/2023 19.23, Richard Henderson wrote:
> > > > On 10/4/23 03:05, Philippe Mathieu-Daudé wrote:
> > > > > Hi,
> > > > > 
> > > > > I'm getting a bunch of errors for 'optarg' declared in <unistd.h>:
> > > > 
> > > > I thought things like this is why we were trying -Wshadow=local.
> > > > 
> > > > I think it's unlikely that we'll be able to prevent all such cases.
> > > 
> > > Given the broad range of operating systems and libraries that we
> > > support in QEMU, I agree with Richard - it will likely be impossible
> > > to enable that option without =local by default without risking that
> > > compilation breaks on some exotic systems or new versions of various
> > > libraries.
> > 
> > -Wshadow=local doesn't seem to work here which is why I switched
> > to -Wshadow. I probably misunderstood something from Markus cover
> > letter. My setup is:
> > 
> > C compiler for the host machine: clang (clang 14.0.3 "Apple clang
> > version 14.0.3 (clang-1403.0.22.14.1)")
> > 
> > I suppose we'll figure that out when eventually enabling -Wshadow=local
> > on CI. Meanwhile I already cleaned the 'optarg' warnings that were
> > bugging me, see:
> > https://lore.kernel.org/qemu-devel/20231004120019.93101-1-philmd@linaro.org/
> > I'll try to get -Wshadow=local, but the other series still seems a
> > good cleanup, as I used more meaningful variable names.
> 
> If I got that right, -Wshadow=local only works with gcc and not with clang
> yet, so we'll need a check in configure or meson.build and will be able to
> only use it when it's available.
> 
> If we could use "-Wshadow" to check global variables, too, that would be
> great, but given my experience with some other project, it's very unlikely
> that you can get it running reliably everywhere, since there is often a bad
> library header somewhere that declares some global variable(s) that spoil
> your plans (IIRC I've once seen a bad library that even declared a global
> variable called "x" ... and you certainly don't want to rename all
> occurances of "x" in the QEMU source code just because of a bad library ...
> however, that's been many years ago, though, maybe the situation got better
> nowadays, so if you like, feel free to continue your quest - just be aware
> that it might not be solvable at the end).

FWIW, libvirt has successfully used -Wshadow for 10 years with no
real ongoing burden. There is of course a bit of an initial pain,
but if you have good CI coverage (as we do), we'll be able to validate
all the important platforms.

Windows is normally the worst platform for -Wshadow since some win32
headers defnie some incredibly generic names !

For the unimportant/niche platforms, people can send fixes as needed,
and shouldn't be using -Werror for production builds anyway.

IOW, if we can see a path to going all the way to -Wshadow that isn't
going to need 100's more patches, we might as well do it, or at least
accept patches to walk towards full -Wshadow.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|