[Qemu-devel] [PATCH for-2.10] util/oslib-posix.c: Avoid warning on NetBSD

[Qemu-devel] [PATCH for-2.10] util/oslib-posix.c: Avoid warning on NetBSD

[Peter Maydell]

On NetBSD the compiler warns:
util/oslib-posix.c: In function 'sigaction_invoke':
util/oslib-posix.c:589:5: warning: missing braces around initializer [-Wmissing-braces]
     siginfo_t si = { 0 };
     ^
util/oslib-posix.c:589:5: warning: (near initialization for 'si.si_pad') [-Wmissing-braces]

because on this platform siginfo_t is defined as
  typedef union siginfo {
          char    si_pad[128];    /* Total size; for future expansion */
          struct _ksiginfo _info;
  } siginfo_t;

Avoid this warning by initializing the struct with {} instead;
this is a GCC extension but we use it all over the codebase already.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 util/oslib-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index b2dea48..cacf0ef 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -586,7 +586,7 @@ void qemu_free_stack(void *stack, size_t sz)
 void sigaction_invoke(struct sigaction *action,
                       struct qemu_signalfd_siginfo *info)
 {
-    siginfo_t si = { 0 };
+    siginfo_t si = {};
     si.si_signo = info->ssi_signo;
     si.si_errno = info->ssi_errno;
     si.si_code = info->ssi_code;
-- 
2.7.4

Re: [Qemu-devel] [PATCH for-2.10] util/oslib-posix.c: Avoid warning on NetBSD

[Eric Blake]

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

On 07/20/2017 11:32 AM, Peter Maydell wrote:
> On NetBSD the compiler warns:
> util/oslib-posix.c: In function 'sigaction_invoke':
> util/oslib-posix.c:589:5: warning: missing braces around initializer [-Wmissing-braces]
>      siginfo_t si = { 0 };
>      ^

Uggh. That is a broken compiler.  C99 declares that 'anything = {0}' is
supposed to be a valid way to zero-initialize anything.

> util/oslib-posix.c:589:5: warning: (near initialization for 'si.si_pad') [-Wmissing-braces]
> 
> because on this platform siginfo_t is defined as
>   typedef union siginfo {
>           char    si_pad[128];    /* Total size; for future expansion */
>           struct _ksiginfo _info;
>   } siginfo_t;
> 
> Avoid this warning by initializing the struct with {} instead;
> this is a GCC extension but we use it all over the codebase already.

Well, I'm glad that works to shut up the broken compiler.

> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  util/oslib-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index b2dea48..cacf0ef 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -586,7 +586,7 @@ void qemu_free_stack(void *stack, size_t sz)
>  void sigaction_invoke(struct sigaction *action,
>                        struct qemu_signalfd_siginfo *info)
>  {
> -    siginfo_t si = { 0 };
> +    siginfo_t si = {};
>      si.si_signo = info->ssi_signo;
>      si.si_errno = info->ssi_errno;
>      si.si_code = info->ssi_code;
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH for-2.10] util/oslib-posix.c: Avoid warning on NetBSD

[Peter Maydell]

On 20 July 2017 at 19:26, Eric Blake <eblake@redhat.com> wrote:
> On 07/20/2017 11:32 AM, Peter Maydell wrote:
>> On NetBSD the compiler warns:
>> util/oslib-posix.c: In function 'sigaction_invoke':
>> util/oslib-posix.c:589:5: warning: missing braces around initializer [-Wmissing-braces]
>>      siginfo_t si = { 0 };
>>      ^
>
> Uggh. That is a broken compiler.  C99 declares that 'anything = {0}' is
> supposed to be a valid way to zero-initialize anything.

Looks like maybe it was https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.10] util/oslib-posix.c: Avoid warning on NetBSD

[Eric Blake]

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

