[Qemu-devel] [PATCH 00/11] configure: Fix -Werror issues

[Qemu-devel] [PATCH 00/11] configure: Fix -Werror issues

[Peter Maydell]

This patch series:
 1. turns off -Werror for configure tests
 2. fixes a large pile of warnings in various configure tests
 3. turns on -Werror for configure tests again, but in a way that means
    that errors mean configure stops so the warnings are as obvious
    to developers as they would be for a -Werror compile of the main code

The "turn off -Werror" patch has already been sent to the list;
I've included it here as it is a dependency, but it should probably
be committed ASAP even if the rest of this series needs review and
updating. I've also put all of Stefan's recent patches in since without
them configure won't run once -Werror is reenabled.

NB: this works for me, but it's possible there are some still lurking
warnings in a few compilation tests that are themselves protected by
tests for some library/host I don't have. They should be easy to fix
as they turn up, though, and the configure failure message is clear
about how to work around in the meantime.

Peter Maydell (7):
  configure: Don't run configure tests with -Werror enabled
  configure: -march=i486 belongs in QEMU_CFLAGS, not CFLAGS
  configure: Fix compile warning in PNG test
  configure: Fix warnings in VDE library probe
  configure: Fix compile warning in utimensat/futimens test
  configure: -I\$(SRC_PATH) goes in QEMU_INCLUDES not QEMU_CFLAGS
  configure: Check for -Werror causing failures when compiling tests

Stefan Weil (4):
  configure: Fix build with ALSA audio driver
  configure: Fix build with capabilities
  configure: Replace bash code by standard shell code
  configure: Fix errors in test for__sync_fetch_and_and

 configure |   73 +++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 57 insertions(+), 16 deletions(-)

-- 
1.7.5.4

[Qemu-devel] [PATCH 01/11] configure: Don't run configure tests with -Werror enabled

[Peter Maydell]

Don't run configure tests with -Werror in the compiler flags. The idea
of -Werror is that it makes problems very obvious to developers, so
they get fixed quickly. However, when running configure tests, failures
due to -Werror are far from obvious -- they simply result in the test
quietly failing when it should have passed. Not using -Werror is in
line with recommended practice in the Autoconf world.

This commit is essentially backing out the changes in commit 417c9d72.
Instead we fix the problem that commit was trying to address in a
different way: we add -Werror only for the test of the nss headers,
with a comment that this is specifically intended to detect a bug
in some releases of nss.

We also have to clean up a bug in the smartcard test where it was
trying to include smartcard_cflags in the test compile flags: this
would always result in a failure with -Werror, because they include
an escaped "$(SRC_PATH)" which is only valid when used in the final
makefile.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Stefan Weil <sw@weilnetz.de>
---
 configure |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 0a3896e..383fa3d 100755
--- a/configure
+++ b/configure
@@ -1156,9 +1156,10 @@ gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
 gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags"
 gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
 gcc_flags="-fstack-protector-all -Wendif-labels $gcc_flags"
-if test "$werror" = "yes" ; then
-    gcc_flags="-Werror $gcc_flags"
-fi
+# Note that we do not add -Werror to gcc_flags here, because that would
+# enable it for all configure tests. If a configure test failed due
+# to -Werror this would just silently disable some features,
+# so it's too error prone.
 cat > $TMPC << EOF
 int main(void) { return 0; }
 EOF
@@ -2656,8 +2657,16 @@ EOF
         smartcard_cflags="-I\$(SRC_PATH)/libcacard"
         libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
         libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
+        test_cflags="$libcacard_cflags"
+        # The header files in nss < 3.13.3 have a bug which causes them to
+        # emit a warning. If we're going to compile QEMU with -Werror, then
+        # test that the headers don't have this bug. Otherwise we would pass
+        # the configure test but fail to compile QEMU later.
+        if test "$werror" = "yes"; then
+            test_cflags="-Werror $test_cflags"
+        fi
         if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 && \
-          compile_prog "$smartcard_cflags $libcacard_cflags" "$libcacard_libs"; then
+          compile_prog "$test_cflags" "$libcacard_libs"; then
             smartcard_nss="yes"
             QEMU_CFLAGS="$QEMU_CFLAGS $smartcard_cflags $libcacard_cflags"
             libs_softmmu="$libcacard_libs $libs_softmmu"
@@ -2903,6 +2912,11 @@ if test -z "$zero_malloc" ; then
     fi
 fi
 
