Re: [libvirt] [PATCH 1/2] API: make declaration of _LAST enum values conditional
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)