Patchwork [v1,04/23] qerror: refactor error to make the human message reusable

login
register
mail settings
Submitter Michael Roth
Date May 18, 2011, 12:51 a.m.
Message ID <1305679930-4215-5-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/96090/
State New
Headers show

Comments

Michael Roth - May 18, 2011, 12:51 a.m.
From: Anthony Liguori <aliguori@us.ibm.com>


Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile.objs   |    2 +-
 error.c         |    1 +
 qerror-report.c |  139 +++++++++++++++++++++++++++++++++++++++++++
 qerror.c        |  178 +++++++++++++++---------------------------------------
 qerror.h        |    7 ++
 5 files changed, 198 insertions(+), 129 deletions(-)
 create mode 100644 qerror-report.c
Stefan Hajnoczi - May 18, 2011, 8:09 a.m.
On Wed, May 18, 2011 at 1:51 AM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> +#if 0
> +    qerror_abort(qerror, "expected '%c' in '%s'", c, entry->desc);
> +#else
> +    fprintf(stderr, "expected '%c' in '%s'", c, entry->desc);
>     abort();
> +#endif

There are a couple of #if 0 in the patch, please resolve them.

Stefan
Luiz Capitulino - May 18, 2011, 1:54 p.m.
On Wed, 18 May 2011 09:09:28 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Wed, May 18, 2011 at 1:51 AM, Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > +#if 0
> > +    qerror_abort(qerror, "expected '%c' in '%s'", c, entry->desc);
> > +#else
> > +    fprintf(stderr, "expected '%c' in '%s'", c, entry->desc);
> >     abort();
> > +#endif
> 
> There are a couple of #if 0 in the patch, please resolve them.

Yes and this does more than the subject says.

Patch

diff --git a/Makefile.objs b/Makefile.objs
index 0803297..18917c4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -2,7 +2,7 @@ 
 # QObject
 qobject-obj-y = qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
 qobject-obj-y += qjson.o json-lexer.o json-streamer.o json-parser.o
-qobject-obj-y += qerror.o
+qobject-obj-y += qerror.o qerror-report.o
 
 #######################################################################
 # oslib-obj-y is code depending on the OS (win32 vs posix)
diff --git a/error.c b/error.c
index 3c5a740..ddc9105 100644
--- a/error.c
+++ b/error.c
@@ -14,6 +14,7 @@ 
 #include "qemu-objects.h"
 #include "qerror.h"
 #include <assert.h>
