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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org