+# Now we've finished running tests it's OK to add -Werror to the compiler flags
+if test "$werror" = "yes"; then
+    QEMU_CFLAGS="-Werror $QEMU_CFLAGS"
+fi
+
 if test "$solaris" = "no" ; then
     if $ld --version 2>/dev/null | grep "GNU ld" >/dev/null 2>/dev/null ; then
         LDFLAGS="-Wl,--warn-common $LDFLAGS"
-- 
1.7.5.4

[Qemu-devel] [PATCH 02/11] configure: Fix build with ALSA audio driver

[Peter Maydell]

From: Stefan Weil <sw@weilnetz.de>

Since commit 417c9d72d48275d19c60861896efd4962d21aca2,
all configure tests normally run with -Werror.

Some of these tests now fail because they raised a compiler warning.

Here a build breakage for ALSA (configure --audio-drv-list=alsa) is fixed.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 383fa3d..63156a7 100755
--- a/configure
+++ b/configure
@@ -1889,7 +1889,7 @@ for drv in $audio_drv_list; do
     case $drv in
     alsa)
     audio_drv_probe $drv alsa/asoundlib.h -lasound \
-        "snd_pcm_t **handle; return snd_pcm_close(*handle);"
+        "return snd_pcm_close((snd_pcm_t *)0);"
     libs_softmmu="-lasound $libs_softmmu"
     ;;
 
-- 
1.7.5.4

[Qemu-devel] [PATCH 03/11] configure: Fix build with capabilities

[Peter Maydell]

From: Stefan Weil <sw@weilnetz.de>

Since commit 417c9d72d48275d19c60861896efd4962d21aca2 all configure tests
normally run with -Werror. Some of these tests now fail because they
raised a compiler warning.

This patch fixes support for capabilities.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 63156a7..feadbe7 100755
--- a/configure
+++ b/configure
@@ -2083,7 +2083,7 @@ if test "$cap" != "no" ; then
   cat > $TMPC <<EOF
 #include <stdio.h>
 #include <sys/capability.h>
-int main(void) { cap_t caps; caps = cap_init(); }
+int main(void) { cap_t caps; caps = cap_init(); return caps != NULL; }
 EOF
   if compile_prog "" "-lcap" ; then
     cap=yes
-- 
1.7.5.4

[Qemu-devel] [PATCH 04/11] configure: Replace bash code by standard shell code

[Peter Maydell]

From: Stefan Weil <sw@weilnetz.de>

"+=" does not work with dash and other simple /bin/sh implementations.

The new code prepends the flag while the old code either did not work
(it continued after an error message which typically was not read) or
appended the flag. That difference should not matter here.

Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index feadbe7..f54415d 100755
--- a/configure
+++ b/configure
@@ -2810,7 +2810,7 @@ int main(int argc, char **argv)
 }
 EOF
   if ! compile_prog "" "" ; then
-    CFLAGS+="-march=i486"
+    CFLAGS="-march=i486 $CFLAGS"
   fi
 fi
 
-- 
1.7.5.4

[Qemu-devel] [PATCH 05/11] configure: -march=i486 belongs in QEMU_CFLAGS, not CFLAGS

[Peter Maydell]

The distinction between QEMU_CFLAGS and CFLAGS is that the
former is for flags without which QEMU can't compile, whereas
the latter is for flags like "-g -O2" which the user can
safely override. "-march=i486" is in the former category, and
so belongs in QEMU_CFLAGS.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index f54415d..3de4ea7 100755
--- a/configure
+++ b/configure
@@ -2810,7 +2810,7 @@ int main(int argc, char **argv)
 }
 EOF
   if ! compile_prog "" "" ; then
-    CFLAGS="-march=i486 $CFLAGS"
+    QEMU_CFLAGS="-march=i486 $QEMU_CFLAGS"
   fi
 fi
 
-- 
1.7.5.4

[Qemu-devel] [PATCH 06/11] configure: Fix errors in test for__sync_fetch_and_and

[Peter Maydell]

From: Stefan Weil <sw@weilnetz.de>

The old test code raises two compiler warnings which are errors since
commit 417c9d72d48275d19c60861896efd4962d21aca2.

These errors could result in compilations with compiler flag
-march486 (so all nice features of newer processors got lost).

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 3de4ea7..aced52e 100755
--- a/configure
+++ b/configure
@@ -2797,7 +2797,7 @@ fi
 # specification is necessary
 if test "$vhost_net" = "yes" && test "$cpu" = "i386"; then
   cat > $TMPC << EOF
