Wshadow: Better name for 'optarg'?

Wshadow: Better name for 'optarg'?

[Philippe Mathieu-Daudé]

Hi,

I'm getting a bunch of errors for 'optarg' declared in <unistd.h>:

NAME
      getopt – get option character from command line argument list

LIBRARY
      Standard C Library (libc, -lc)

SYNOPSIS
      #include <unistd.h>

      extern char *optarg;


qom/object_interfaces.c:262:53: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)
                                                     ^
qom/object_interfaces.c:298:46: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
bool user_creatable_add_from_str(const char *optarg, Error **errp)
                                              ^
qom/object_interfaces.c:313:49: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
void user_creatable_process_cmdline(const char *optarg)
                                                 ^
util/guest-random.c:90:45: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
int qemu_guest_random_seed_main(const char *optarg, Error **errp)
                                             ^
trace/control.c:288:34: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
void trace_opt_parse(const char *optarg)
                                  ^
/Users/philmd/source/qemu/include/qemu/plugin.h:245:54: error: 
declaration shadows a variable in the global scope [-Werror,-Wshadow]
static inline void qemu_plugin_opt_parse(const char *optarg,
                                                      ^
os-posix.c:103:31: error: declaration shadows a variable in the global 
scope [-Werror,-Wshadow]
bool os_set_runas(const char *optarg)
                               ^
os-posix.c:176:32: error: declaration shadows a variable in the global 
scope [-Werror,-Wshadow]
void os_set_chroot(const char *optarg)
                                ^
softmmu/tpm.c:178:59: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
                                                           ^
/Users/philmd/source/qemu/include/qemu/plugin.h:245:54: error: 
declaration shadows a variable in the global scope [-Werror,-Wshadow]
static inline void qemu_plugin_opt_parse(const char *optarg,
                                                      ^
softmmu/vl.c:1069:44: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
static void parse_display_qapi(const char *optarg)
                                            ^
softmmu/vl.c:1224:39: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
static void monitor_parse(const char *optarg, const char *mode, bool pretty)
                                       ^
softmmu/vl.c:1598:17: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
     const char *optarg;
                 ^
softmmu/vl.c:1634:17: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
     const char *optarg = qdict_get_try_str(qdict, "type");
                 ^
softmmu/vl.c:1784:45: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
static void object_option_parse(const char *optarg)
                                             ^
softmmu/vl.c:2710:17: error: declaration shadows a variable in the 
global scope [-Werror,-Wshadow]
     const char *optarg;
                 ^
net/net.c:1680:35: error: declaration shadows a variable in the global 
scope [-Werror,-Wshadow]
bool netdev_is_modern(const char *optarg)
                                   ^
net/net.c:1714:38: error: declaration shadows a variable in the global 
scope [-Werror,-Wshadow]
void netdev_parse_modern(const char *optarg)
                                      ^
net/net.c:1728:60: error: declaration shadows a variable in the global 
scope [-Werror,-Wshadow]
void net_client_parse(QemuOptsList *opts_list, const char *optarg)
                                                            ^
semihosting/config.c:134:49: error: declaration shadows a variable in 
the global scope [-Werror,-Wshadow]
int qemu_semihosting_config_options(const char *optarg)
                                                 ^

/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: 
note: previous declaration is here
extern char *optarg;                    /* getopt(3) external variables */
              ^

Do we want to clean those? Any good name suggestion?

Thanks,

Phil.

Re: Wshadow: Better name for 'optarg'?

[Daniel P. Berrangé]

On Wed, Oct 04, 2023 at 12:05:04PM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> I'm getting a bunch of errors for 'optarg' declared in <unistd.h>:
> 
> NAME
>      getopt – get option character from command line argument list
> 
> LIBRARY
>      Standard C Library (libc, -lc)
> 
> SYNOPSIS
>      #include <unistd.h>
> 
>      extern char *optarg;
> 
> 
> qom/object_interfaces.c:262:53: error: declaration shadows a variable in the
> global scope [-Werror,-Wshadow]
> ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)

snip

> Do we want to clean those? Any good name suggestion?

Yes.  any of "argval", "opts", "optstr", "optval".

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 :|

Re: Wshadow: Better name for 'optarg'?

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1281 bytes --]

On Wed, Oct 4, 2023, 4:16 AM Daniel P. Berrangé <berrange@redhat.com> wrote:

