[Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Luiz Capitulino]

Build error found by buildbot:

 qga/commands-posix.c: In function 'qmp_guest_shutdown':
 qga/commands-posix.c:65: error: 'environ' undeclared (first use in this function)
 qga/commands-posix.c:65: error: (Each undeclared identifier is reported only once
 qga/commands-posix.c:65: error: for each function it appears in.)

On F16 environ is declared in <unistd.h>, but that obviously doesn't
happen on all systems.

Tested on OpenBSD 4.9.

PS: I get zillions of -Wno-redundant-decls errors on OpenBSD 4.8 and still
    get other warnnigs if I drop that flag.

 configure            |   19 +++++++++++++++++++
 qga/commands-posix.c |    6 +++++-
 2 files changed, 24 insertions(+), 1 deletion(-)

[Qemu-devel] [PATCH 1/2] configure: check if environ is declared

[Luiz Capitulino]

Some systems may declare environ automatically, others don't. Check for it.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 configure |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/configure b/configure
index b55a792..1f338f8 100755
--- a/configure
+++ b/configure
@@ -2831,6 +2831,21 @@ if compile_prog "" "" ; then
     linux_magic_h=yes
 fi
 
+########################################
+# check if environ is declared
+
+has_environ=no
+cat > $TMPC << EOF
+#include <unistd.h>
+int main(void) {
+    environ = environ;
+    return 0;
+}
+EOF
+if compile_prog "" "" ; then
+    has_environ=yes
+fi
+
 ##########################################
 # End of CC checks
 # After here, no more $cc or $ld runs
@@ -3342,6 +3357,10 @@ if test "$linux_magic_h" = "yes" ; then
   echo "CONFIG_LINUX_MAGIC_H=y" >> $config_host_mak
 fi
 
+if test "$has_environ" = "yes" ; then
+  echo "CONFIG_HAS_ENVIRON=y" >> $config_host_mak
+fi
+
 # USB host support
 case "$usb" in
 linux)
-- 
1.7.9.2.384.g4a92a

[Qemu-devel] [PATCH 2/2] qemu-ga: Fix missing environ declaration

[Luiz Capitulino]

Commit 3674838cd05268954bb6473239cd7f700a79bf0f uses the environ global
variable, but is relying on environ to be declared somewhere else.

This worked for me because on F16 environ is declared in <unistd.h>, but
that doesn't happen in OpenBSD for example, causing a build failure.

This commit fixes the build error by declaring environ if it hasn't
being declared yet.

Also fixes a build warning due to a missing <sys/wait.h> include.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qga/commands-posix.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 7664be1..dab3bf9 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -14,12 +14,17 @@
 #include <glib.h>
 #include <sys/types.h>
 #include <sys/ioctl.h>
+#include <sys/wait.h>
 #include "qga/guest-agent-core.h"
 #include "qga-qmp-commands.h"
 #include "qerror.h"
 #include "qemu-queue.h"
 #include "host-utils.h"
 
+#ifndef CONFIG_HAS_ENVIRON
+extern char **environ;
+#endif
+
 #if defined(__linux__)
 #include <mntent.h>
 #include <linux/fs.h>
@@ -27,7 +32,6 @@
 #include <arpa/inet.h>
 #include <sys/socket.h>
 #include <net/if.h>
-#include <sys/wait.h>
 
 #if defined(__linux__) && defined(FIFREEZE)
 #define CONFIG_FSFREEZE
-- 
1.7.9.2.384.g4a92a

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Michael Roth]

On Wed, May 23, 2012 at 03:48:03PM -0300, Luiz Capitulino wrote:
> Build error found by buildbot:
> 
>  qga/commands-posix.c: In function 'qmp_guest_shutdown':
>  qga/commands-posix.c:65: error: 'environ' undeclared (first use in this function)
>  qga/commands-posix.c:65: error: (Each undeclared identifier is reported only once
>  qga/commands-posix.c:65: error: for each function it appears in.)
> 
> On F16 environ is declared in <unistd.h>, but that obviously doesn't
> happen on all systems.
> 
> Tested on OpenBSD 4.9.
> 
> PS: I get zillions of -Wno-redundant-decls errors on OpenBSD 4.8 and still
>     get other warnnigs if I drop that flag.
> 
>  configure            |   19 +++++++++++++++++++
>  qga/commands-posix.c |    6 +++++-
>  2 files changed, 24 insertions(+), 1 deletion(-)
> 

