| Message ID | 1500568341-8389-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Headers | show |
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
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
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
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
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
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 >
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
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;
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(-) -- 2.7.4