On 07/20/2017 03:53 PM, Peter Maydell wrote:
> On 20 July 2017 at 19:26, Eric Blake <eblake@redhat.com> wrote:
>> On 07/20/2017 11:32 AM, Peter Maydell wrote:
>>> On NetBSD the compiler warns:
>>> util/oslib-posix.c: In function 'sigaction_invoke':
>>> util/oslib-posix.c:589:5: warning: missing braces around initializer [-Wmissing-braces]
>>>      siginfo_t si = { 0 };
>>>      ^
>>
>> Uggh. That is a broken compiler.  C99 declares that 'anything = {0}' is
>> supposed to be a valid way to zero-initialize anything.
> 
> Looks like maybe it was https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119

Would it be better to add a configure check for broken -Wmissing-braces
(where we would then add -Wno-missing-braces to the (outdated) NetBSD
compiler)?

But then again, we already rely on gcc/clang's extension of foo={}
(which is valid for C++, what a shame that the two languages picked
different universal-zero initializers), so it's probably less churn to
just fix the few outliers to also rely on the extension than it is to
bloat configure just to permanently work around the broken compiler.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH for-2.10] util/oslib-posix.c: Avoid warning on NetBSD

[Peter Maydell]

On 20 July 2017 at 22:10, Eric Blake <eblake@redhat.com> wrote:
> On 07/20/2017 03:53 PM, Peter Maydell wrote:
>> On 20 July 2017 at 19:26, Eric Blake <eblake@redhat.com> wrote:
>>> On 07/20/2017 11:32 AM, Peter Maydell wrote:
>>>> On NetBSD the compiler warns:
>>>> util/oslib-posix.c: In function 'sigaction_invoke':
>>>> util/oslib-posix.c:589:5: warning: missing braces around initializer [-Wmissing-braces]
>>>>      siginfo_t si = { 0 };
>>>>      ^
>>>
>>> Uggh. That is a broken compiler.  C99 declares that 'anything = {0}' is
>>> supposed to be a valid way to zero-initialize anything.
>>
>> Looks like maybe it was https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
>
> Would it be better to add a configure check for broken -Wmissing-braces
> (where we would then add -Wno-missing-braces to the (outdated) NetBSD
> compiler)?

There's also a pointer to a clang bug which is not marked as fixed,
and even in gcc it's down as not fixed in 4.8 or 4.9 if I'm reading
the bug comments right.

> But then again, we already rely on gcc/clang's extension of foo={}
> (which is valid for C++, what a shame that the two languages picked
> different universal-zero initializers), so it's probably less churn to
> just fix the few outliers to also rely on the extension than it is to
> bloat configure just to permanently work around the broken compiler.

This is AFAICT the only instance that needs fixing, because the
compiler only warns if the first element in the struct isn't
a simple data type that can be initialized with '= 0'. If NetBSD
had happened to use a similar definition to Linux (which happens
to start with "int si_signo;") we wouldn't have had to change this
initializer. (The fact that so many still-prevalent gcc versions
behave like this is probably why the only case the NetBSD compile
found is one that is an initializer of a system-header-defined
struct where NetBSD has taken a somewhat odd approach.)

(Personally if I were the C standards folks I'd fix it by
blessing the gcc-extension that makes "{}" a valid zero
initializer and bringing it into line with C++.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.10] util/oslib-posix.c: Avoid warning on NetBSD

[Peter Maydell]

On 20 July 2017 at 19:26, Eric Blake <eblake@redhat.com> wrote:
> On 07/20/2017 11:32 AM, Peter Maydell wrote:
>> On NetBSD the compiler warns:
>> util/oslib-posix.c: In function 'sigaction_invoke':
>> util/oslib-posix.c:589:5: warning: missing braces around initializer [-Wmissing-braces]
>>      siginfo_t si = { 0 };
>>      ^
>
> Uggh. That is a broken compiler.  C99 declares that 'anything = {0}' is
> supposed to be a valid way to zero-initialize anything.
>
>> util/oslib-posix.c:589:5: warning: (near initialization for 'si.si_pad') [-Wmissing-braces]
>>
>> because on this platform siginfo_t is defined as
>>   typedef union siginfo {
>>           char    si_pad[128];    /* Total size; for future expansion */
>>           struct _ksiginfo _info;
>>   } siginfo_t;
>>
>> Avoid this warning by initializing the struct with {} instead;
>> this is a GCC extension but we use it all over the codebase already.
>
> Well, I'm glad that works to shut up the broken compiler.
>
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  util/oslib-posix.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks; applied to master.

