[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 5/7] error reporting: Provide error_report_e
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [RFC PATCH 5/7] error reporting: Provide error_report_errnoval (and error_vreport_errnoval) |
Date: |
Thu, 26 Apr 2018 12:31:55 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 |
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
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [RFC PATCH 3/7] error reporting: Use error_report_errno in obvious places, (continued)
- [Qemu-devel] [RFC PATCH 7/7] error reporting: HACKING: Say to use error_report_errno, Ian Jackson, 2018/04/26
- [Qemu-devel] [RFC PATCH 2/7] error reporting: Provide error_report_errno (and error_vreport_errno), Ian Jackson, 2018/04/26
- [Qemu-devel] [RFC PATCH 5/7] error reporting: Provide error_report_errnoval (and error_vreport_errnoval), Ian Jackson, 2018/04/26
- Re: [Qemu-devel] [RFC PATCH 5/7] error reporting: Provide error_report_errnoval (and error_vreport_errnoval),
Eric Blake <=
- [Qemu-devel] [RFC PATCH 6/7] error reporting: Use error_report_errnoval in obvious places, Ian Jackson, 2018/04/26