Re: [PATCH for 7.2] Fix broken configure with -Wunused-parameter

[Stefan Weil]

Am 03.11.22 um 12:48 schrieb Peter Maydell:

> On Wed, 2 Nov 2022 at 20:24, Stefan Weil via <qemu-devel@nongnu.org> wrote:
> > The configure script fails because it tries to compile small C programs
> > with a main function which is declared with arguments argc and argv
> > although those arguments are unused.
> >
> > Running `configure -extra-cflags=-Wunused-parameter` triggers the problem.
> > configure for a native build does abort but shows the error in config.log.
> > A cross build configure for Windows with Debian stable aborts with an
> > error.
> >
> > Avoiding unused arguments fixes this.
> >
> > Signed-off-by: Stefan Weil <sw@weilnetz.de>
> > ---
> >
> > See https://gitlab.com/qemu-project/qemu/-/issues/1295.
> >
> > I noticed the problem because I often compile with -Wextra.
> >
> > Stefan
> >
> >   configure | 7 ++++---
> >   1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/configure b/configure
> > index 4275f5419f..1106c04fea 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1258,6 +1258,7 @@ if test "$stack_protector" != "no"; then
> >     cat > $TMPC << EOF
> >   int main(int argc, char *argv[])
> >   {
> > +    (void)argc;
> I'm not a huge fan of this syntax, and it doesn't match the way
> we deal with "argument is unused" elsewhere in the codebase
> (where we either don't care about it or else use the GCC 'unused'
> attribute hidden behind the glib G_GNUC_UNUSED macro).


Any other variant is also fine for me, for example "using" argc by a 
"return argc == 0;" instead of "return 0;". Would that be better? If 
there is an accepted variant, I can either send a v2 patch, or maybe 
such a trivial change can be applied when merging.


> I am surprised that this didn't get caught by the check in
> do_compiler_werror(), which is supposed to report "this
> configure test passed without -Werror but failed with
> -Werror, so configure is probably buggy.". That's what's
> supposed to catch "your compiler warns on stuff our doesn't
> in the test case programs".
>
> If you're building with --disable-werror then configure
> should be OK anyway. This is probably a good idea if you want
> to build with extra warning arguments in --extra-cflags.
> If it doesn't work right even with --disable-werror that's
> also something we should investigate.


Cross builds for Windows fail with and without --disable-werror. See 
also my bug report https://gitlab.com/qemu-project/qemu/-/issues/1295.

You are right that this is strange and should be investigated, 
especially because native builds don't fail like that.


> >       char arr[64], *p = arr, *c = argv[0];
> >       while (*c) {
> >           *p++ = *c++;
> > @@ -1607,7 +1608,7 @@ fi
> >
> >   if test "$safe_stack" = "yes"; then
> >   cat > $TMPC << EOF
> > -int main(int argc, char *argv[])
> > +int main(void)
> >   {
> >   #if ! __has_feature(safe_stack)
> >   #error SafeStack Disabled
> > @@ -1629,7 +1630,7 @@ EOF
> >     fi
> >   else
> >   cat > $TMPC << EOF
> > -int main(int argc, char *argv[])
> > +int main(void)
> >   {
> >   #if defined(__has_feature)
> >   #if __has_feature(safe_stack)
> > @@ -1675,7 +1676,7 @@ static const int Z = 1;
> >   #define TAUT(X) ((X) == Z)
> >   #define PAREN(X, Y) (X == Y)
> >   #define ID(X) (X)
> > -int main(int argc, char *argv[])
> > +int main(void)
> >   {
> >       int x = 0, y = 0;
> >       x = ID(x);
> No objection to the cases where we can pass "void", that's
> a neater way to write the test anyway.
>
> thanks
> -- PMM

OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key

OpenPGP_signature
Description: OpenPGP digital signature