[libvirt] [PATCH 2/3] datatypes: avoid redundant __FUNCTION__

[Dave Allan]

On Wed, May 12, 2010 at 10:31:50AM +0200, Matthias Bolte wrote:
> 2010/5/12 Eric Blake <eblake at redhat.com>:

<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