diff mbox series

[22/56] json: Report first rather than last parse error

Message ID 20180808120334.10970-23-armbru@redhat.com
State New
Headers show
Series json: Fixes, error reporting improvements, cleanups | expand

Commit Message

Markus Armbruster Aug. 8, 2018, 12:03 p.m. UTC 
Quiz time!  When a parser reports multiple errors, but the user gets
to see just one, which one is (on average) the least useful one?

Yes, you're right, it's the last one!  You're clearly familiar with
compilers.

Which one does QEMU report?

Right again, the last one!  You're clearly familiar with QEMU.

Reproducer: feeding

    {"abc\xC2ijk": 1}\n

to QMP produces

    {"error": {"class": "GenericError", "desc": "JSON parse error, key is not a string in object"}}

Report the first error instead.  The reproducer now produces

    {"error": {"class": "GenericError", "desc": "JSON parse error, invalid UTF-8 sequence in string"}}

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/json-parser.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Eric Blake Aug. 10, 2018, 3:25 p.m. UTC | #1 
On 08/08/2018 07:03 AM, Markus Armbruster wrote:
> Quiz time!  When a parser reports multiple errors, but the user gets
> to see just one, which one is (on average) the least useful one?

:)

> 
> Reproducer: feeding
> 
>      {"abc\xC2ijk": 1}\n
> 
> to QMP produces
> 
>      {"error": {"class": "GenericError", "desc": "JSON parse error, key is not a string in object"}}
> 
> Report the first error instead.  The reproducer now produces
> 
>      {"error": {"class": "GenericError", "desc": "JSON parse error, invalid UTF-8 sequence in string"}}

Yes, definite improvement.

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox series

Patch

diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 6b60b07e09..b3a95be3c8 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -54,13 +54,13 @@  static void GCC_FMT_ATTR(3, 4) parse_error(JSONParserContext *ctxt,
 {
     va_list ap;
     char message[1024];
+
+    if (ctxt->err) {
+        return;
+    }
     va_start(ap, msg);
     vsnprintf(message, sizeof(message), msg, ap);
     va_end(ap);
-    if (ctxt->err) {
-        error_free(ctxt->err);
-        ctxt->err = NULL;
-    }
     error_setg(&ctxt->err, "JSON parse error, %s", message);
 }