On Fri, Jan 20, 2012 at 10:41:37PM +0100, 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
* 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.
Now that we have the _LAST members for these enums, we can kill off many of these crazy switch statements, and just use VIR_ENUM_IMPL which is statically checked. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|