[Qemu-devel] [PATCH v2 13/18] json-parser: set an error if parsing returned NULL

[Marc-André Lureau]

Let's make json_parser_parse_err() suck less, and simplify caller
error handling.

 * monitor.c handle_qmp_command(): drop workaround

 * qga/main.c process_event(): improve error report, QERR_JSON_PARSING
   case is handled by json_parser_parse_err() now.

 * qobject/json-parser.c json_parser_parse(): Ignores the error.

 * qobject/qjson.c qobject_from_jsonv() via parse_json()
  - qobject_from_json()
    ~ block.c parse_json_filename() - removed workaround
    ~ block/rbd.c - abort (generated json - should never fail)
    ~ qapi/qobject-input-visitor.c - removed workaround
    ~ tests/check-qjson.c - assert or crash
    ~ tests/test-visitor-serialization.c - assert or crash

  - qobject_from_jsonf()
    Now dies in error_handle_fatal() instead of the assertion.
    Only used in tests, ok.

  - tests/test-qobject-input-visitor.c
  - tests/libqtest.c qmp_fd_sendv()
    Passes &error_abort, does nothing when qobject_from_jsonv() returns
    null.  The fix makes this catch invalid JSON instead of silently
    ignoring it.

Update the function documentation for possible return values.

Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---
 block.c                      |  5 -----
 monitor.c                    |  4 ----
 qapi/qobject-input-visitor.c |  5 -----
 qga/main.c                   |  2 +-
 qobject/json-parser.c        | 16 +++++++++++++++-
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/block.c b/block.c
index a2fe05ea96..42eaa8b7dc 100644
--- a/block.c
+++ b/block.c
@@ -1478,11 +1478,6 @@ static QDict *parse_json_filename(const char *filename, 
Error **errp)
 
     options_obj = qobject_from_json(filename, errp);
     if (!options_obj) {
-        /* Work around qobject_from_json() lossage TODO fix that */
-        if (errp && !*errp) {
-            error_setg(errp, "Could not parse the JSON options");
-            return NULL;
-        }
         error_prepend(errp, "Could not parse the JSON options: ");
         return NULL;
     }
diff --git a/monitor.c b/monitor.c
index 2abb3c2fc7..5a41a230b9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4114,10 +4114,6 @@ static void handle_qmp_command(JSONMessageParser 
*parser, GQueue *tokens)
     QMPRequest *req_obj;
 
     req = json_parser_parse(tokens, NULL, &err);
-    if (!req && !err) {
-        /* json_parser_parse() sucks: can fail without setting @err */
-        error_setg(&err, QERR_JSON_PARSING);
-    }
 
     qdict = qobject_to(QDict, req);
     if (qdict) {
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index da57f4cc24..3e88b27f9e 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -725,11 +725,6 @@ Visitor *qobject_input_visitor_new_str(const char *str,
     if (is_json) {
         obj = qobject_from_json(str, errp);
         if (!obj) {
-            /* Work around qobject_from_json() lossage TODO fix that */
-            if (errp && !*errp) {
-                error_setg(errp, "JSON parse error");
-                return NULL;
-            }
             return NULL;
         }
         args = qobject_to(QDict, obj);
diff --git a/qga/main.c b/qga/main.c
index 043f7c3ead..9032bb0c7a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -614,7 +614,7 @@ static void process_event(JSONMessageParser *parser, GQueue 
*tokens)
     }
     req = qobject_to(QDict, obj);
     if (!req) {
-        error_setg(&err, QERR_JSON_PARSING);
+        error_setg(&err, "Input must be a JSON object");
         goto err;
     }
     if (!qdict_haskey(req, "execute")) {
diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 0c0b478149..c3b0c216cf 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -24,6 +24,7 @@
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-lexer.h"
 #include "qapi/qmp/json-streamer.h"
+#include "qapi/qmp/qerror.h"
 
 typedef struct JSONParserContext
 {
@@ -548,6 +549,14 @@ static QObject *parse_value(JSONParserContext *ctxt, 
va_list *ap)
     }
 }
 
+/**
+ * json_parser_parse:
+ *
+ * If @tokens is null, return null.
+ * Else if @tokens parse okay, return the parse tree.
+ * Else set an error and return null.
+ *
+ **/
 QObject *json_parser_parse(GQueue *tokens, va_list *ap, Error **errp)
 {
     JSONParserContext ctxt = { .buf = tokens };
@@ -559,7 +568,12 @@ QObject *json_parser_parse(GQueue *tokens, va_list *ap, 
Error **errp)
 
     result = parse_value(&ctxt, ap);
 
-    error_propagate(errp, ctxt.err);
+    if (!result && !ctxt.err) {
+        /* TODO: improve error reporting */
+        error_setg(errp, QERR_JSON_PARSING);
+    } else {
+        error_propagate(errp, ctxt.err);
+    }
 
     g_queue_free_full(ctxt.buf, g_free);
     g_free(ctxt.current);
-- 
2.18.0.129.ge3331758f1