-int sfaa(unsigned *ptr)
+static int sfaa(int *ptr)
 {
   return __sync_fetch_and_and(ptr, 0);
 }
-- 
1.7.5.4

[Qemu-devel] [PATCH 07/11] configure: Fix compile warning in PNG test

[Peter Maydell]

Fix compile warning (variable 'png_ptr' set but not used) in the
PNG detection test code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index aced52e..784325a 100755
--- a/configure
+++ b/configure
@@ -1727,7 +1727,7 @@ cat > $TMPC <<EOF
 int main(void) {
     png_structp png_ptr;
     png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
-    return 0;
+    return png_ptr != 0;
 }
 EOF
   if $pkg_config libpng --modversion >/dev/null 2>&1; then
-- 
1.7.5.4

[Qemu-devel] [PATCH 08/11] configure: Fix warnings in VDE library probe

[Peter Maydell]

Fix compile warnings in the VDE library probe ("passing argument 1 of
'vde_open_real' discards 'const' qualifier from pointer target type",
ditto argument 2).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 784325a..9c2a84d 100755
--- a/configure
+++ b/configure
@@ -1820,7 +1820,8 @@ if test "$vde" != "no" ; then
 int main(void)
 {
     struct vde_open_args a = {0, 0, 0};
-    vde_open("", "", &a);
+    char s[] = "";
+    vde_open(s, s, &a);
     return 0;
 }
 EOF
-- 
1.7.5.4

[Qemu-devel] [PATCH 09/11] configure: Fix compile warning in utimensat/futimens test

[Peter Maydell]

Fix compile warning in the utimensat/futimens test ("implicit
declaration of function 'utimensat'", ditto futimens) by
adding a missing include.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 9c2a84d..638e486 100755
--- a/configure
+++ b/configure
@@ -2341,6 +2341,7 @@ cat > $TMPC << EOF
 #define _ATFILE_SOURCE
 #include <stddef.h>
 #include <fcntl.h>
+#include <sys/stat.h>
 
 int main(void)
 {
-- 
1.7.5.4

[Qemu-devel] [PATCH 10/11] configure: -I\$(SRC_PATH) goes in QEMU_INCLUDES not QEMU_CFLAGS

[Peter Maydell]

If the smartcard configure check passes, add '-I\$(SRC_PATH)/libcacard'
to QEMU_INCLUDES, not QEMU_CFLAGS. Otherwise the unexpanded SRC_PATH
will cause a warning in every following configure test.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 638e486..8140464 100755
--- a/configure
+++ b/configure
@@ -2656,7 +2656,7 @@ if test "$smartcard" != "no" ; then
 #include <pk11pub.h>
 int main(void) { PK11_FreeSlot(0); return 0; }
 EOF
-        smartcard_cflags="-I\$(SRC_PATH)/libcacard"
+        smartcard_includes="-I\$(SRC_PATH)/libcacard"
         libcacard_libs="$($pkg_config --libs nss 2>/dev/null) $glib_libs"
         libcacard_cflags="$($pkg_config --cflags nss 2>/dev/null) $glib_cflags"
         test_cflags="$libcacard_cflags"
@@ -2670,7 +2670,8 @@ EOF
         if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 && \
           compile_prog "$test_cflags" "$libcacard_libs"; then
             smartcard_nss="yes"
-            QEMU_CFLAGS="$QEMU_CFLAGS $smartcard_cflags $libcacard_cflags"
+            QEMU_CFLAGS="$QEMU_CFLAGS $libcacard_cflags"
+            QEMU_INCLUDES="$QEMU_INCLUDES $smartcard_includes"
             libs_softmmu="$libcacard_libs $libs_softmmu"
         else
             if test "$smartcard_nss" = "yes"; then
-- 
1.7.5.4

[Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Peter Maydell]

Add support for checking whether test case code can compile without
warnings, by recompiling each successful test with -Werror. If the
-Werror version doesn't pass, we bail out. This gives us the same
level of visibility of warnings in test code as --enable-werror
provides for the main compile.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 configure |   32 ++++++++++++++++++++++++++++----
 1 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 8140464..1939bdb 100755
--- a/configure
+++ b/configure
@@ -27,16 +27,40 @@ printf " '%s'" "$0" "$@" >> config.log
 echo >> config.log
 echo "#" >> config.log
 
+do_cc() {
+    # Run the compiler, capturing its output to the log.
+    echo $cc "$@" >> config.log
+    $cc "$@" >> config.log 2>&1 || return $?
+    # Test passed. If this is an --enable-werror build, rerun
+    # the test with -Werror and bail out if it fails. This
+    # makes warning-generating-errors in configure test code
+    # obvious to developers.
+    if test "$werror" != "yes"; then
+        return 0
+    fi
+    # Don't bother rerunning the compile if we were already using -Werror
+    case "$*" in
+        *-Werror*)
+           return 0
+        ;;
+    esac
+    echo $cc -Werror "$@" >> config.log
+    $cc -Werror "$@" >> config.log 2>&1 && return $?
+    echo "ERROR: configure test passed without -Werror but failed with -Werror."
+    echo "This is probably a bug in the configure script. The failing command"
+    echo "will be at the bottom of config.log."
+    echo "You can run configure with --disable-werror to bypass this check."
+    exit 1
+}
+
 compile_object() {
-  echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log
-  $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1
+  do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
 }
 
 compile_prog() {
   local_cflags="$1"
   local_ldflags="$2"
-  echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log
-  $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1
+  do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
 }
 
 # symbolically link $1 to $2.  Portable version of "ln -sf".
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 07/11] configure: Fix compile warning in PNG test

