Re: [Qemu-devel] [RFC PATCH 5/7] error reporting: Provide error_report_errnoval (and error_vreport_errnoval)

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2160 bytes --]

On 04/26/2018 11:53 AM, Ian Jackson wrote:
> This will let us replace more open coded calls to error_report and
> strerror.
> 
> I have chosen to provide all of
>    error_report_errno       error_vreport_errno
>    error_report_errnoval    error_vreport_errnoval
> because the former are much more common, and deserve a short spelling;
> whereas there are still at least 30-40 potential callers of the latter.

As mentioned in 2/7, that's inconsistent with error_setg_errno().  I'd
MUCH rather see us have JUST error_[v]report_errno with a mandatory
error parameter, rather than blindly relying on implicit use of errno;
it's not that much harder to write:

error_report_errno(errno, "fmt string...", args);

in the common case when directly using errno is intended.

>  
> +void error_vreport_errnoval(int errnoval,
> +                            const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0);
> +void error_report_errnoval(int errnoval,
> +                           const char *fmt, ...) GCC_FMT_ATTR(2, 3);

Bikeshedding - can we name the parameter 'os_error' (as in
error_seg_errno), or 'err' or 'errval', rather than the longer 'errnoval'?


> +++ b/util/qemu-error.c
> @@ -257,6 +257,18 @@ void error_vreport_errno(const char *fmt, va_list ap)
>  }
>  
>  /*
> + * Print an error message to current monitor if we have one, else to stderr.
> + * Format arguments like vsprintf().  The resulting message should be
> + * a single phrase, with no newline or trailing punctuation.
> + * Prepend the current location and append ": " strerror(errnoval) "\n".
> + * It's wrong to call this in a QMP monitor.  Use error_setg() there.
> + */
> +void error_vreport_errnoval(int errnoval, const char *fmt, va_list ap)
> +{
> +    vreport(REPORT_TYPE_ERROR, errnoval, fmt, ap);
> +}

Should this explicitly document that passing 0 for errnoval is
acceptable or forbidden?  If acceptable, does that mean no suffix (other
than \n) is added?  If forbidden, do we want to assert() that?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]