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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list