> On Wed, Oct 04, 2023 at 12:05:04PM +0200, Philippe Mathieu-Daudé wrote:
> > Hi,
> >
> > I'm getting a bunch of errors for 'optarg' declared in <unistd.h>:
> >
> > NAME
> >      getopt – get option character from command line argument list
> >
> > LIBRARY
> >      Standard C Library (libc, -lc)
> >
> > SYNOPSIS
> >      #include <unistd.h>
> >
> >      extern char *optarg;
> >
> >
> > qom/object_interfaces.c:262:53: error: declaration shadows a variable in
> the
> > global scope [-Werror,-Wshadow]
> > ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)
>
> snip
>
> > Do we want to clean those? Any good name suggestion?
>
> Yes.  any of "argval", "opts", "optstr", "optval".
>

For the parsing in bsd-user I just removed the variable entirely and
removed the updating of its value since the parsing code was trying to do
what getopt also did...

Warner

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 :|
>
>
>

[-- Attachment #2: Type: text/html, Size: 2599 bytes --]

Re: Wshadow: Better name for 'optarg'?

[Richard Henderson]

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.


r~

Re: Wshadow: Better name for 'optarg'?

[Thomas Huth]

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.

  Thomas

Re: Wshadow: Better name for 'optarg'?

[Philippe Mathieu-Daudé]

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.

Regards,

Phil.

Re: Wshadow: Better name for 'optarg'?

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

On Wed, Oct 4, 2023, 11:44 AM Philippe Mathieu-Daudé <philmd@linaro.org>
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 had trouble with -Wshadow=local with clang too.

In general I agree not wanting it by default... but for globals defined by
the standard, we'd definitely want to fix.

Warner

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.
>
> Regards,
>
> Phil.
>
>

[-- Attachment #2: Type: text/html, Size: 2626 bytes --]

Re: Wshadow: Better name for 'optarg'?

[Thomas Huth]

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).

  Thomas

Re: Wshadow: Better name for 'optarg'?

[Daniel P. Berrangé]

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 :|

Re: Wshadow: Better name for 'optarg'?

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> 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.

I'm aiming just for -Wshadow=local now.  If somebody else gets us all
the way to -Wshadow, I'll clap from the sidelines.

I'm mildly skeptical about -Wshadow without =local when targeting a wide
range of toolchains over a long time.

Not an objection to cleanup patches such as yours!

Re: Wshadow: Better name for 'optarg'?

[Philippe Mathieu-Daudé]

On 5/10/23 07:17, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> 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.
> 
> I'm aiming just for -Wshadow=local now.  If somebody else gets us all
> the way to -Wshadow, I'll clap from the sidelines.
> 
> I'm mildly skeptical about -Wshadow without =local when targeting a wide
> range of toolchains over a long time.

Well sorry about the confusion, this is an oversight from my part:
I didn't understood your work is focused on GCC, so I was trying to
get it working on my Darwin host which default to Clang (from what
Warner also said, it seems to be the default on FreeBSD too).

> Not an objection to cleanup patches such as yours!
> 

Re: Wshadow: Better name for 'optarg'?

[Philippe Mathieu-Daudé]

On 4/10/23 19:56, 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).

Nah I'm not interested in such a quest, this is simply an oversight
that this is restricted to GCC.

My view on those warnings cleanups is, once we start, either we
finish the full conversion and enforce the warning, or better not
to start wasting energy. I wanted to help Markus, getting closer
to the end. Sorry about the confusion, I'll wait this get enforced
for our GCC jobs on CI before revisiting.

Regards,

Phil.

Re: Wshadow: Better name for 'optarg'?

[Daniel P. Berrangé]

On Thu, Oct 05, 2023 at 07:17:17AM +0200, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
> > 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.
> 
> I'm aiming just for -Wshadow=local now.  If somebody else gets us all
> the way to -Wshadow, I'll clap from the sidelines.
> 
> I'm mildly skeptical about -Wshadow without =local when targeting a wide
> range of toolchains over a long time.

We don't need to claim that QEMU will build warning-free on all possible
toolchains, only our CI covered platforms get that expectation. If users
see warnings on untested toolchains they can either send further patches,
or turn off -Werror, and/or contribute to CI coverage.

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 :|

Re: Wshadow: Better name for 'optarg'?

[Claudio Fontana]

