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

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



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 :|


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