Thanks, applied to qga branch. Will send a 1.1 pull request for these.

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Luiz Capitulino]

On Wed, 23 May 2012 15:08:43 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On Wed, May 23, 2012 at 03:48:03PM -0300, Luiz Capitulino wrote:
> > Build error found by buildbot:
> > 
> >  qga/commands-posix.c: In function 'qmp_guest_shutdown':
> >  qga/commands-posix.c:65: error: 'environ' undeclared (first use in this function)
> >  qga/commands-posix.c:65: error: (Each undeclared identifier is reported only once
> >  qga/commands-posix.c:65: error: for each function it appears in.)
> > 
> > On F16 environ is declared in <unistd.h>, but that obviously doesn't
> > happen on all systems.
> > 
> > Tested on OpenBSD 4.9.

Just tested on 5.1 too.

> > PS: I get zillions of -Wno-redundant-decls errors on OpenBSD 4.8 and still
> >     get other warnnigs if I drop that flag.

All the -Wno-redundant-decls warnings are still there on 5.1, if I remove that
flag I still get the following ones. Michael, do you want me to fix the qemu-ga
one for 1.1?

  LINK  qemu-ga
/usr/local/lib/libglib-2.0.so.2992.0: warning: vsprintf() is often misused, please use vsnprintf()
/usr/local/lib/libglib-2.0.so.2992.0: warning: stpcpy() is dangerous GNU crap; don't use it
/usr/local/lib/libglib-2.0.so.2992.0: warning: strcpy() is almost always misused, please use strlcpy()
qemu-ga.o(.text+0x358): In function `ga_open_pidfile':
/home/lcapitulino/qmp-unstable/qemu-ga.c:258: warning: sprintf() is often misused, please use snprintf()

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Michael Roth]

On Wed, May 23, 2012 at 06:25:36PM -0300, Luiz Capitulino wrote:
> On Wed, 23 May 2012 15:08:43 -0500
> Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> 
> > On Wed, May 23, 2012 at 03:48:03PM -0300, Luiz Capitulino wrote:
> > > Build error found by buildbot:
> > > 
> > >  qga/commands-posix.c: In function 'qmp_guest_shutdown':
> > >  qga/commands-posix.c:65: error: 'environ' undeclared (first use in this function)
> > >  qga/commands-posix.c:65: error: (Each undeclared identifier is reported only once
> > >  qga/commands-posix.c:65: error: for each function it appears in.)
> > > 
> > > On F16 environ is declared in <unistd.h>, but that obviously doesn't
> > > happen on all systems.
> > > 
> > > Tested on OpenBSD 4.9.
> 
> Just tested on 5.1 too.
> 
> > > PS: I get zillions of -Wno-redundant-decls errors on OpenBSD 4.8 and still
> > >     get other warnnigs if I drop that flag.
> 
> All the -Wno-redundant-decls warnings are still there on 5.1, if I remove that
> flag I still get the following ones. Michael, do you want me to fix the qemu-ga
> one for 1.1?

I'll take a patch, but unless it constitutes a build fix or an actual
overrun I don't think it's worth targetting for 1.1

The sprintf() has been there for a while now so I think it's okay to fix
later since openbsd does seem to have enough coverage to catch qemu-ga
breakages and it hasn't been mentioned yet.

The usage will also be safe for any architecture where pid_t is
defined to be to be 2^64 or less, so we should be good there as well.

> 
>   LINK  qemu-ga
> /usr/local/lib/libglib-2.0.so.2992.0: warning: vsprintf() is often misused, please use vsnprintf()
> /usr/local/lib/libglib-2.0.so.2992.0: warning: stpcpy() is dangerous GNU crap; don't use it
> /usr/local/lib/libglib-2.0.so.2992.0: warning: strcpy() is almost always misused, please use strlcpy()
> qemu-ga.o(.text+0x358): In function `ga_open_pidfile':
> /home/lcapitulino/qmp-unstable/qemu-ga.c:258: warning: sprintf() is often misused, please use snprintf()
> 

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Andreas Färber]

Am 24.05.2012 00:02, schrieb Michael Roth:
> On Wed, May 23, 2012 at 06:25:36PM -0300, Luiz Capitulino wrote:
>> All the -Wno-redundant-decls warnings are still there on 5.1, if I remove that
>> flag I still get the following ones. Michael, do you want me to fix the qemu-ga
>> one for 1.1?
[...]
> The sprintf() has been there for a while now so I think it's okay to fix
> later since openbsd does seem to have enough coverage to catch qemu-ga
> breakages and it hasn't been mentioned yet.

