[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv3 1/3] build: Check for broken GCC -Wlogical-op in configure



On 12/14/2012 08:08 AM, Viktor Mihajlovski wrote:
> Some older versions of GCC report a false positive on code like
>   char * haystack, needle;
>   strchr(haystack, needle);
> 
> Added an extra check in configure.ac which will
>   #define BROKEN_GCC_WLOGICALOP 1
> in this case, allowing to special handle "offending" code.
> 
> Signed-off-by: Viktor Mihajlovski <mihajlov linux vnet ibm com>
> ---
>  configure.ac |   23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)

I know this is already pushed, but I think it would fit better under
m4/virt-compile-warnings.m4 rather than at the top-level configure.ac.

> +
> +if test "x$gcc_false_strchr_warning" = xyes; then
> +  AC_DEFINE_UNQUOTED([BROKEN_GCC_WLOGICALOP], 1,
> +  		     [GCC -Wlogical-op is reporting false positive on strchr])
> +fi

Potential ouch.  This sets BROKEN_GCC_WLOGICALOP to 1 on all non-gcc
compilers.  Then in patch 2/3, you blindly use this condition to request
a pragma:

+/*
+  we need to ignore warnings about strchr caused by -Wlogical-op
+  for some GCC versions.
+  Doing it via a local pragma keeps the damage smaller than
+  disabling it on the package level.
+  Unfortunately, the affected GCCs don't allow diagnostic push/pop
+  which would have further reduced the impact.
+ */
+# if BROKEN_GCC_WLOGICALOP
+# pragma GCC diagnostic ignored "-Wlogical-op"
+# endif

We are not guaranteed that other compilers behave nicely when presented
with unknown #pragma GCC.  So far, our use of such pragmas has been
guarded by knowing that we are using gcc; so I'd feel better if the
configure-time check for setting this flag were limited to just gcc in
the first place (which it would be easier to do by moving this test to
m4/virt-compile-warnings.m4).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]