+#include <glib.h>
 
 struct Error
 {
diff --git a/qerror-report.c b/qerror-report.c
new file mode 100644
index 0000000..85773d8
--- /dev/null
+++ b/qerror-report.c
@@ -0,0 +1,139 @@ 
+/*
+ * QError Module
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ *
+ * Authors:
+ *  Luiz Capitulino <lcapitulino@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ */
+
+#include "qemu-common.h"
+#include "qerror.h"
+#include "monitor.h"
+#include "qjson.h"
+
+static void GCC_FMT_ATTR(2, 3) qerror_abort(const QError *qerr,
+                                            const char *fmt, ...)
+{
+    va_list ap;
+
+    fprintf(stderr, "qerror: bad call in function '%s':\n", qerr->func);
+    fprintf(stderr, "qerror: -> ");
+
+    va_start(ap, fmt);
+    vfprintf(stderr, fmt, ap);
+    va_end(ap);
+
+    fprintf(stderr, "\nqerror: call at %s:%d\n", qerr->file, qerr->linenr);
+    abort();
+}
+
+static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
+                                               const char *fmt, va_list *va)
+{
+    QObject *obj;
+
+    obj = qobject_from_jsonv(fmt, va);
+    if (!obj) {
+        qerror_abort(qerr, "invalid format '%s'", fmt);
+    }
+    if (qobject_type(obj) != QTYPE_QDICT) {
+        qerror_abort(qerr, "error format is not a QDict '%s'", fmt);
+    }
+
+    qerr->error = qobject_to_qdict(obj);
+
+    obj = qdict_get(qerr->error, "class");
+    if (!obj) {
+        qerror_abort(qerr, "missing 'class' key in '%s'", fmt);
+    }
+    if (qobject_type(obj) != QTYPE_QSTRING) {
+        qerror_abort(qerr, "'class' key value should be a QString");
+    }
+
+    obj = qdict_get(qerr->error, "data");
+    if (!obj) {
+        qerror_abort(qerr, "missing 'data' key in '%s'", fmt);
+    }
+    if (qobject_type(obj) != QTYPE_QDICT) {
+        qerror_abort(qerr, "'data' key value should be a QDICT");
+    }
+}
+
+/**
+ * qerror_from_info(): Create a new QError from error information
+ *
+ * The information consists of:
+ *
+ * - file   the file name of where the error occurred
+ * - linenr the line number of where the error occurred
+ * - func   the function name of where the error occurred
+ * - fmt    JSON printf-like dictionary, there must exist keys 'class' and
+ *          'data'
+ * - va     va_list of all arguments specified by fmt
+ *
+ * Return strong reference.
+ */
+QError *qerror_from_info(const char *file, int linenr, const char *func,
+                         const char *fmt, va_list *va)
+{
+    QError *qerr;
+
+    qerr = qerror_new();
+    loc_save(&qerr->loc);
+    qerr->linenr = linenr;
+    qerr->file = file;
+    qerr->func = func;
+
+    if (!fmt) {
+        qerror_abort(qerr, "QDict not specified");
+    }
+
+    qerror_set_data(qerr, fmt, va);
+    qerror_set_desc(qerr, fmt);
+
+    return qerr;
+}
+
+/**
+ * qerror_print(): Print QError data
+ *
+ * This function will print the member 'desc' of the specified QError object,
+ * it uses error_report() for this, so that the output is routed to the right
+ * place (ie. stderr or Monitor's device).
+ */
+void qerror_print(QError *qerror)
+{
+    QString *qstring = qerror_human(qerror);
+    loc_push_restore(&qerror->loc);
+    error_report("%s", qstring_get_str(qstring));
+    loc_pop(&qerror->loc);
+    QDECREF(qstring);
+}
+
+void qerror_report_internal(const char *file, int linenr, const char *func,
+                            const char *fmt, ...)
+{
+    va_list va;
+    QError *qerror;
+
+    va_start(va, fmt);
+    qerror = qerror_from_info(file, linenr, func, fmt, &va);
+    va_end(va);
+
+    if (monitor_cur_is_qmp()) {
+        monitor_set_error(cur_mon, qerror);
+    } else {
+        qerror_print(qerror);
+        QDECREF(qerror);
+    }
+}
+
+void qerror_report_err(Error *err)
+{
+    qerror_report(QERR_UNDEFINED_ERROR);
+}
+
diff --git a/qerror.c b/qerror.c
index 4855604..49bed93 100644
--- a/qerror.c
+++ b/qerror.c
@@ -227,55 +227,17 @@  QError *qerror_new(void)
     return qerr;
 }
 
-static void GCC_FMT_ATTR(2, 3) qerror_abort(const QError *qerr,
-                                            const char *fmt, ...)
+static void parse_error(const QErrorStringTable *entry, int c)
 {
-    va_list ap;
-
-    fprintf(stderr, "qerror: bad call in function '%s':\n", qerr->func);
-    fprintf(stderr, "qerror: -> ");
-
-    va_start(ap, fmt);
-    vfprintf(stderr, fmt, ap);
-    va_end(ap);
-
-    fprintf(stderr, "\nqerror: call at %s:%d\n", qerr->file, qerr->linenr);
+#if 0
+    qerror_abort(qerror, "expected '%c' in '%s'", c, entry->desc);
+#else
+    fprintf(stderr, "expected '%c' in '%s'", c, entry->desc);
     abort();
+#endif
 }
 
