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

[Luiz Capitulino]

On Wed,  3 Oct 2012 16:36:48 +0200
Paolo Bonzini <address@hidden> wrote:

> These functions help maintaining homogeneous formatting of error
> messages that include strerror values.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>

Acked-by: Luiz Capitulino <address@hidden>

One small comment below.

> ---
>  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, ...)

The function's name makes me expect that something else is done with os_errno
other than appending the strerror() string to the user message, but I can't
suggest anything better (and allows for storing errno in the Error object in
the future if we want to).

> +{
> +    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;
> +}
> +
>  Error *error_copy(const Error *err)
>  {
>      Error *err_new;
> diff --git a/error.h b/error.h
> index da7fed3..4d52e73 100644
> --- a/error.h
> +++ b/error.h
> @@ -30,10 +30,19 @@ typedef struct Error Error;
>  void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) 
> GCC_FMT_ATTR(3, 4);
>  
>  /**
> + * Set an indirect pointer to an error given a ErrorClass value and a
> + * printf-style human message, followed by a strerror() string if
> + * @os_error is not zero.
> + */
> +void error_set_errno(Error **err, int os_error, ErrorClass err_class, const 
> char *fmt, ...) GCC_FMT_ATTR(4, 5);
> +
> +/**
>   * Same as error_set(), but sets a generic error
>   */
>  #define error_setg(err, fmt, ...) \
>      error_set(err, ERROR_CLASS_GENERIC_ERROR, fmt, ## __VA_ARGS__)
> +#define error_setg_errno(err, os_error, fmt, ...) \
> +    error_set_errno(err, os_error, ERROR_CLASS_GENERIC_ERROR, fmt, ## 
> __VA_ARGS__)
>  
>  /**
>   * Returns true if an indirect pointer to an error is pointing to a valid