On 10/4/23 12:05, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> I'm getting a bunch of errors for 'optarg' declared in <unistd.h>:
> 
> NAME
>       getopt – get option character from command line argument list
> 
> LIBRARY
>       Standard C Library (libc, -lc)
> 
> SYNOPSIS
>       #include <unistd.h>
> 
>       extern char *optarg;
> 
> 
> qom/object_interfaces.c:262:53: error: declaration shadows a variable in 
> the global scope [-Werror,-Wshadow]
> ObjectOptions *user_creatable_parse_str(const char *optarg, Error **errp)
>                                                      ^
> qom/object_interfaces.c:298:46: error: declaration shadows a variable in 
> the global scope [-Werror,-Wshadow]
> bool user_creatable_add_from_str(const char *optarg, Error **errp)
>                                               ^
> qom/object_interfaces.c:313:49: error: declaration shadows a variable in 
> the global scope [-Werror,-Wshadow]
> void user_creatable_process_cmdline(const char *optarg)
>                                                  ^
> util/guest-random.c:90:45: error: declaration shadows a variable in the 
> global scope [-Werror,-Wshadow]
> int qemu_guest_random_seed_main(const char *optarg, Error **errp)
>                                              ^
> trace/control.c:288:34: error: declaration shadows a variable in the 
> global scope [-Werror,-Wshadow]
> void trace_opt_parse(const char *optarg)
>                                   ^
> /Users/philmd/source/qemu/include/qemu/plugin.h:245:54: error: 
> declaration shadows a variable in the global scope [-Werror,-Wshadow]
> static inline void qemu_plugin_opt_parse(const char *optarg,
>                                                       ^
> os-posix.c:103:31: error: declaration shadows a variable in the global 
> scope [-Werror,-Wshadow]
> bool os_set_runas(const char *optarg)
>                                ^
> os-posix.c:176:32: error: declaration shadows a variable in the global 
> scope [-Werror,-Wshadow]
> void os_set_chroot(const char *optarg)
>                                 ^
> softmmu/tpm.c:178:59: error: declaration shadows a variable in the 
> global scope [-Werror,-Wshadow]
> int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
>                                                            ^
> /Users/philmd/source/qemu/include/qemu/plugin.h:245:54: error: 
> declaration shadows a variable in the global scope [-Werror,-Wshadow]
> static inline void qemu_plugin_opt_parse(const char *optarg,
>                                                       ^
> softmmu/vl.c:1069:44: error: declaration shadows a variable in the 
> global scope [-Werror,-Wshadow]
> static void parse_display_qapi(const char *optarg)
>                                             ^
> softmmu/vl.c:1224:39: error: declaration shadows a variable in the 
> global scope [-Werror,-Wshadow]
> static void monitor_parse(const char *optarg, const char *mode, bool pretty)
>                                        ^
> softmmu/vl.c:1598:17: error: declaration shadows a variable in the 
> global scope [-Werror,-Wshadow]
>      const char *optarg;
>                  ^
> softmmu/vl.c:1634:17: error: declaration shadows a variable in the 
> global scope [-Werror,-Wshadow]
>      const char *optarg = qdict_get_try_str(qdict, "type");
>                  ^
> softmmu/vl.c:1784:45: error: declaration shadows a variable in the 
> global scope [-Werror,-Wshadow]
> static void object_option_parse(const char *optarg)
>                                              ^
> softmmu/vl.c:2710:17: error: declaration shadows a variable in the 
> global scope [-Werror,-Wshadow]
>      const char *optarg;
>                  ^
> net/net.c:1680:35: error: declaration shadows a variable in the global 
> scope [-Werror,-Wshadow]
> bool netdev_is_modern(const char *optarg)
>                                    ^
> net/net.c:1714:38: error: declaration shadows a variable in the global 
> scope [-Werror,-Wshadow]
> void netdev_parse_modern(const char *optarg)
>                                       ^
> net/net.c:1728:60: error: declaration shadows a variable in the global 
> scope [-Werror,-Wshadow]
> void net_client_parse(QemuOptsList *opts_list, const char *optarg)
>                                                             ^
> semihosting/config.c:134:49: error: declaration shadows a variable in 
> the global scope [-Werror,-Wshadow]
> int qemu_semihosting_config_options(const char *optarg)
>                                                  ^
> 
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14: 
> note: previous declaration is here
> extern char *optarg;                    /* getopt(3) external variables */
>               ^
> 
> Do we want to clean those? Any good name suggestion?

I would suggest "arg".

> 
> Thanks,
> 
> Phil.
>