-static void GCC_FMT_ATTR(2, 0) qerror_set_data(QError *qerr,
-                                               const char *fmt, va_list *va)
-{
-    QObject *obj;
-
-    obj = qobject_from_jsonv(fmt, va);
-    if (!obj) {
-        qerror_abort(qerr, "invalid format '%s'", fmt);
-    }
-    if (qobject_type(obj) != QTYPE_QDICT) {
-        qerror_abort(qerr, "error format is not a QDict '%s'", fmt);
-    }
-
-    qerr->error = qobject_to_qdict(obj);
-
-    obj = qdict_get(qerr->error, "class");
-    if (!obj) {
-        qerror_abort(qerr, "missing 'class' key in '%s'", fmt);
-    }
-    if (qobject_type(obj) != QTYPE_QSTRING) {
-        qerror_abort(qerr, "'class' key value should be a QString");
-    }
-    
-    obj = qdict_get(qerr->error, "data");
-    if (!obj) {
-        qerror_abort(qerr, "missing 'data' key in '%s'", fmt);
-    }
-    if (qobject_type(obj) != QTYPE_QDICT) {
-        qerror_abort(qerr, "'data' key value should be a QDICT");
-    }
-}
-
-static void qerror_set_desc(QError *qerr, const char *fmt)
+void qerror_set_desc(QError *qerr, const char *fmt)
 {
     int i;
 
@@ -288,50 +250,15 @@  static void qerror_set_desc(QError *qerr, const char *fmt)
         }
     }
 
+#if 0
     qerror_abort(qerr, "error format '%s' not found", fmt);
+#else
+    abort();
+#endif
 }
 
