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

On 11/14/2011 05:54 PM, Stefan Berger wrote: The glibc headers are expanding the pre-patch code to: if ((__extension__ (__builtin_constant_p (*cur) && !__builtin_constant_p (toescape) && (*cur) == '\0' ? (char *) __rawmemchr (toescape, *cur) : __builtin_strchr (toescape, *cur)))) but warning about: util/buf.c:469: warning: logical '&&' with non-zero constant will always evaluate as true [-Wlogical-op] I see two constants in that expression: !__builtin_constant_p (toescape) is always 1 (*cur) == '\0' is always 0 (since the containing while loop already guaranteed *cur != 0) There's definitely at least one gcc bug: that of an incorrect error message. After all, the expression 'expra && exprb' is constant in only three cases: 'expra' is false, 'exprb' is false, or 'expra' is true and 'exprb' is true. But the gcc message isn't clear on whether it is warning about 'expra' or 'exprb', not to mention that it claims the overall expression is true even though we just proved the overall expression is always false by virtue of exprb being false. Whether there is a secondary bug, about gcc being noisy with -Wlogical-op inside __extension__, is also worth arguing. 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. 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. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org