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