The buildbot did catch it and so did I, unfortunately. The issue here is
rather that
a) your qemu-ga branch is still not covered by the buildbots (Stefan
told me the other day we need to contact Daniel Gollub for that), and
b) Anthony tested all PULLs locally instead of pushing early so that the
existing build bots could've detect any breakage on master before rc3
was set in stone.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Michael Roth]

On Thu, May 24, 2012 at 12:15:18AM +0200, Andreas Färber wrote:
> Am 24.05.2012 00:02, schrieb Michael Roth:
> > On Wed, May 23, 2012 at 06:25:36PM -0300, Luiz Capitulino wrote:
> >> All the -Wno-redundant-decls warnings are still there on 5.1, if I remove that
> >> flag I still get the following ones. Michael, do you want me to fix the qemu-ga
> >> one for 1.1?
> [...]
> > The sprintf() has been there for a while now so I think it's okay to fix
> > later since openbsd does seem to have enough coverage to catch qemu-ga
> > breakages and it hasn't been mentioned yet.
> 
> The buildbot did catch it and so did I, unfortunately. The issue here is
> rather that
> a) your qemu-ga branch is still not covered by the buildbots (Stefan
> told me the other day we need to contact Daniel Gollub for that), and

Hi Daniel,

Would it be possible to get QEMU Guest Agent tree covered by the QEMU build
bots? We've had a recent string of BSD breakages we'd like to start catching
in advance.

The staging branch is available at:

git://github.com/mdroth/qemu.git qga

> b) Anthony tested all PULLs locally instead of pushing early so that the
> existing build bots could've detect any breakage on master before rc3
> was set in stone.
> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
> 

Re: [Qemu-devel] [PATCH 1.1 0/2]: qemu-ga: fix build on openbsd

[Michael Roth]

On Thu, May 24, 2012 at 12:15:18AM +0200, Andreas Färber wrote:
> Am 24.05.2012 00:02, schrieb Michael Roth:
> > On Wed, May 23, 2012 at 06:25:36PM -0300, Luiz Capitulino wrote:
> >> All the -Wno-redundant-decls warnings are still there on 5.1, if I remove that
> >> flag I still get the following ones. Michael, do you want me to fix the qemu-ga
> >> one for 1.1?
> [...]
> > The sprintf() has been there for a while now so I think it's okay to fix
> > later since openbsd does seem to have enough coverage to catch qemu-ga
> > breakages and it hasn't been mentioned yet.
> 
> The buildbot did catch it and so did I, unfortunately. The issue here is

An actual breakage due to the sprintf()? Or are you referring to the
original breakage due to environ not being declared? I agree the latter
needs to be in for 1.1

WRT to the sprintf(), there seem to be other offenders in the tree atm, which
is why I'm not sure it makes sense to target a qemu-ga specific fix for 1.1. It
seems like that kind breakage would've been present for a long time now,
not just for qemu-ga but other targets as well.

For example, here's the latest successful default_openbsd build (just prior to
the environ breakage):

http://buildbot.b1-systems.de/qemu/builders/default_openbsd_4.9/builds/271/steps/compile/logs/stdio

  LINK  alpha-softmmu/qemu-system-alpha
/usr/local/lib/libglib-2.0.so.2992.0: warning: vsprintf() is often misused, please use vsnprintf()
/usr/local/lib/libglib-2.0.so.2992.0: warning: stpcpy() is dangerous GNU crap; don't use it
../libdis/i386-dis.o(.text+0x325): In function `print_displacement':
: warning: strcpy() is almost always misused, please use strlcpy()
/usr/local/lib/libcurl.so.22.0: warning: strcat() is almost always misused, please use strlcat()

translate.o(.text+0x744): In function `cpu_alpha_init':
: warning: sprintf() is often misused, please use snprintf()

I'm not adamantly opposed or anything, though.

> rather that
> a) your qemu-ga branch is still not covered by the buildbots (Stefan
> told me the other day we need to contact Daniel Gollub for that), and
> b) Anthony tested all PULLs locally instead of pushing early so that the
> existing build bots could've detect any breakage on master before rc3
> was set in stone.
> 
> Andreas
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>