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 :| -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list