-- PMM

Re: [Qemu-devel] [Qemu-trivial] [PATCH for-2.10] util/oslib-posix.c: Avoid warning on NetBSD

[Kamil Rytarowski]

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

On 21.07.2017 12:22, Peter Maydell wrote:
> On 20 July 2017 at 19:26, Eric Blake <eblake@redhat.com> wrote:
>> On 07/20/2017 11:32 AM, Peter Maydell wrote:
>>> On NetBSD the compiler warns:
>>> util/oslib-posix.c: In function 'sigaction_invoke':
>>> util/oslib-posix.c:589:5: warning: missing braces around initializer [-Wmissing-braces]
>>>      siginfo_t si = { 0 };
>>>      ^
>>
>> Uggh. That is a broken compiler.  C99 declares that 'anything = {0}' is
>> supposed to be a valid way to zero-initialize anything.
>>

Not necessarily broken compiler. According to strict C standard we need
to initialize it in this case with "{{0}}"... which is ugly. A portable
option is to go for memset(3). {} is also fine and I hope it will be
added to C standard.

>>> util/oslib-posix.c:589:5: warning: (near initialization for 'si.si_pad') [-Wmissing-braces]
>>>
>>> because on this platform siginfo_t is defined as
>>>   typedef union siginfo {
>>>           char    si_pad[128];    /* Total size; for future expansion */
>>>           struct _ksiginfo _info;
>>>   } siginfo_t;
>>>
>>> Avoid this warning by initializing the struct with {} instead;
>>> this is a GCC extension but we use it all over the codebase already.
>>
>> Well, I'm glad that works to shut up the broken compiler.
>>
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>  util/oslib-posix.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
> 
> Thanks; applied to master.
> 

Thank you!

> -- PMM
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [Qemu-trivial] [PATCH for-2.10] util/oslib-posix.c: Avoid warning on NetBSD

[Eric Blake]

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

On 07/21/2017 07:18 AM, Kamil Rytarowski wrote:
> On 21.07.2017 12:22, Peter Maydell wrote:
>> On 20 July 2017 at 19:26, Eric Blake <eblake@redhat.com> wrote:
>>> On 07/20/2017 11:32 AM, Peter Maydell wrote:
>>>> On NetBSD the compiler warns:
>>>> util/oslib-posix.c: In function 'sigaction_invoke':
>>>> util/oslib-posix.c:589:5: warning: missing braces around initializer [-Wmissing-braces]
>>>>      siginfo_t si = { 0 };
>>>>      ^
>>>
>>> Uggh. That is a broken compiler.  C99 declares that 'anything = {0}' is
>>> supposed to be a valid way to zero-initialize anything.
>>>
> 
> Not necessarily broken compiler. According to strict C standard we need
> to initialize it in this case with "{{0}}"... which is ugly.

No, the C standard SPECIFICALLY allows for missing {}, so that {0} is
the universal zero-initializer.

  typedef union siginfo {
          char    si_pad[128];    /* Total size; for future expansion */
          struct _ksiginfo _info;
  } siginfo_t;

C99 6.7.8 Initialization paragraph 20

> If the aggregate or union contains elements or members that are aggregates or unions,
> these rules apply recursively to the subaggregates or contained unions. If the initializer of
> a subaggregate or contained union begins with a left brace, the initializers enclosed by
> that brace and its matching right brace initialize the elements or members of the
> subaggregate or the contained union. Otherwise, only enough initializers from the list are
> taken to account for the elements or members of the subaggregate or the first member of
> the contained union; any remaining initializers are left to initialize the next element or
> member of the aggregate of which the current subaggregate or contained union is a part.

and also gives an example of missing braces being valid, in paragraph 28:

> EXAMPLE 5
> The declaration
> struct { int a[3], b; } w[] = { { 1 }, 2 };
> is a definition with an inconsistently bracketed initialization. It defines an array with two element
> structures: w[0].a[0] is 1 and w[1].a[0] is 2; all the other elements are zero.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]