Re: [Qemu-devel] [PATCH 26/34] error, qerror: add ErrorClass argument to error functions

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> The new argument is added to functions qerror_report() and error_set().
> It's stored in Error and QError. qerror_report_err() is also updated to
> take care of it.
>
> The QERR_ macros are changed to contain a place holder value for the
> new argument, so that the value is used on all current calls to
> qerror_report() and error_set() (and also to initialize qerror_table[]).
>
> Next commit will update the QERR_ macros with a proper ErrorClass
> value.
>
> Signed-off-by: Luiz Capitulino <address@hidden>
> ---
>  error.c  |   6 ++-
>  error.h  |   3 +-
>  qerror.c |  10 +++--
>  qerror.h | 145 
> ++++++++++++++++++++++++++++++++-------------------------------
>  4 files changed, 88 insertions(+), 76 deletions(-)
>
> diff --git a/error.c b/error.c
> index 216cb08..6c8f7b8 100644
> --- a/error.c
> +++ b/error.c
> @@ -14,6 +14,7 @@
>  #include "error.h"
>  #include "qjson.h"
>  #include "qdict.h"
> +#include "qapi-types.h"
>  #include "error_int.h"
>  #include "qerror.h"
>  
> @@ -21,9 +22,10 @@ struct Error
>  {
>      QDict *obj;
>      char *msg;
> +    ErrorClass err_class;
>  };
>  
> -void error_set(Error **errp, const char *fmt, ...)
> +void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
>  {
>      Error *err;
>      va_list ap;
> @@ -39,6 +41,7 @@ void error_set(Error **errp, const char *fmt, ...)
>      err->obj = qobject_to_qdict(qobject_from_jsonv(fmt, &ap));
>      va_end(ap);
>      err->msg = qerror_format(fmt, err->obj);
> +    err->err_class = err_class;
>  
>      *errp = err;
>  }
> @@ -49,6 +52,7 @@ Error *error_copy(const Error *err)
>  
>      err_new = g_malloc0(sizeof(*err));
>      err_new->msg = g_strdup(err->msg);
> +    err_new->err_class = err->err_class;
>      err_new->obj = err->obj;
>      QINCREF(err_new->obj);
>  
> diff --git a/error.h b/error.h
> index 3d038a5..905613a 100644
> --- a/error.h
> +++ b/error.h
> @@ -13,6 +13,7 @@
>  #define ERROR_H
>  
>  #include "compiler.h"
> +#include "qapi-types.h"
>  #include <stdbool.h>
>  
>  /**
> @@ -26,7 +27,7 @@ typedef struct Error Error;
>   * Currently, qerror.h defines these error formats.  This function is not
>   * meant to be used outside of QEMU.
>   */
> -void error_set(Error **err, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
> +void error_set(Error **err, ErrorClass err_class, const char *fmt, ...) 
> GCC_FMT_ATTR(3, 4);
>  
>  /**
>   * Returns true if an indirect pointer to an error is pointing to a valid
> diff --git a/qerror.c b/qerror.c
> index 664efb3..19a1902 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -386,13 +386,15 @@ static QDict *error_obj_from_fmt_no_fail(const char 
> *fmt, va_list *va)
>   *
>   * Return strong reference.
>   */
> -static QError *qerror_from_info(const char *fmt, va_list *va)
> +static QError *qerror_from_info(ErrorClass err_class, const char *fmt,
> +                                va_list *va)
>  {
>      QError *qerr;
>  
>      qerr = qerror_new();
>      loc_save(&qerr->loc);
>  
> +    qerr->err_class = err_class;
>      qerr->error = error_obj_from_fmt_no_fail(fmt, va);
>      qerr->err_msg = qerror_format(fmt, qerr->error);
>  
> @@ -518,13 +520,13 @@ static void qerror_print(QError *qerror)
>      QDECREF(qstring);
>  }
>  
> -void qerror_report(const char *fmt, ...)
> +void qerror_report(ErrorClass eclass, const char *fmt, ...)
>  {
>      va_list va;
>      QError *qerror;
>  
>      va_start(va, fmt);
> -    qerror = qerror_from_info(fmt, &va);
> +    qerror = qerror_from_info(eclass, fmt, &va);
>      va_end(va);
>  
>      if (monitor_cur_is_qmp()) {
> @@ -540,6 +542,7 @@ struct Error
   /* Evil... */
   struct Error
>  {
>      QDict *obj;
>      char *msg;
> +    ErrorClass err_class;
>  };

Evil indeed.

>  
>  void qerror_report_err(Error *err)
> @@ -551,6 +554,7 @@ void qerror_report_err(Error *err)
>      QINCREF(err->obj);
>      qerr->error = err->obj;
>      qerr->err_msg = g_strdup(err->msg);
> +    qerr->err_class = err->err_class;
>  
>      if (monitor_cur_is_qmp()) {
>          monitor_set_error(cur_mon, qerr);
> diff --git a/qerror.h b/qerror.h
> index 2e6a49d..bcc93f8 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -16,9 +16,11 @@
>  #include "qstring.h"
>  #include "qemu-error.h"
>  #include "error.h"
> +#include "qapi-types.h"
>  #include <stdarg.h>
>  
>  typedef struct QErrorStringTable {
> +    ErrorClass err_class;
>      const char *error_fmt;
>      const char *desc;
>  } QErrorStringTable;
> @@ -28,10 +30,11 @@ typedef struct QError {
>      QDict *error;
>      Location loc;
>      char *err_msg;
> +    ErrorClass err_class;
>  } QError;
>  
>  QString *qerror_human(const QError *qerror);
> -void qerror_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> +void qerror_report(ErrorClass err_class, const char *fmt, ...) 
> GCC_FMT_ATTR(2, 3);
>  void qerror_report_err(Error *err);
>  void assert_no_error(Error *err);
>  char *qerror_format(const char *fmt, QDict *error);
> @@ -42,214 +45,214 @@ char *qerror_format(const char *fmt, QDict *error);
>   * Use scripts/check-qerror.sh to check.
>   */
>  #define QERR_ADD_CLIENT_FAILED \
> -    "{ 'class': 'AddClientFailed', 'data': {} }"
> +    -1, "{ 'class': 'AddClientFailed', 'data': {} }"

The QERR_ macros now expand into two arguments, which isn't nice.  But
it avoids a lot of churn.

I hope further clean up of the error reporting can get rid of the
macros.

[...]