[Stefan Weil]

Am 18.07.2012 16:10, schrieb Peter Maydell:
> Fix compile warning (variable 'png_ptr' set but not used) in the
> PNG detection test code.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   configure |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index aced52e..784325a 100755
> --- a/configure
> +++ b/configure
> @@ -1727,7 +1727,7 @@ cat > $TMPC <<EOF
>   int main(void) {
>       png_structp png_ptr;
>       png_ptr = png_create_write_struct(PNG_LIBPNG_VER_STRING, NULL, NULL, NULL);
> -    return 0;
> +    return png_ptr != 0;
>   }
>   EOF
>     if $pkg_config libpng --modversion >/dev/null 2>&1; then

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [Qemu-devel] [PATCH 09/11] configure: Fix compile warning in utimensat/futimens test

[Stefan Weil]

Am 18.07.2012 16:10, schrieb Peter Maydell:
> Fix compile warning in the utimensat/futimens test ("implicit
> declaration of function 'utimensat'", ditto futimens) by
> adding a missing include.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   configure |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/configure b/configure
> index 9c2a84d..638e486 100755
> --- a/configure
> +++ b/configure
> @@ -2341,6 +2341,7 @@ cat > $TMPC << EOF
>   #define _ATFILE_SOURCE
>   #include <stddef.h>
>   #include <fcntl.h>
> +#include <sys/stat.h>
>   
>   int main(void)
>   {

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Blue Swirl]

On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Add support for checking whether test case code can compile without
> warnings, by recompiling each successful test with -Werror. If the
> -Werror version doesn't pass, we bail out. This gives us the same
> level of visibility of warnings in test code as --enable-werror
> provides for the main compile.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  configure |   32 ++++++++++++++++++++++++++++----
>  1 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index 8140464..1939bdb 100755
> --- a/configure
> +++ b/configure
> @@ -27,16 +27,40 @@ printf " '%s'" "$0" "$@" >> config.log
>  echo >> config.log
>  echo "#" >> config.log
>
> +do_cc() {
> +    # Run the compiler, capturing its output to the log.
> +    echo $cc "$@" >> config.log
> +    $cc "$@" >> config.log 2>&1 || return $?
> +    # Test passed. If this is an --enable-werror build, rerun
> +    # the test with -Werror and bail out if it fails. This
> +    # makes warning-generating-errors in configure test code
> +    # obvious to developers.
> +    if test "$werror" != "yes"; then
> +        return 0
> +    fi
> +    # Don't bother rerunning the compile if we were already using -Werror
> +    case "$*" in
> +        *-Werror*)
> +           return 0
> +        ;;
> +    esac
> +    echo $cc -Werror "$@" >> config.log
> +    $cc -Werror "$@" >> config.log 2>&1 && return $?
> +    echo "ERROR: configure test passed without -Werror but failed with -Werror."
> +    echo "This is probably a bug in the configure script. The failing command"
> +    echo "will be at the bottom of config.log."
> +    echo "You can run configure with --disable-werror to bypass this check."
> +    exit 1

This should be degraded to a warning, I'm getting configure breakage
in some cases:
config-host.mak is out-of-date, running configure
ERROR: configure test passed without -Werror but failed with -Werror.
This is probably a bug in the configure script. The failing command
will be at the bottom of config.log.
You can run configure with --disable-werror to bypass this check.

gcc -Werror -m32 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall
-Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wold-style-declaration
-Wold-style-definition -Wtype-limits -I/usr/include/libpng12 -o
/tmp/qemu-conf--24538-.exe /tmp/qemu-conf--24538-.c -m32 -g
cc1: warnings being treated as errors
/tmp/qemu-conf--24538-.c:2: error: unknown option after '#pragma GCC
diagnostic' kind

> +}
> +
>  compile_object() {
> -  echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log
> -  $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1
> +  do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
>  }
>
>  compile_prog() {
>    local_cflags="$1"
>    local_ldflags="$2"
> -  echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log
> -  $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1
> +  do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
>  }
>
>  # symbolically link $1 to $2.  Portable version of "ln -sf".
> --
> 1.7.5.4
>
>

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Peter Maydell]

