2010/7/23 Eric Blake <eblake@xxxxxxxxxx>:
> This would have detected the bug in commit 38ad33931 (Aug 09), which
> we missed until commit f828ca35 (Jul 10); over 11 months later.
>
> However, on Fedora 13, it also triggers LOTS of warnings from
> the libcurl-devel header for one file:
>
> esx/esx_vi.c: In function 'esxVI_CURL_Perform':
> esx/esx_vi.c:232: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
> esx/esx_vi.c:232: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
> esx/esx_vi.c:232: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
> ...
>
> I figure the gross hack to disable the warnings in the third-party
> code, along with the reduction in type-safety in just esx_vi.c,
> is worth the improved compiler checking throughout the rest of libvirt.
>
> * acinclude.m4 (--enable-compile-warnings=error): Add
> -Wlogical-op.
> * src/esx/esx_vi.c (includes): Hack around broken libcurl-devel
> header, to avoid compilation warning.
> Suggested by Daniel P. Berrange.
> ---
>
> In response to
> https://www.redhat.com/archives/libvir-list/2010-July/msg00497.html
> and fixing some long lines while I was at it.
>
> acinclude.m4 | 16 +++++++++++++---
> src/esx/esx_vi.c | 10 ++++++++++
> 2 files changed, 23 insertions(+), 3 deletions(-)
>
> @@ -36,6 +36,16 @@
> #include "esx_vi_methods.h"
> #include "esx_util.h"
>
> +
> +/* XXX "esx_vi.h" includes <curl/curl.h>; as of
> + * libcurl-devel-7.20.1-3.fc13.x86_64, curl ships a version of this
> + * header that #defines several wrapper macros around underlying
> + * functions to add type safety for gcc only. However, these macros
> + * spuriously trip gcc's -Wlogical-op warning. Avoid the warning by
> + * nuking the wrappers; even if it removes some type-check safety. */
> +# undef curl_easy_getinfo
> +# undef curl_easy_setopt
> +
> #define VIR_FROM_THIS VIR_FROM_ESX
No need to hack here. We can define CURL_DISABLE_TYPECHECK to disable
those type checks.
Also the warnings are not that spuriously. If you look at
typecheck-gcc.h you'll see that the condition of the if clauses only
depend onto type information. This type information is static at
compile- and runtime, so gcc is correct to warn about them being fixed
true or false.
This also affects the XenAPI driver because it includes curl.h too.
Therefore, I suggest the attached variation of your patch.
Matthias
From 53904deb188a19d506af5d2ec82b543df952ecd9 Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@xxxxxxxxxx>
Date: Fri, 23 Jul 2010 14:28:31 -0600
Subject: [PATCH] maint: turn on gcc logical-op checking
This would have detected the bug in commit 38ad33931 (Aug 09), which
we missed until commit f828ca35 (Jul 10); over 11 months later.
However, on Fedora 13, it also triggers LOTS of warnings from
the libcurl-devel header for two files:
esx/esx_vi.c: In function 'esxVI_CURL_Perform':
esx/esx_vi.c:232: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
esx/esx_vi.c:232: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
esx/esx_vi.c:232: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
...
xenapi/xenapi_driver.c: In function 'call_func':
xenapi/xenapi_driver.c:1872: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
xenapi/xenapi_driver.c:1872: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
xenapi/xenapi_driver.c:1872: error: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op]
...
libcurl allows to disable the type-checking code that triggers those
warnings, along with the reduction in type-safety of calls to some
libcurl functions. I figure this is worth the improved compiler
checking throughout the rest of libvirt.
* acinclude.m4 (--enable-compile-warnings=error): Add -Wlogical-op.
* configure.ac: Add -DCURL_DISABLE_TYPECHECK to LIBCURL_CFLAGS to
avoid compilation warning.
Suggested by Daniel P. Berrange.
Tweaked by Matthias Bolte.
---
acinclude.m4 | 16 +++++++++++++---
configure.ac | 6 ++++++
2 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index 8c97184..838ec46 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -36,9 +36,19 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
try_compiler_flags="-Wall -Wformat -Wformat-security -Wmissing-prototypes $common_flags"
;;
maximum|error)
- try_compiler_flags="-Wall -Wformat -Wformat-security -Wmissing-prototypes -Wnested-externs -Wpointer-arith"
- try_compiler_flags="$try_compiler_flags -Wextra -Wshadow -Wcast-align -Wwrite-strings -Waggregate-return"
- try_compiler_flags="$try_compiler_flags -Wstrict-prototypes -Winline -Wredundant-decls -Wno-sign-compare"
+ try_compiler_flags="-Wall -Wformat -Wformat-security"
+ try_compiler_flags="$try_compiler_flags -Wmissing-prototypes"
+ try_compiler_flags="$try_compiler_flags -Wnested-externs "
+ try_compiler_flags="$try_compiler_flags -Wpointer-arith"
+ try_compiler_flags="$try_compiler_flags -Wextra -Wshadow"
+ try_compiler_flags="$try_compiler_flags -Wcast-align"
+ try_compiler_flags="$try_compiler_flags -Wwrite-strings"
+ try_compiler_flags="$try_compiler_flags -Waggregate-return"
+ try_compiler_flags="$try_compiler_flags -Wstrict-prototypes"
+ try_compiler_flags="$try_compiler_flags -Winline"
+ try_compiler_flags="$try_compiler_flags -Wredundant-decls"
+ try_compiler_flags="$try_compiler_flags -Wno-sign-compare"
+ try_compiler_flags="$try_compiler_flags -Wlogical-op"
try_compiler_flags="$try_compiler_flags $common_flags"
if test "$enable_compile_warnings" = "error" ; then
try_compiler_flags="$try_compiler_flags -Werror"
diff --git a/configure.ac b/configure.ac
index 08b7eb6..dda8f1b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1631,6 +1631,12 @@ if test "$with_xenapi" = "yes" ; then
fi
AM_CONDITIONAL([WITH_XENAPI], [test "$with_xenapi" = "yes"])
+# XXX as of libcurl-devel-7.20.1-3.fc13.x86_64, curl ships a version
+# of <curl/curl.h> that #defines several wrapper macros around underlying
+# functions to add type safety for gcc only. However, these macros
+# spuriously trip gcc's -Wlogical-op warning. Avoid the warning by
+# disabling the wrappers; even if it removes some type-check safety.
+LIBCURL_CFLAGS="-DCURL_DISABLE_TYPECHECK $LIBCURL_CFLAGS"
AC_SUBST([LIBCURL_CFLAGS])
AC_SUBST([LIBCURL_LIBS])
--
1.7.0.4
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list