Patchwork [02/22] qerror: expose a function to format an error

login
register
mail settings
Submitter Anthony Liguori
Date March 7, 2011, 1:22 a.m.
Message ID <1299460984-15849-3-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/85629/
State New
Headers show

Comments

Anthony Liguori - March 7, 2011, 1:22 a.m.
This will let Error share the QError human formatting.  This is only used for
HMP.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Stefan Hajnoczi - March 7, 2011, 11:14 a.m.
On Mon, Mar 7, 2011 at 1:22 AM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> This will let Error share the QError human formatting.  This is only used for
> HMP.
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>
> diff --git a/qerror.c b/qerror.c
> index 4855604..13d53c9 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -326,12 +326,18 @@ QError *qerror_from_info(const char *file, int linenr, const char *func,
>     return qerr;
>  }
>
> -static void parse_error(const QError *qerror, int c)
> +static void parse_error(const QErrorStringTable *entry, int c)
>  {
> -    qerror_abort(qerror, "expected '%c' in '%s'", c, qerror->entry->desc);
> +#if 0
> +    qerror_abort(qerror, "expected '%c' in '%s'", c, entry->desc);
> +#else
> +    fprintf(stderr, "expected '%c' in '%s'", c, entry->desc);
> +    abort();
> +#endif

It is not obvious to me what these #if 0 are doing.  Was this just a
quick hack that needs to be fixed before merge?

Stefan
Anthony Liguori - March 7, 2011, 1:38 p.m.
On 03/07/2011 05:14 AM, Stefan Hajnoczi wrote:
> On Mon, Mar 7, 2011 at 1:22 AM, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>    
>> This will let Error share the QError human formatting.  This is only used for
>> HMP.
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>>
>> diff --git a/qerror.c b/qerror.c
>> index 4855604..13d53c9 100644
>> --- a/qerror.c
>> +++ b/qerror.c
>> @@ -326,12 +326,18 @@ QError *qerror_from_info(const char *file, int linenr, const char *func,
>>      return qerr;
>>   }
>>
>> -static void parse_error(const QError *qerror, int c)
>> +static void parse_error(const QErrorStringTable *entry, int c)
>>   {
>> -    qerror_abort(qerror, "expected '%c' in '%s'", c, qerror->entry->desc);
>> +#if 0
>> +    qerror_abort(qerror, "expected '%c' in '%s'", c, entry->desc);
>> +#else
>> +    fprintf(stderr, "expected '%c' in '%s'", c, entry->desc);
>> +    abort();
>> +#endif
>>      
> It is not obvious to me what these #if 0 are doing.  Was this just a
> quick hack that needs to be fixed before merge?
>    

I should just drop the #if 0 parts.

This code path should disappear once we finish the QAPI conversion.  The 
specific path only comes into play when a QError definition is incorrect 
which should only happen in development.

I didn't think it was worth trying to preserve the error messages as 
part of the code refactoring.

Regards,

Anthony Liguori

> Stefan
>
>

Patch

diff --git a/qerror.c b/qerror.c
index 4855604..13d53c9 100644
--- a/qerror.c
+++ b/qerror.c
@@ -326,12 +326,18 @@  QError *qerror_from_info(const char *file, int linenr, const char *func,
     return qerr;
 }
 
-static void parse_error(const QError *qerror, int c)
+static void parse_error(const QErrorStringTable *entry, int c)
 {
-    qerror_abort(qerror, "expected '%c' in '%s'", c, qerror->entry->desc);
+#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 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;
@@ -339,24 +345,31 @@  static const char *append_field(QString *outstr, const QError *qerror,
     QString *key_qs;
     const char *end, *key;
 
-    if (*start != '%')
-        parse_error(qerror, '%');
+    if (*start != '%') {
+        parse_error(entry, '%');
+    }
     start++;
-    if (*start != '(')
-        parse_error(qerror, '(');
+    if (*start != '(') {
+        parse_error(entry, '(');
+    }
     start++;
 
     end = strchr(start, ')');
-    if (!end)
-        parse_error(qerror, ')');
+    if (!end) {
+        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,41 +380,66 @@  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;
 }
 
+QString *qerror_format(const char *fmt, QDict *error)
+{
+    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);
+}
+
+/**
+ * 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)
+{
+    return qerror_format_desc(qerror->error, qerror->entry);
+}
+
 /**
  * qerror_print(): Print QError data
  *
diff --git a/qerror.h b/qerror.h
index f732d45..fd63ee9 100644
--- a/qerror.h
+++ b/qerror.h
@@ -42,6 +42,7 @@  void qerror_report_internal(const char *file, int linenr, const char *func,
 #define qerror_report(fmt, ...) \
     qerror_report_internal(__FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
 QError *qobject_to_qerror(const QObject *obj);
+QString *qerror_format(const char *fmt, QDict *error);
 
 /*
  * QError class list