Re: [libvirt] [PATCH] maint: turn on gcc logical-op checking
On 07/24/2010 02:36 PM, Matthias Bolte wrote:
> + * nuking the wrappers; even if it removes some type-check
safety. */
> +# undef curl_easy_getinfo
> +# undef curl_easy_setopt
> +
> #define VIR_FROM_THIS VIR_FROM_ESX
No need to hack here. We can define CURL_DISABLE_TYPECHECK to disable
those type checks.
Yes, that sounds nicer.
Also the warnings are not that spuriously. If you look at
typecheck-gcc.h you'll see that the condition of the if clauses only
depend onto type information. This type information is static at
compile- and runtime, so gcc is correct to warn about them being fixed
true or false.
In the sense that the conditions are always true or false, yes, gcc
could warn. But because they are hidden by macro expansion (especially
since the macros come from a third-party header), it would be nice if
gcc would avoid the warning, because it is much different then
explicitly writing the always-true && directly in the .c file.
Meanwhile, I can't help but wonder if the curl.h headers could be
written in a manner less prone to trip the gcc warning in the first
place. That is, instead of doing:
if (arg == type1 && cond)
warn();
if (arg == type2 && cond)
warn();
...
it could instead do:
switch (arg) {
type1: if (cond) warn(); break;
type2: if (cond) warn(); break;
}
...
But that's not for libvirt to decide.
This also affects the XenAPI driver because it includes curl.h too.
Therefore, I suggest the attached variation of your patch.
ACK to your version as being better than mine. But maybe we should wait
for a third-party ACK before pushing either version?
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)