Re: [Qemu-devel] [PATCH 01/14] qerror: add qerror_report_err()

[Anthony Liguori]

On 08/24/2011 03:15 PM, Luiz Capitulino wrote:
> On Wed, 24 Aug 2011 13:42:56 -0500
> Anthony Liguori<address@hidden>  wrote:
>
> > This provides a bridge between Error (new error mechanism) and QError (old error
> > mechanism).  Errors can be propagated whereas QError cannot.
> >
> > The minor evilness avoids layering violations.  Since QError should go away RSN,
> > it seems like a reasonable hack.
> >
> > Signed-off-by: Anthony Liguori<address@hidden>
> > ---
> >   qerror.c |   33 +++++++++++++++++++++++++++++++++
> >   qerror.h |    2 ++
> >   2 files changed, 35 insertions(+), 0 deletions(-)
> >
> > diff --git a/qerror.c b/qerror.c
> > index 3d64b80..fa647a6 100644
> > --- a/qerror.c
> > +++ b/qerror.c
> > @@ -478,6 +478,39 @@ void qerror_report_internal(const char *file, int linenr, 
> > const char *func,
> >       }
> >   }
> >
> > +/* Evil... */
> > +struct Error
> > +{
> > +    QDict *obj;
> > +    const char *fmt;
> > +    char *msg;
> > +};
>
> Given that we're in hack mode, I think I'd prefer to have struct Error in
> error.h and then include it here.

That adds a QObject dependency to error.h which cascades a bunch of 
dependencies.  I don't like this much either but I think it's the least 
of a few evils.

> > +
> > +void qerror_report_err(Error *err)
> > +{
> > +    QError *qerr;
> > +    int i;
> > +
> > +    qerr = qerror_new();
> > +    loc_save(&qerr->loc);
> > +    QINCREF(err->obj);
> > +    qerr->error = err->obj;
> > +
> > +    for (i = 0; qerror_table[i].error_fmt; i++) {
> > +        if (strcmp(qerror_table[i].error_fmt, err->fmt) == 0) {
> > +            qerr->entry =&qerror_table[i];
> > +            return;
>
> You have to drop this return, else the if clause below won't be
> executed. Or you could just use qerror_set_desc().


Thanks!

Regards,

Anthony Liguori

> > +        }
> > +    }
> > +
> > +    if (monitor_cur_is_qmp()) {
> > +        monitor_set_error(cur_mon, qerr);
> > +    } else {
> > +        qerror_print(qerr);
> > +        QDECREF(qerr);
> > +    }
> > +}
> > +
> >   /**
> >    * qobject_to_qerror(): Convert a QObject into a QError
> >    */
> > diff --git a/qerror.h b/qerror.h
> > index 8058456..4fe24aa 100644
> > --- a/qerror.h
> > +++ b/qerror.h
> > @@ -15,6 +15,7 @@
> >   #include "qdict.h"
> >   #include "qstring.h"
> >   #include "qemu-error.h"
> > +#include "error.h"
> >   #include<stdarg.h>
> >
> >   typedef struct QErrorStringTable {
> > @@ -39,6 +40,7 @@ QString *qerror_human(const QError *qerror);
> >   void qerror_print(QError *qerror);
> >   void qerror_report_internal(const char *file, int linenr, const char *func,
> >                               const char *fmt, ...) GCC_FMT_ATTR(4, 5);
> > +void qerror_report_err(Error *err);
> >   QString *qerror_format(const char *fmt, QDict *error);
> >   #define qerror_report(fmt, ...) \
> >       qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)