| Message ID | 20171020101219.2378-1-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/20/17 12:12, Paolo Bonzini wrote: > GCC 4.9 and newer stopped warning for missing braces around the > "universal" C zero initializer {0}. One such initializer sneaked s/sneaked/snuck/ Hmmm, no, wait, both forms are valid! https://en.wiktionary.org/wiki/sneaked https://en.wiktionary.org/wiki/snuck The irregular form snuck originated by analogy with struck for the past of strike. Snuck was originally limited to a few dialects, but is now very widespread (especially in American English) and is recognized by most dictionaries. The word is now one of the best examples of irregularization of a regular verb, along with dove. > into scsi/qemu-pr-helper.c and is breaking the build with such > older GCC versions. > > Detect the lack of support for the idiom, and disable the warning > in that case. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > Of course it's always possible to use "memset", but {0} > is neater in my opinion. Strongly agree. > > configure | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git a/configure b/configure > index 7766e74125..0b2d595c6e 100755 > --- a/configure > +++ b/configure > @@ -1651,6 +1651,19 @@ EOF > fi > fi > > +# Disable -Wmissing-braces on older compilers that warn even for > +# the "universal" C zero initializer {0}. > +cat > $TMPC << EOF > +struct { > + int a[2]; > +} x = {0}; > +EOF > +if compile_object "-Werror" "" ; then > + : > +else Is this an established idiom for the configure script, in place of: if ! compile_object "-Werror" "" ; then ? Looks good to me otherwise. Thanks! Laszlo > + QEMU_CFLAGS="$QEMU_CFLAGS -Wno-missing-braces" > +fi > + > # Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC and > # large functions that use global variables. The bug is in all releases of > # GCC, but it became particularly acute in 4.6.x and 4.7.x. It is fixed in >
On 20/10/2017 12:27, Laszlo Ersek wrote: >> +if compile_object "-Werror" "" ; then >> + : >> +else > Is this an established idiom for the configure script, in place of: > > if ! compile_object "-Werror" "" ; then > > ? > > Looks good to me otherwise. I tend not use "if !". In general that's just me, but in this case I think it's useful to point out that a successful compile does nothing; generally it's the successful compile that adjusts command line arguments. Paolo
On 10/20/17 12:37, Paolo Bonzini wrote: > On 20/10/2017 12:27, Laszlo Ersek wrote: >>> +if compile_object "-Werror" "" ; then >>> + : >>> +else >> Is this an established idiom for the configure script, in place of: >> >> if ! compile_object "-Werror" "" ; then >> >> ? >> >> Looks good to me otherwise. > > I tend not use "if !". In general that's just me, but in this case I > think it's useful to point out that a successful compile does nothing; > generally it's the successful compile that adjusts command line arguments. Thanks for the explanation. Reviewed-by: Laszlo Ersek <lersek@redhat.com> Cheers Laszlo
On 20 October 2017 at 11:12, Paolo Bonzini <pbonzini@redhat.com> wrote: > GCC 4.9 and newer stopped warning for missing braces around the > "universal" C zero initializer {0}. One such initializer sneaked > into scsi/qemu-pr-helper.c and is breaking the build with such > older GCC versions. > > Detect the lack of support for the idiom, and disable the warning > in that case. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> AFAIK "{}" will work and not be warned about anywhere (we use it extensively already) so you don't need to fall back to memset... thanks -- PMM
On 20/10/2017 12:48, Peter Maydell wrote: > On 20 October 2017 at 11:12, Paolo Bonzini <pbonzini@redhat.com> wrote: >> GCC 4.9 and newer stopped warning for missing braces around the >> "universal" C zero initializer {0}. One such initializer sneaked >> into scsi/qemu-pr-helper.c and is breaking the build with such >> older GCC versions. >> >> Detect the lack of support for the idiom, and disable the warning >> in that case. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > AFAIK "{}" will work and not be warned about anywhere (we > use it extensively already) so you don't need to fall back to > memset... Doh, of course that will work in scsi/qemu-pr-helper.c. In general you can use {0} but not {} to initialize a scalar, like IDontKnowIfItsAnArrayOrPointer x = {}; //might fail IDontKnowIfItsAnArrayOrPointer x = {0}; //always works I'm not sure if that matters. Paolo
On 10/20/17 13:02, Paolo Bonzini wrote: > On 20/10/2017 12:48, Peter Maydell wrote: >> On 20 October 2017 at 11:12, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> GCC 4.9 and newer stopped warning for missing braces around the >>> "universal" C zero initializer {0}. One such initializer sneaked >>> into scsi/qemu-pr-helper.c and is breaking the build with such >>> older GCC versions. >>> >>> Detect the lack of support for the idiom, and disable the warning >>> in that case. >>> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> >> AFAIK "{}" will work and not be warned about anywhere (we >> use it extensively already) so you don't need to fall back to >> memset... > > Doh, of course that will work in scsi/qemu-pr-helper.c. In general you > can use {0} but not {} to initialize a scalar, like > > IDontKnowIfItsAnArrayOrPointer x = {}; //might fail > IDontKnowIfItsAnArrayOrPointer x = {0}; //always works > > I'm not sure if that matters. My remark below might matter even less, but: I'd find it regrettable if we suppressed a wrong gcc warning about a valid C construct by replacing the construct with a GNU-ism that is *not* standard C. :/ Laszlo
On 20/10/2017 17:08, Laszlo Ersek wrote: > On 10/20/17 13:02, Paolo Bonzini wrote: >> On 20/10/2017 12:48, Peter Maydell wrote: >>> On 20 October 2017 at 11:12, Paolo Bonzini <pbonzini@redhat.com> wrote: >>>> GCC 4.9 and newer stopped warning for missing braces around the >>>> "universal" C zero initializer {0}. One such initializer sneaked >>>> into scsi/qemu-pr-helper.c and is breaking the build with such >>>> older GCC versions. >>>> >>>> Detect the lack of support for the idiom, and disable the warning >>>> in that case. >>>> >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>> >>> AFAIK "{}" will work and not be warned about anywhere (we >>> use it extensively already) so you don't need to fall back to >>> memset... >> >> Doh, of course that will work in scsi/qemu-pr-helper.c. In general you >> can use {0} but not {} to initialize a scalar, like >> >> IDontKnowIfItsAnArrayOrPointer x = {}; //might fail >> IDontKnowIfItsAnArrayOrPointer x = {0}; //always works >> >> I'm not sure if that matters. > > My remark below might matter even less, but: > > I'd find it regrettable if we suppressed a wrong gcc warning about a > valid C construct by replacing the construct with a GNU-ism that is > *not* standard C. :/ Is {} to initialize structs a GNUism? (Initializing a scalar with {} is a hard error with GCC). Paolo
On 10/20/17 17:09, Paolo Bonzini wrote: > On 20/10/2017 17:08, Laszlo Ersek wrote: >> On 10/20/17 13:02, Paolo Bonzini wrote: >>> On 20/10/2017 12:48, Peter Maydell wrote: >>>> On 20 October 2017 at 11:12, Paolo Bonzini <pbonzini@redhat.com> >>>> wrote: >>>>> GCC 4.9 and newer stopped warning for missing braces around the >>>>> "universal" C zero initializer {0}. One such initializer sneaked >>>>> into scsi/qemu-pr-helper.c and is breaking the build with such >>>>> older GCC versions. >>>>> >>>>> Detect the lack of support for the idiom, and disable the warning >>>>> in that case. >>>>> >>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >>>> >>>> AFAIK "{}" will work and not be warned about anywhere (we >>>> use it extensively already) so you don't need to fall back to >>>> memset... >>> >>> Doh, of course that will work in scsi/qemu-pr-helper.c. In general >>> you can use {0} but not {} to initialize a scalar, like >>> >>> IDontKnowIfItsAnArrayOrPointer x = {}; //might fail >>> IDontKnowIfItsAnArrayOrPointer x = {0}; //always works >>> >>> I'm not sure if that matters. >> >> My remark below might matter even less, but: >> >> I'd find it regrettable if we suppressed a wrong gcc warning about a >> valid C construct by replacing the construct with a GNU-ism that is >> *not* standard C. :/ > > Is {} to initialize structs a GNUism? It is; > struct { > int a, b; > } x = {}; $ gcc -fsyntax-only -std=c99 -pedantic -Wall -Wextra x.c > x.c:3:7: warning: ISO C forbids empty initializer braces [-Wpedantic] > } x = {}; > ^ > x.c:3:1: warning: missing initializer for field 'a' of 'struct <anonymous>' [-Wmissing-field-initializers] > } x = {}; > ^ > x.c:2:7: note: 'a' declared here > int a, b; > ^ If you replace "{}" with "{ 0 }", the same command line stays quiet. (I used: gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16).) > (Initializing a scalar with {} is a hard error with GCC). Thanks, Laszlo
On 20/10/2017 17:32, Laszlo Ersek wrote: > On 10/20/17 17:09, Paolo Bonzini wrote: >> On 20/10/2017 17:08, Laszlo Ersek wrote: >>> My remark below might matter even less, but: >>> >>> I'd find it regrettable if we suppressed a wrong gcc warning about a >>> valid C construct by replacing the construct with a GNU-ism that is >>> *not* standard C. :/ >> >> Is {} to initialize structs a GNUism? > > It is; > >> struct { >> int a, b; >> } x = {}; > > $ gcc -fsyntax-only -std=c99 -pedantic -Wall -Wextra x.c > >> x.c:3:7: warning: ISO C forbids empty initializer braces [-Wpedantic] >> } x = {}; >> ^ >> x.c:3:1: warning: missing initializer for field 'a' of 'struct <anonymous>' [-Wmissing-field-initializers] >> } x = {}; >> ^ >> x.c:2:7: note: 'a' declared here >> int a, b; >> ^ > > If you replace "{}" with "{ 0 }", the same command line stays quiet. > > (I used: gcc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-16).) Oh. So there _was_ some logic to my original configure patch. Currently we have 82 occurrences of empty initializers and 92 occurrences of {0}, so I think it makes sense to go ahead with it. On the other hand, without a more precise option than -Wpedantic, empty initializers are going to creep in again. Paolo
diff --git a/configure b/configure index 7766e74125..0b2d595c6e 100755 --- a/configure +++ b/configure @@ -1651,6 +1651,19 @@ EOF fi fi +# Disable -Wmissing-braces on older compilers that warn even for +# the "universal" C zero initializer {0}. +cat > $TMPC << EOF +struct { + int a[2]; +} x = {0}; +EOF +if compile_object "-Werror" "" ; then + : +else + QEMU_CFLAGS="$QEMU_CFLAGS -Wno-missing-braces" +fi + # Workaround for http://gcc.gnu.org/PR55489. Happens with -fPIE/-fPIC and # large functions that use global variables. The bug is in all releases of # GCC, but it became particularly acute in 4.6.x and 4.7.x. It is fixed in
GCC 4.9 and newer stopped warning for missing braces around the "universal" C zero initializer {0}. One such initializer sneaked into scsi/qemu-pr-helper.c and is breaking the build with such older GCC versions. Detect the lack of support for the idiom, and disable the warning in that case. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- Of course it's always possible to use "memset", but {0} is neater in my opinion. configure | 13 +++++++++++++ 1 file changed, 13 insertions(+)