From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: Thomas Huth <thuth@redhat.com>,
Richard Henderson <richard.henderson@linaro.org>,
Markus Armbruster <armbru@redhat.com>
Cc: "QEMU Developers" <qemu-devel@nongnu.org>,
"Daniel P. Berrangé" <berrange@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: Wshadow: Better name for 'optarg'?
Date: Thu, 5 Oct 2023 08:56:39 +0200 [thread overview]
Message-ID: <1628d62e-28ac-58f9-9efb-147db95d2b37@linaro.org> (raw)
In-Reply-To: <11abc551-188e-85c0-fe55-b2b58d35105d@redhat.com>
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.
next prev parent reply other threads:[~2023-10-05 6:57 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-04 10:05 Wshadow: Better name for 'optarg'? Philippe Mathieu-Daudé
2023-10-04 10:15 ` Daniel P. Berrangé
2023-10-04 13:14 ` Warner Losh
2023-10-04 17:23 ` Richard Henderson
2023-10-04 17:35 ` Thomas Huth
2023-10-04 17:43 ` Philippe Mathieu-Daudé
2023-10-04 17:47 ` Warner Losh
2023-10-04 17:56 ` Thomas Huth
2023-10-04 18:02 ` Daniel P. Berrangé
2023-10-05 6:56 ` Philippe Mathieu-Daudé [this message]
2023-10-05 5:17 ` Markus Armbruster
2023-10-05 6:52 ` Philippe Mathieu-Daudé
2023-10-05 8:44 ` Daniel P. Berrangé
2023-10-05 8:50 ` Claudio Fontana
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1628d62e-28ac-58f9-9efb-147db95d2b37@linaro.org \
--to=philmd@linaro.org \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).