[libvirt] Non-zero constant warning on RHEL 6.1 with 0.9.7

[Stefan Berger]

On 11/14/2011 10:36 PM, Eric Blake wrote:
> On 11/14/2011 05:54 PM, Stefan Berger wrote:
>> On 11/11/2011 04:06 PM, Eric Blake wrote:
>>> On 11/08/2011 11:46 PM, Justin Clift wrote:
>>>> Hi guys,
>>>>
>>>> Just checking 0.9.7 on RHEL 6.1 x86_64.  Noticed this when compiling
>>>> with make -j 3:
>>>>
>>>>       CC     libvirt_lxc-command.o
>>>>     util/buf.c: In function 'virBufferEscape':
>>>>     util/buf.c:469: warning: logical '&&' with non-zero constant will
>>>> always evaluate as true [-Wlogical-op]
>>>>
>>>> Obviously not fatal, but it figured someone might want to keep things
>>>> warning free. :)
>>> This is a bug in gcc, although I'm not sure if anyone has raised a bug
>>> report against the gcc folks yet:
>>> https://www.redhat.com/archives/libvir-list/2011-October/msg00837.html
>>>
>>> I'm open to suggestions on how to work around it; perhaps we just need
>>> to disable -Wlogical-op if glibc's headers are turning on the optimized
>>> strchr (is that -D_FORTIFY_SOURCE that does it?) :(
>>
> At any rate, I agree that your proposed patch skirts the issue by using
> strstr() instead of strchr(), so it would indeed do the trick (while
> under the hood, strstr() on a single-byte needle tends to defer to
> strchr() anyways).  It feels kind of gross, without at least a comment
> explaining that we are working around a gcc bug in -Wlogical-op (and
> even better, a URL of the gcc bug report); but I'm inclined to ACK it if
> we can come up with an appropriate comment.
>
What about this here:

--- a/src/util/buf.c
+++ b/src/util/buf.c
@@ -466,7 +466,11 @@ virBufferEscape(virBufferPtr buf, const char *toescape,
      cur = str;
      out = escaped;
      while (*cur != 0) {
-        if (strchr(toescape, *cur))
+        /* strchr work-around for gcc 4.3 & 4.4 bug with -Wlogical-op
+         * http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36513
+         */
+        char needle[2] = { *cur, 0 };
+        if (strstr(toescape, needle))
              *out++ = '\\';
          *out++ = *cur;
          cur++;


> An even more aggressive patch would use strpbrk() to locate each
> character that needs escaping and memcpy() to quickly copy chunks of
> non-escaped text, rather than processing things one byte at a time, but
> I'm not sure whether that would be a micro-optimization or whether this
> function is called often enough to make it worth the extra complexity.
>
> What does 'make V=1' show as the exact compiler line used when
> triggering the error?  I still haven't been able to reproduce the bogus
> compiler warning on a much smaller test case, but suspect I may be
> missing a particular flag that makes it apparent.
>
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I.. 
-I../gnulib/lib -I../gnulib/lib -I../include -I../src/util -I../include 
-DIN_LIBVIRT -I/usr/include/libxml2 -Wall -W -Wformat-y2k 
-Wformat-security -Winit-self -Wmissing-include-dirs -Wunused 
-Wunknown-pragmas -Wstrict-aliasing -Wshadow -Wpointer-arith 
-Wbad-function-cast -Wcast-align -Wwrite-strings -Wlogical-op 
-Waggregate-return -Wstrict-prototypes -Wold-style-definition 
-Wmissing-prototypes -Wmissing-declarations -Wmissing-noreturn 
-Wmissing-format-attribute -Wredundant-decls -Wnested-externs -Winline 
-Winvalid-pch -Wvolatile-register-var -Wdisabled-optimization 
-Wbuiltin-macro-redefined -Wmudflap -Wpacked-bitfield-compat -Wsync-nand 
-Wattributes -Wcoverage-mismatch -Wmultichar 
-Wno-missing-field-initializers -Wno-sign-compare 
-Wframe-larger-than=4096 -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-all 
--param=ssp-buffer-size=4 -fexceptions -fasynchronous-unwind-tables 
-fdiagnostics-show-option -funit-at-a-time -g -O2 -MT 
libvirt_util_la-buf.lo -MD -MP -MF .deps/libvirt_util_la-buf.Tpo -c 
util/buf.c  -fPIC -DPIC -o .libs/libvirt_util_la-buf.o
util/buf.c: In function 'virBufferEscape':
util/buf.c:469: warning: logical '&&' with non-zero constant will always 
evaluate as true [-Wlogical-op]


    Stefan