On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> +    echo "ERROR: configure test passed without -Werror but failed with -Werror."
>> +    echo "This is probably a bug in the configure script. The failing command"
>> +    echo "will be at the bottom of config.log."
>> +    echo "You can run configure with --disable-werror to bypass this check."
>> +    exit 1
>
> This should be degraded to a warning

The idea is that by running configure with --enable-werror (either
explicitly or implicitly for a git build) you've asked for compile
warnings to cause build failure. We're just doing that in configure
as well as for the main build.

>, I'm getting configure breakage
> in some cases:

The thesis of the patch series is that these represent bugs to be fixed.

-- PMM

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Blue Swirl]

On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> +    echo "ERROR: configure test passed without -Werror but failed with -Werror."
>>> +    echo "This is probably a bug in the configure script. The failing command"
>>> +    echo "will be at the bottom of config.log."
>>> +    echo "You can run configure with --disable-werror to bypass this check."
>>> +    exit 1
>>
>> This should be degraded to a warning
>
> The idea is that by running configure with --enable-werror (either
> explicitly or implicitly for a git build) you've asked for compile
> warnings to cause build failure. We're just doing that in configure
> as well as for the main build.
>
>>, I'm getting configure breakage
>> in some cases:
>
> The thesis of the patch series is that these represent bugs to be fixed.

Yes, but then the errors should be fixed by the series. When adding
new GCC warning flags, I also fixed the causes for the warning first.
Committing known build breakages is not OK.

>
> -- PMM

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Peter Maydell]

On 28 July 2012 13:09, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>, I'm getting configure breakage in some cases:
>>
>> The thesis of the patch series is that these represent bugs to be fixed.
>
> Yes, but then the errors should be fixed by the series. When adding
> new GCC warning flags, I also fixed the causes for the warning first.
> Committing known build breakages is not OK.

I fixed all the ones I saw on my system, obviously. I'll have a look
at this one.

I think it would be worth committing patches 1-10 now anyway : those fix
configure issues (patch 1 in particular fixes the problem of tests
silently failing when they should pass).

-- PMM

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Blue Swirl]

On Sat, Jul 28, 2012 at 12:14 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 28 July 2012 13:09, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>>, I'm getting configure breakage in some cases:
>>>
>>> The thesis of the patch series is that these represent bugs to be fixed.
>>
>> Yes, but then the errors should be fixed by the series. When adding
>> new GCC warning flags, I also fixed the causes for the warning first.
>> Committing known build breakages is not OK.
>
> I fixed all the ones I saw on my system, obviously. I'll have a look
> at this one.
>
> I think it would be worth committing patches 1-10 now anyway : those fix
> configure issues (patch 1 in particular fixes the problem of tests
> silently failing when they should pass).

OK, I'll test those.

>
> -- PMM

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Blue Swirl]

On Sat, Jul 28, 2012 at 12:14 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 28 July 2012 13:09, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>>, I'm getting configure breakage in some cases:
>>>
>>> The thesis of the patch series is that these represent bugs to be fixed.
>>
>> Yes, but then the errors should be fixed by the series. When adding
>> new GCC warning flags, I also fixed the causes for the warning first.
>> Committing known build breakages is not OK.
>
> I fixed all the ones I saw on my system, obviously. I'll have a look
> at this one.
>
> I think it would be worth committing patches 1-10 now anyway : those fix
> configure issues (patch 1 in particular fixes the problem of tests
> silently failing when they should pass).

I'm getting this error, probably because now Valgrind support is enabled:
  CC    coroutine-ucontext.o
cc1: warnings being treated as errors
/src/qemu/coroutine-ucontext.c:204: error: unknown option after
'#pragma GCC diagnostic' kind
/src/qemu/coroutine-ucontext.c:209: error: unknown option after
'#pragma GCC diagnostic' kind

