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 <ian.jack...@eu.citrix.com>
> ---
> 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
