[Qemu-devel] [PATCH 07/12] json-parser: always set an error if return NULL

[Marc-André Lureau]

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

 * qga/main.c process_event() doesn't need further changes after
   previous cleanup.

 * 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 - abort, ok
    ~ tests/test-visitor-serialization.c - abort, ok

  - 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.

Signed-off-by: Marc-André Lureau <address@hidden>
---
 block.c                      | 5 -----
 monitor.c                    | 4 ----
 qapi/qobject-input-visitor.c | 5 -----
 qobject/json-parser.c        | 8 +++++++-
 4 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/block.c b/block.c
index ac8b3a3511..9046d66465 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 ec40a80d81..a3efe96d1d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4112,10 +4112,6 @@ static void handle_qmp_command(JSONMessageParser 
*parser, GQueue *tokens)
     QMPRequest *req_obj;
 
     req = json_parser_parse_err(tokens, NULL, &err);
-    if (!req && !err) {
-        /* json_parser_parse_err() 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/qobject/json-parser.c b/qobject/json-parser.c
index a5aa790d62..82c3167629 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
 {
@@ -591,7 +592,12 @@ QObject *json_parser_parse_err(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);
+    }
 
     parser_context_free(ctxt);
 
-- 
2.18.0.rc1