Maybe the compiler does not understand this pragma and Valgrind check
should also fail in that case.

$ gcc -v
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian
4.4.5-8' --with-bugurl=file:///usr/share/doc/gcc-4.4/README.Bugs
--enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.4 --enable-shared --enable-multiarch
--enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix
--with-gxx-include-dir=/usr/include/c++/4.4 --libdir=/usr/lib
--enable-nls --enable-clocale=gnu --enable-libstdcxx-debug
--enable-objc-gc --with-arch-32=i586 --with-tune=generic
--enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.4.5 (Debian 4.4.5-8)

>
> -- PMM

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Peter Maydell]

On 28 July 2012 13:31, Blue Swirl <blauwirbel@gmail.com> wrote:
> I'm getting this error, probably because now Valgrind support is enabled:
>   CC    coroutine-ucontext.o
> cc1: warnings being treated as errors
> /src/qemu/coroutine-ucontext.c:204: error: unknown option after
> '#pragma GCC diagnostic' kind
> /src/qemu/coroutine-ucontext.c:209: error: unknown option after
> '#pragma GCC diagnostic' kind
>
> Maybe the compiler does not understand this pragma and Valgrind check
> should also fail in that case.

Yeah, I think this is one of the few tests which want to explicitly
check "is this construct going to provoke a compiler warning" --
fix is for that test to explictly put -Werror in the cflags in
the compile_prog line.

-- PMM

Re: [Qemu-devel] [PATCH 00/11] configure: Fix -Werror issues

[Blue Swirl]

Thanks, applied patches 1 to 10.

On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patch series:
>  1. turns off -Werror for configure tests
>  2. fixes a large pile of warnings in various configure tests
>  3. turns on -Werror for configure tests again, but in a way that means
>     that errors mean configure stops so the warnings are as obvious
>     to developers as they would be for a -Werror compile of the main code
>
> The "turn off -Werror" patch has already been sent to the list;
> I've included it here as it is a dependency, but it should probably
> be committed ASAP even if the rest of this series needs review and
> updating. I've also put all of Stefan's recent patches in since without
> them configure won't run once -Werror is reenabled.
>
> NB: this works for me, but it's possible there are some still lurking
> warnings in a few compilation tests that are themselves protected by
> tests for some library/host I don't have. They should be easy to fix
> as they turn up, though, and the configure failure message is clear
> about how to work around in the meantime.
>
> Peter Maydell (7):
>   configure: Don't run configure tests with -Werror enabled
>   configure: -march=i486 belongs in QEMU_CFLAGS, not CFLAGS
>   configure: Fix compile warning in PNG test
>   configure: Fix warnings in VDE library probe
>   configure: Fix compile warning in utimensat/futimens test
>   configure: -I\$(SRC_PATH) goes in QEMU_INCLUDES not QEMU_CFLAGS
>   configure: Check for -Werror causing failures when compiling tests
>
> Stefan Weil (4):
>   configure: Fix build with ALSA audio driver
>   configure: Fix build with capabilities
>   configure: Replace bash code by standard shell code
>   configure: Fix errors in test for__sync_fetch_and_and
>
>  configure |   73 +++++++++++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 57 insertions(+), 16 deletions(-)
>
> --
> 1.7.5.4
>
>

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Blue Swirl]

On Sat, Jul 28, 2012 at 1:48 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 28 July 2012 13:31, Blue Swirl <blauwirbel@gmail.com> wrote:
>> I'm getting this error, probably because now Valgrind support is enabled:
>>   CC    coroutine-ucontext.o
>> cc1: warnings being treated as errors
>> /src/qemu/coroutine-ucontext.c:204: error: unknown option after
>> '#pragma GCC diagnostic' kind
>> /src/qemu/coroutine-ucontext.c:209: error: unknown option after
>> '#pragma GCC diagnostic' kind
>>
>> Maybe the compiler does not understand this pragma and Valgrind check
>> should also fail in that case.
>
> Yeah, I think this is one of the few tests which want to explicitly
> check "is this construct going to provoke a compiler warning" --
> fix is for that test to explictly put -Werror in the cflags in
> the compile_prog line.

