Re: [Qemu-devel] [PATCH] qemu-ga: guest-shutdown: use only async-signal-safe functions

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon, 14 May 2012 12:41:16 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 05/14/2012 12:03 PM, Luiz Capitulino wrote:
> > On Mon, 14 May 2012 15:01:17 -0300
> > Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > 
> >>>> +
> >>>> +        ret = execle("/sbin/shutdown", "shutdown", shutdown_flag, "+0",
> >>>> +                    "hypervisor initiated shutdown", (char*)NULL, environ);
> >>>
> >>> Where was 'environ' declared?  POSIX says that environ must exist, but
> >>> that it is the one variable where you must declare it yourself rather
> >>> than getting it from a public header.  (For convenience, glibc declares
> >>> environ in <unistd.h> when using _GNU_SOURCE, but when you are asking
> >>> for strict standards namespace compliance, it disappears.)
> >>
> >> I'll declare it then.
> > 
> > -Wredundant-decls doesn't like it:
> > 
> > /home/lcapitulino/work/src/qmp-unstable/qga/commands-posix.c:38:15: warning: redundant redeclaration of ‘environ’ [-Wredundant-decls]
> > /usr/include/unistd.h:546:15: note: previous declaration of ‘environ’ was here
> >   LINK  qemu-ga
> 
> Hmm, gnulib works around that by probing for whether environ was
> declared at configure time, in order to conditionalize whether to output
> a declaration of its own; but obviously we aren't using gnulib.  Maybe,
> since we know that the glibc declaration is guarded by _GNU_SOURCE, we
> could likewise guard our declaration to only occur in the situations
> where glibc is not declaring it?  Or maybe we can factor things into a
> common header used by other qemu files, where the header file itself is
> tagged in such a way to silence gcc warnings about any possible
> duplicate declaration, while still leaving the .c files that use this
> common header clean for use of -Wredundant-decls?

What's the worst case if we leave it as is? The build will brake if environ
ends up not being declared by some arch, right? In that case we could leave it
as is and fix it when it brakes :)