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

[Anthony Liguori <anthony@codemonkey.ws>]

On 08/24/2011 03:15 PM, Luiz Capitulino wrote:
> On Wed, 24 Aug 2011 13:42:56 -0500
> Anthony Liguori<aliguori@us.ibm.com>  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<aliguori@us.ibm.com>
>> ---
>>   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__)
>
>