On Wed, May 12, 2010 at 10:31:50AM +0200, Matthias Bolte wrote: > 2010/5/12 Eric Blake <eblake@xxxxxxxxxx>: <snip> > As Chris pointed out a while ago: > > https://www.redhat.com/archives/libvir-list/2010-April/msg01241.html > > There are some error codes that have string representations attached > which imply a certain usage style. VIR_ERR_INVALID_ARG is one of > those. See virErrorMsg: > > case VIR_ERR_INVALID_ARG: > if (info == NULL) > errmsg = _("invalid argument in"); > else > errmsg = _("invalid argument in %s"); > break; > > This implies that you pass the function name as additional > information. But I must admit that I never used VIR_ERR_INVALID_ARG as > it is implied, because I wasn't aware of that until recently. > > The VIR_ERR_XML_ERROR is an even worse example: > > case VIR_ERR_XML_ERROR: > if (info == NULL) > errmsg = _("XML description not well formed or invalid"); > else > errmsg = _("XML description for %s is not well formed > or invalid"); > break; > > Unrelated to your patch I suggest that we unify the string > representations for error codes to a common style: > > case VIR_ERR_INVALID_ARG: > if (info == NULL) > errmsg = _("invalid argument"); > else > errmsg = _("invalid argument: %s"); > break; > > case VIR_ERR_XML_ERROR: > if (info == NULL) > errmsg = _("XML description not well formed or invalid"); > else > errmsg = _("XML description not well formed or invalid: %s"); > break; > > And adapt the callers. +1 A common style for error messages is the right way to go, and I like the style Matthias proposes of "general error message: %s", "specific error" IMO, most of the time outputting a function name in an error message only obscures the message, and in the cases in which it's actually useful information, the writer of the message can always include it manually. The same goes for pointer addresses. I think this rework is a real usability enhancement. If we have consensus that we should do it, I'll submit patches for the storage and node device code. Dave -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list