[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH 1/7] error reporting: Introduce errnoval par
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [RFC PATCH 1/7] error reporting: Introduce errnoval parameter to vreport |
Date: |
Thu, 26 Apr 2018 12:24:37 -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 allow new callers of vreport to specify that an errno value
> should be printed too. Update all existing callers.
>
> We use strerror rather than strerror_r because strerror_r presents
> portability difficulties. Replacing strerror with strerror_r (or
> something else) is left to the future.
Is g_strerror() suitably easy to use, which would at least avoid the
portability difficulties?
>
> No functional change yet.
>
> Signed-off-by: Ian Jackson <address@hidden>
> ---
> util/qemu-error.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/util/qemu-error.c b/util/qemu-error.c
> index a25d3b9..9acc4b5 100644
> --- a/util/qemu-error.c
> +++ b/util/qemu-error.c
> @@ -191,12 +191,14 @@ bool enable_timestamp_msg;
> /*
> * Print a message to current monitor if we have one, else to stderr.
> * @report_type is the type of message: error, warning or informational.
> + * If @errnoval is nonnegative it is fed to strerror and printed too.
That implies 0 is fed to strerror(), which is not the case. Better
would be "If @errnoval is positive...". Or, if we wanted, we could say
"If @errnoval is not zero, its absolute value is fed to strerror" to let
people pass in both EIO and -EIO for the same output (something that
strerror() can't do, but our wrapper can) - but I don't know if that
convenience is a good thing.
> * Format arguments like vsprintf(). The resulting message should be
> * a single phrase, with no newline or trailing punctuation.
> * Prepend the current location and append a newline.
> * It's wrong to call this in a QMP monitor. Use error_setg() there.
> */
> -static void vreport(report_type type, const char *fmt, va_list ap)
> +static void vreport(report_type type, int errnoval,
Bikeshedding: Is 'err' or 'errval' a better name in terms of length and
legibility?
> + const char *fmt, va_list ap)
> {
> GTimeVal tv;
> gchar *timestr;
> @@ -222,6 +224,11 @@ static void vreport(report_type type, const char *fmt,
> va_list ap)
> }
>
> error_vprintf(fmt, ap);
> +
> + if (errnoval >= 0) {
> + error_printf(": %s", strerror(errnoval));
Off-by-one. You do NOT want to print strerror(0) (that results in
appending ": Success" or similar to what is supposed to be an error
message).
> + }
> +
> error_printf("\n");
> }
>
> @@ -234,7 +241,7 @@ static void vreport(report_type type, const char *fmt,
> va_list ap)
> */
> void error_vreport(const char *fmt, va_list ap)
> {
> - vreport(REPORT_TYPE_ERROR, fmt, ap);
> + vreport(REPORT_TYPE_ERROR, -1, fmt, ap);
Passing -1 to suppress the output feels awkward, especially if 0 can be
used for the same purpose and would then let us use -errno and errno
interchangeable by passing the absolute value to sterror.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
[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