-/**
- * qerror_from_info(): Create a new QError from error information
- *
- * The information consists of:
- *
- * - file   the file name of where the error occurred
- * - linenr the line number of where the error occurred
- * - func   the function name of where the error occurred
- * - fmt    JSON printf-like dictionary, there must exist keys 'class' and
- *          'data'
- * - va     va_list of all arguments specified by fmt
- *
- * Return strong reference.
- */
-QError *qerror_from_info(const char *file, int linenr, const char *func,
-                         const char *fmt, va_list *va)
-{
-    QError *qerr;
-
-    qerr = qerror_new();
-    loc_save(&qerr->loc);
-    qerr->linenr = linenr;
-    qerr->file = file;
-    qerr->func = func;
-
-    if (!fmt) {
-        qerror_abort(qerr, "QDict not specified");
-    }
-
-    qerror_set_data(qerr, fmt, va);
-    qerror_set_desc(qerr, fmt);
-
-    return qerr;
-}
-
-static void parse_error(const QError *qerror, int c)
-{
-    qerror_abort(qerror, "expected '%c' in '%s'", c, qerror->entry->desc);
-}
-
-static const char *append_field(QString *outstr, const QError *qerror,
+static const char *append_field(QDict *error, QString *outstr,
+                                const QErrorStringTable *entry,
                                 const char *start)
 {
     QObject *obj;
@@ -340,23 +267,27 @@  static const char *append_field(QString *outstr, const QError *qerror,
     const char *end, *key;
 
     if (*start != '%')
-        parse_error(qerror, '%');
+        parse_error(entry, '%');
     start++;
     if (*start != '(')
-        parse_error(qerror, '(');
+        parse_error(entry, '(');
     start++;
 
     end = strchr(start, ')');
     if (!end)
-        parse_error(qerror, ')');
+        parse_error(entry, ')');
 
     key_qs = qstring_from_substr(start, 0, end - start - 1);
     key = qstring_get_str(key_qs);
 
-    qdict = qobject_to_qdict(qdict_get(qerror->error, "data"));
+    qdict = qobject_to_qdict(qdict_get(error, "data"));
     obj = qdict_get(qdict, key);
     if (!obj) {
+#if 0
         qerror_abort(qerror, "key '%s' not found in QDict", key);
+#else
+        abort();
+#endif
     }
 
     switch (qobject_type(obj)) {
@@ -367,73 +298,64 @@  static const char *append_field(QString *outstr, const QError *qerror,
             qstring_append_int(outstr, qdict_get_int(qdict, key));
             break;
         default:
+#if 0
             qerror_abort(qerror, "invalid type '%c'", qobject_type(obj));
+#else
+            abort();
+#endif
     }
 
     QDECREF(key_qs);
     return ++end;
 }
 
-/**
- * qerror_human(): Format QError data into human-readable string.
- *
- * Formats according to member 'desc' of the specified QError object.
- */
-QString *qerror_human(const QError *qerror)
+static QString *qerror_format_desc(QDict *error,
+                                   const QErrorStringTable *entry)
 {
-    const char *p;
     QString *qstring;
+    const char *p;
 
-    assert(qerror->entry != NULL);
+    assert(entry != NULL);
 
     qstring = qstring_new();
 
-    for (p = qerror->entry->desc; *p != '\0';) {
+    for (p = entry->desc; *p != '\0';) {
         if (*p != '%') {
             qstring_append_chr(qstring, *p++);
         } else if (*(p + 1) == '%') {
             qstring_append_chr(qstring, '%');
             p += 2;
         } else {
-            p = append_field(qstring, qerror, p);
+            p = append_field(error, qstring, entry, p);
         }
     }
 
     return qstring;
 }
 
-/**
- * qerror_print(): Print QError data
- *
- * This function will print the member 'desc' of the specified QError object,
- * it uses error_report() for this, so that the output is routed to the right
- * place (ie. stderr or Monitor's device).
- */
-void qerror_print(QError *qerror)
+QString *qerror_format(const char *fmt, QDict *error)
 {
-    QString *qstring = qerror_human(qerror);
-    loc_push_restore(&qerror->loc);
-    error_report("%s", qstring_get_str(qstring));
-    loc_pop(&qerror->loc);
-    QDECREF(qstring);
+    const QErrorStringTable *entry = NULL;
+    int i;
+
+    for (i = 0; qerror_table[i].error_fmt; i++) {
+        if (strcmp(qerror_table[i].error_fmt, fmt) == 0) {
+            entry = &qerror_table[i];
+            break;
+        }
+    }
+
+    return qerror_format_desc(error, entry);
 }
 
-void qerror_report_internal(const char *file, int linenr, const char *func,
-                            const char *fmt, ...)
+/**
+ * qerror_human(): Format QError data into human-readable string.
+ *
+ * Formats according to member 'desc' of the specified QError object.
+ */
+QString *qerror_human(const QError *qerror)
 {
-    va_list va;
-    QError *qerror;
-
-    va_start(va, fmt);
-    qerror = qerror_from_info(file, linenr, func, fmt, &va);
-    va_end(va);
-
-    if (monitor_cur_is_qmp()) {
-        monitor_set_error(cur_mon, qerror);
-    } else {
-        qerror_print(qerror);
-        QDECREF(qerror);
-    }
+    return qerror_format_desc(qerror->error, qerror->entry);
 }
 
 /**
diff --git a/qerror.h b/qerror.h
index df61d2c..09ad7dc 100644
--- a/qerror.h
+++ b/qerror.h
@@ -16,6 +16,7 @@ 
 #include "qstring.h"
 #include "qemu-error.h"
 #include <stdarg.h>
+#include "error.h"
 
 typedef struct QErrorStringTable {
     const char *desc;
@@ -41,7 +42,10 @@  void qerror_report_internal(const char *file, int linenr, const char *func,
                             const char *fmt, ...) GCC_FMT_ATTR(4, 5);
 #define qerror_report(fmt, ...) \
     qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
+void qerror_report_err(Error *err);
 QError *qobject_to_qerror(const QObject *obj);
+QString *qerror_format(const char *fmt, QDict *error);
+void qerror_set_desc(QError *qerr, const char *fmt);
 
 /*
  * QError class list
@@ -120,6 +124,9 @@  QError *qobject_to_qerror(const QObject *obj);
 #define QERR_JSON_PARSING \
     "{ 'class': 'JSONParsing', 'data': {} }"
 
+#define QERR_JSON_PARSE_ERROR \
+        "{ 'class': 'JSONParseError', 'data': { 'message': %s } }"
+
 #define QERR_KVM_MISSING_CAP \
     "{ 'class': 'KVMMissingCap', 'data': { 'capability': %s, 'feature': %s } }"