Now with your Xen configure patches in place, I'm not getting errors
with this applied except for Clang (which I didn't test earlier).
Maybe this should be applied.

clang -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall
-Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits
-I/usr/include/libpng12 -o /tmp/qemu-conf--25992-.exe
/tmp/qemu-conf--25992-.c -Wl,-z,relro -Wl,-z,now -pie -m64 -g
/tmp/qemu-conf--25992-.c:4:32: warning: self-comparison always
evaluates to true [-Wtautological-compare]
int main(void) { return preadv == preadv; }
                               ^
1 warning generated.
clang -Werror -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall
-Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits
-I/usr/include/libpng12 -o /tmp/qemu-conf--25992-.exe
/tmp/qemu-conf--25992-.c -Wl,-z,relro -Wl,-z,now -pie -m64 -g
/tmp/qemu-conf--25992-.c:4:32: error: self-comparison always evaluates
to true [-Werror,-Wtautological-compare]
int main(void) { return preadv == preadv; }
                               ^
1 error generated.
ERROR: configure test passed without -Werror but failed with -Werror.

clang -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall
-Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits
-I/usr/include/libpng12 -o /tmp/qemu-conf--25992-.exe
/tmp/qemu-conf--25992-.c -Wl,-z,relro -Wl,-z,now -pie -m64 -g
/tmp/qemu-conf--25992-.c:13:26: warning: self-comparison always
evaluates to true [-Wtautological-compare]
    return epoll_create1 == epoll_create1;
                         ^
1 warning generated.
clang -Werror -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall
-Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits
-I/usr/include/libpng12 -o /tmp/qemu-conf--25992-.exe
/tmp/qemu-conf--25992-.c -Wl,-z,relro -Wl,-z,now -pie -m64 -g
/tmp/qemu-conf--25992-.c:13:26: error: self-comparison always
evaluates to true [-Werror,-Wtautological-compare]
    return epoll_create1 == epoll_create1;
                         ^
1 error generated.
ERROR: configure test passed without -Werror but failed with -Werror.

clang -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall
-Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits
-I/usr/include/libpng12 -I/usr/include/nss -I/usr/include/nspr
-pthread -I/usr/include/glib-2.0
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -o
/tmp/qemu-conf--25992-.exe /tmp/qemu-conf--25992-.c -Wl,-z,relro
-Wl,-z,now -pie -m64 -g
/tmp/qemu-conf--25992-.c:3:13: warning: explicitly assigning a
variable of type 'char **' to itself [-Wself-assign]
    environ = environ;
    ~~~~~~~ ^ ~~~~~~~
1 warning generated.
clang -Werror -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall
-Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing
-fstack-protector-all -Wendif-labels -Wmissing-include-dirs
-Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wold-style-definition -Wtype-limits
-I/usr/include/libpng12 -I/usr/include/nss -I/usr/include/nspr
-pthread -I/usr/include/glib-2.0
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -o
/tmp/qemu-conf--25992-.exe /tmp/qemu-conf--25992-.c -Wl,-z,relro
-Wl,-z,now -pie -m64 -g
/tmp/qemu-conf--25992-.c:3:13: error: explicitly assigning a variable
of type 'char **' to itself [-Werror,-Wself-assign]
    environ = environ;
    ~~~~~~~ ^ ~~~~~~~
1 error generated.
ERROR: configure test passed without -Werror but failed with -Werror.

I used this hack to get more than one warning:
diff --git a/configure b/configure
index d8ec050..eb2ed2f 100755
--- a/configure
+++ b/configure
@@ -46,11 +46,11 @@ do_cc() {
     esac
     echo $cc -Werror "$@" >> config.log
     $cc -Werror "$@" >> config.log 2>&1 && return $?
-    echo "ERROR: configure test passed without -Werror but failed
with -Werror."
-    echo "This is probably a bug in the configure script. The failing command"
-    echo "will be at the bottom of config.log."
-    echo "You can run configure with --disable-werror to bypass this check."
-    exit 1
+    echo "ERROR: configure test passed without -Werror but failed
with -Werror." >> config.log
+    echo "This is probably a bug in the configure script. The failing
command" >> config.log
+    echo "will be at the bottom of config.log." >> config.log
+    echo "You can run configure with --disable-werror to bypass this
check." >> config.log
+#    exit 1
 }

 compile_object() {

>
> -- PMM

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Peter Maydell]

On 4 August 2012 18:57, Blue Swirl <blauwirbel@gmail.com> wrote:
> On Sat, Jul 28, 2012 at 1:48 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 28 July 2012 13:31, Blue Swirl <blauwirbel@gmail.com> wrote:
>>> I'm getting this error, probably because now Valgrind support is enabled:
>>>   CC    coroutine-ucontext.o
>>> cc1: warnings being treated as errors
>>> /src/qemu/coroutine-ucontext.c:204: error: unknown option after
>>> '#pragma GCC diagnostic' kind
>>> /src/qemu/coroutine-ucontext.c:209: error: unknown option after
>>> '#pragma GCC diagnostic' kind
>>>
>>> Maybe the compiler does not understand this pragma and Valgrind check
>>> should also fail in that case.
>>
>> Yeah, I think this is one of the few tests which want to explicitly
>> check "is this construct going to provoke a compiler warning" --
>> fix is for that test to explictly put -Werror in the cflags in
>> the compile_prog line.
>
> Now with your Xen configure patches in place, I'm not getting errors
> with this applied except for Clang (which I didn't test earlier).
> Maybe this should be applied.

Yes, I think that (assuming we are going to go down this route at all)
it would be good to apply this 11/11 patch now in good time before
the freeze. Does anybody want to object?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Blue Swirl]

On Thu, Aug 9, 2012 at 6:24 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 4 August 2012 18:57, Blue Swirl <blauwirbel@gmail.com> wrote:
>> On Sat, Jul 28, 2012 at 1:48 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 28 July 2012 13:31, Blue Swirl <blauwirbel@gmail.com> wrote:
>>>> I'm getting this error, probably because now Valgrind support is enabled:
>>>>   CC    coroutine-ucontext.o
>>>> cc1: warnings being treated as errors
>>>> /src/qemu/coroutine-ucontext.c:204: error: unknown option after
>>>> '#pragma GCC diagnostic' kind
>>>> /src/qemu/coroutine-ucontext.c:209: error: unknown option after
>>>> '#pragma GCC diagnostic' kind
>>>>
>>>> Maybe the compiler does not understand this pragma and Valgrind check
>>>> should also fail in that case.
>>>
>>> Yeah, I think this is one of the few tests which want to explicitly
>>> check "is this construct going to provoke a compiler warning" --
>>> fix is for that test to explictly put -Werror in the cflags in
>>> the compile_prog line.
>>
>> Now with your Xen configure patches in place, I'm not getting errors
>> with this applied except for Clang (which I didn't test earlier).
>> Maybe this should be applied.
>
> Yes, I think that (assuming we are going to go down this route at all)
> it would be good to apply this 11/11 patch now in good time before
> the freeze. Does anybody want to object?

I'll send a patch that fixes configure with Clang.

>
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH 11/11] configure: Check for -Werror causing failures when compiling tests

[Blue Swirl]

Thanks, applied.


On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Add support for checking whether test case code can compile without
> warnings, by recompiling each successful test with -Werror. If the
> -Werror version doesn't pass, we bail out. This gives us the same
> level of visibility of warnings in test code as --enable-werror
> provides for the main compile.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  configure |   32 ++++++++++++++++++++++++++++----
>  1 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/configure b/configure
> index 8140464..1939bdb 100755
> --- a/configure
> +++ b/configure
> @@ -27,16 +27,40 @@ printf " '%s'" "$0" "$@" >> config.log
>  echo >> config.log
>  echo "#" >> config.log
>
> +do_cc() {
> +    # Run the compiler, capturing its output to the log.
> +    echo $cc "$@" >> config.log
> +    $cc "$@" >> config.log 2>&1 || return $?
> +    # Test passed. If this is an --enable-werror build, rerun
> +    # the test with -Werror and bail out if it fails. This
> +    # makes warning-generating-errors in configure test code
> +    # obvious to developers.
> +    if test "$werror" != "yes"; then
> +        return 0
> +    fi
> +    # Don't bother rerunning the compile if we were already using -Werror
> +    case "$*" in
> +        *-Werror*)
> +           return 0
> +        ;;
> +    esac
> +    echo $cc -Werror "$@" >> config.log
> +    $cc -Werror "$@" >> config.log 2>&1 && return $?
> +    echo "ERROR: configure test passed without -Werror but failed with -Werror."
> +    echo "This is probably a bug in the configure script. The failing command"
> +    echo "will be at the bottom of config.log."
> +    echo "You can run configure with --disable-werror to bypass this check."
> +    exit 1
> +}
> +
>  compile_object() {
> -  echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log
> -  $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1
> +  do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC
>  }
>
>  compile_prog() {
>    local_cflags="$1"
>    local_ldflags="$2"
> -  echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log
> -  $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1
> +  do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags
>  }
>
>  # symbolically link $1 to $2.  Portable version of "ln -sf".
> --
> 1.7.5.4
>
>