Re: [Qemu-devel] [PATCH 01/18] error: add error_set_errno and error_setg_errno

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> These functions help maintaining homogeneous formatting of error
> messages that include strerror values.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  error.c | 28 ++++++++++++++++++++++++++++
>  error.h |  9 +++++++++
>  2 file modificati, 37 inserzioni(+)
>
> diff --git a/error.c b/error.c
> index 1f05fc4..128d88c 100644
> --- a/error.c
> +++ b/error.c
> @@ -43,6 +43,34 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
>      *errp = err;
>  }
>  
> +void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
> +                     const char *fmt, ...)
> +{
> +    Error *err;
> +    char *msg1;
> +    va_list ap;
> +
> +    if (errp == NULL) {
> +        return;
> +    }
> +    assert(*errp == NULL);
> +
> +    err = g_malloc0(sizeof(*err));
> +
> +    va_start(ap, fmt);
> +    msg1 = g_strdup_vprintf(fmt, ap);
> +    if (os_errno != 0) {
> +        err->msg = g_strdup_printf("%s: %s", msg1, strerror(os_errno));
> +        g_free(msg1);
> +    } else {
> +        err->msg = msg1;
> +    }
> +    va_end(ap);
> +    err->err_class = err_class;
> +
> +    *errp = err;
> +}
> +

Duplicates error_set() code without need.  How about:

void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
                     const char *fmt, ...)
{
    char *msg;

    va_start(ap, fmt);
    msg = g_strdup_printf(fmt, ap);
    va_end(ap);
    if (os_errno) {
        error_set(errp, err_class, "%s: %s", msg, strerror(os_errno));
    } else {
        error_set(errp, err_class, "%s", msg);
    }
    g_free(msg);
}

Sketch, not even compile-tested.

Or the other way round, implement error_set() in terms of the more
general error_set_errno():

void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
{
    error_set(errp, err_class, 0, fmt, ...);
}

Except that's not C; real code needs verror_set_errno().

As usual, every variadic function sooner or later needs a buddy that
takes a va_list instead.

>  Error *error_copy(const Error *err)
>  {
>      Error *err_new;
[...]