[Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Paolo Bonzini]

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(+)

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

Re: [Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Laszlo Ersek]

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
> 

Re: [Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Laszlo Ersek]

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

Re: [Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Laszlo Ersek]

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

Re: [Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Laszlo Ersek]

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

Re: [Qemu-devel] [PATCH] build: disable -Wmissing-braces on older compilers

[Paolo Bonzini]

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