[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]