Re: [PATCH 1/2] API: make declaration of _LAST enum values conditional

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

 



On 01/20/2012 02:41 PM, Jiri Denemark wrote:
> On Fri, Jan 20, 2012 at 14:06:26 -0700, Eric Blake wrote:
>> Although this is a public API break, it only affects users that
>> were compiling against *_LAST values, and can be trivially
>> worked around without impacting compilation against older
>> headers, by the user defining LIBVIRT_ENUM_SENTINELS before
>> including libvirt.h.  It is not an ABI break, since enum values
>> do not appear as .so entry points.
>>
>> See this list discussion:
>> https://www.redhat.com/archives/libvir-list/2012-January/msg00804.html
>>

> 
> Overall, I like the patch but I don't agree with the way of silencing
> compilation warnings in the second part.  The main benefit of avoiding default
> in switches used with enums is that the compiler is nice and warns us when new
> item is added to an enum.  I'd prefer not to ruin this.
> 

>> @@ -205,6 +205,7 @@ cpuTestCompResStr(virCPUCompareResult result)
>>      case VIR_CPU_COMPARE_INCOMPATIBLE:  return "INCOMPATIBLE";
>>      case VIR_CPU_COMPARE_IDENTICAL:     return "IDENTICAL";
>>      case VIR_CPU_COMPARE_SUPERSET:      return "SUPERSET";
>> +    default: ;
>         case VIR_CPU_COMPARE_LAST: ;

Ah, using an explicit _LAST rather than a generic default: also silences
the warning.  Nice to know.

Oh, and I just realized that I should have probably named it
VIR_ENUM_SENTINELS, not LIBVIRT_ENUM_SENTINELS (which I had blindly
copied from Daniel's email), to match the namespace that we used for all
our other public macros; I did a quick search and replace to fix that
before pushing.

> 
> ACK with those defaults removed.

Done and pushed, along with patch 2.

-- 
Eric Blake   eblake@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]