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

[Daniel P. Berrange]

On Tue, Nov 15, 2011 at 06:34:27AM -0500, Stefan Berger wrote:
> 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++ = '\\';

I'm puzzelled that we need this change here, but we have 175 other
uses of strchr without trouble

  $ grep strchr src/*/*.c | wc -l
  174

Is the difference just that we're deferencing a char * for the second arg
rather than using a constant value ?

Would it suffice to do

  int c = *cur;
  if (strchr(toescape, c))

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|