[libvirt] [PATCH 3/3] Add sentinel for virErrorNumber enum

[Daniel P. Berrange]

On Thu, May 17, 2012 at 08:39:55AM -0600, Eric Blake wrote:
> > +VIR_ENUM_DECL(virErrorNumber)
> > +VIR_ENUM_IMPL(virErrorNumber, VIR_ERR_NUMBER_LAST,
> > +              N_(""), /* 0 */
> > +              N_("internal error"),
> > +              N_("out of memory"),
> > +              N_("this function is not supported by the current driver"),
> > +              N_("unknown hostname"),
> > +
> > +              N_("no connection driver available"), /* 5 */
> > +              N_("invalid connection pointer"),
> 
> Are you SURE that all of the new messages still make sense?  I'm worried
> that you need to split this into some prerequisite patches that change
> the message contents _and all callers_, before the patch that collapses
> things into a VIR_ENUM.  Using VIR_ERR_INVALID_CONN as an example,
> 
> libvirt.c:virConnectClose calls: virLibConnError(VIR_ERR_INVALID_CONN,
> __FUNCTION__)
> 
> Pre-patch, this results in 'location: custom message':
> 
> virConnectClose: invalid connection pointer in virConnectClose
> 
> post-patch, this results in 'locatoin: category: half a custom message':
> 
> virConnectClose: invalid connection pointer: virConnectClose
> 
> and we have a bad error message.  But if we first clean up all callers
> of VIR_ERR_INVALID_CONN to pass a useful message, rather than the
> current status quo of just a function name, _then_ shortening the error
> string to 'location: category: message' will make more sense.

I have just start doing this *huge* job, and I'm really liking the
results. See work in progress:

  https://www.redhat.com/archives/libvir-list/2012-May/msg01231.html


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