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

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 > > * include/libvirt/libvirt.h.in: Hide all sentinels behind > LIBVIRT_ENUM_SENTINELS, and add missing sentinels. > * src/internal.h (VIR_DEPRECATED): Allow inclusion after > libvirt.h. > (LIBVIRT_ENUM_SENTINELS): Expose sentinels internally. > * daemon/libvirtd.h: Use the sentinels. > * src/remote/remote_protocol.x (includes): Don't expose sentinels. > * python/generator.py (enum): Likewise. > * tests/cputest.c (cpuTestCompResStr): Silence compiler warning. > * tools/virsh.c (vshDomainStateReasonToString) > (vshDomainControlStateToString): Likewise. > --- > daemon/libvirtd.h | 8 +- > include/libvirt/libvirt.h.in | 186 ++++++++++++++++++++++++++++++++++++----- > python/generator.py | 3 +- > src/internal.h | 4 + > src/remote/remote_protocol.x | 3 +- > tests/cputest.c | 3 +- > tools/virsh.c | 9 ++ > 7 files changed, 187 insertions(+), 29 deletions(-) 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.