[01/14] qerror: add qerror_report_err()

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(-)

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.

> +
> +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().

> +        }
> +    }
> +
> +    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__)

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__)
>
>