[PATCH] tests: add test for json-streamer.c error recovery

[PATCH] tests: add test for json-streamer.c error recovery

[Paolo Bonzini]

Before rewriting the error recovery code to work in a push parsing
setup, make sure that we have tests for it.

Cover various cases of invalid JSON, to check that structural
recovery based on balanced brackets and braces works; and
lexer-based recovery which documents "\f" as a sure fire
way to reset the lexer.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tests/unit/check-json-parser.c | 145 +++++++++++++++++++++++++++++++++
 tests/unit/meson.build         |   1 +
 2 files changed, 146 insertions(+)
 create mode 100644 tests/unit/check-json-parser.c

diff --git a/tests/unit/check-json-parser.c b/tests/unit/check-json-parser.c
new file mode 100644
index 00000000000..ca2d5f41097
--- /dev/null
+++ b/tests/unit/check-json-parser.c
@@ -0,0 +1,145 @@
+/*
+ * Unit tests for JSON Parser error recovery
+ *
+ * Copyright 2026 Red Hat
+ * Author: Paolo Bonzini <pbonzini@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/osdep.h"
+
+#include "qapi/error.h"
+#include "qobject/qbool.h"
+#include "qobject/json-parser.h"
+
+typedef struct ParseResult {
+    int errors;
+    QObject *result;
+} ParseResult;
+
+static void parse_emit(void *opaque, QObject *json, Error *err)
+{
+    ParseResult *r = opaque;
+
+    if (err) {
+        r->errors++;
+        error_free(err);
+    } else {
+        qobject_unref(r->result);
+        r->result = json;
+    }
+}
+
+static ParseResult do_parse(const char *input)
+{
+    ParseResult r = { 0, NULL };
+    JSONMessageParser parser;
+
+    json_message_parser_init(&parser, parse_emit, &r, NULL);
+    json_message_parser_feed(&parser, input, strlen(input));
+    json_message_parser_flush(&parser);
+    json_message_parser_destroy(&parser);
+    return r;
+}
+
+static void check_result(const char *input, int expected_errors, QType expected_type)
+{
+    ParseResult r = do_parse(input);
+
+    g_assert_cmpint(r.errors, ==, expected_errors);
+    g_assert_nonnull(r.result);
+    g_assert_cmpint(qobject_type(r.result), ==, expected_type);
+    qobject_unref(r.result);
+}
+
+static void check_result_error(const char *input, int expected_errors)
+{
+    ParseResult r = do_parse(input);
+
+    g_assert_cmpint(r.errors, ==, expected_errors);
+    g_assert_null(r.result);
+}
+
+static void test_simple(void)
+{
+    check_result("false", 0, QTYPE_QBOOL);
+}
+
+static void test_whitespace(void)
+{
+    check_result(" false", 0, QTYPE_QBOOL);
+}
+
+static void test_extra_closing_braces(void)
+{
+    check_result("}}false", 2, QTYPE_QBOOL);
+}
+
+static void test_bad_dict(void)
+{
+    check_result("{ 'abc' }false", 1, QTYPE_QBOOL);
+}
+
+static void test_trailing_comma(void)
+{
+    check_result("[ 'abc', ]false", 1, QTYPE_QBOOL);
+}
+
+static void test_lexer_recovery(void)
+{
+    check_result("\f{}", 1, QTYPE_QDICT);
+    check_result("\f[]", 1, QTYPE_QLIST);
+    check_result("\f:false", 2, QTYPE_QBOOL);
+    check_result("\f,false", 2, QTYPE_QBOOL);
+
+    /*
+     * alphabetic characters do not start a new parsing, this is slightly weird
+     * but it keeps the lexer simple and works well for QMP (where valid input
+     * is a sequence of dictionaries)
+     */
+    check_result_error("\ffalse", 1);
+    check_result_error("\f'str'", 1);
+    check_result_error("\f\"str\"", 1);
+}
+
+static void test_lexer_recovery_nested(void)
+{
+    check_result("{[{\f{}", 1, QTYPE_QDICT);
+    check_result("{[{\f[]", 1, QTYPE_QLIST);
+    check_result("{[{\f:false", 2, QTYPE_QBOOL);
+    check_result("{[{\f,false", 2, QTYPE_QBOOL);
+
+    /* same as test_lexer_recovery */
+    check_result_error("{[{\ffalse", 1);
+    check_result_error("{[{\f'str'", 1);
+    check_result_error("{[{\f\"str\"", 1);
+}
+
+static void test_nested(void)
+{
+    check_result("[{'a']}false", 1, QTYPE_QBOOL);
+}
+
+static void test_nested_multiple(void)
+{
+    check_result("[{'a']}[{'a']}false", 2, QTYPE_QBOOL);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    g_test_add_func("/json-parser/simple", test_simple);
+    g_test_add_func("/json-parser/whitespace", test_whitespace);
+    g_test_add_func("/json-parser/error-recovery/extra-closing-braces", test_extra_closing_braces);
+    g_test_add_func("/json-parser/error-recovery/bad-dict", test_bad_dict);
+    g_test_add_func("/json-parser/error-recovery/trailing-comma", test_trailing_comma);
+    g_test_add_func("/json-parser/error-recovery/lexer", test_lexer_recovery);
+    g_test_add_func("/json-parser/error-recovery/lexer/nested", test_lexer_recovery_nested);
+    g_test_add_func("/json-parser/error-recovery/nested", test_nested);
+    g_test_add_func("/json-parser/error-recovery/nested/multiple", test_nested_multiple);
+
+    return g_test_run();
+}
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 41e8b06c339..03d36748c73 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -10,6 +10,7 @@ tests = {
   'check-qnull': [],
   'check-qobject': [],
   'check-qjson': [],
+  'check-json-parser': [],
   'check-qlit': [],
   'test-error-report': [],
   'test-qobject-output-visitor': [testqapi],
-- 
2.53.0

Re: [PATCH] tests: add test for json-streamer.c error recovery

[Markus Armbruster]

My apologies for the delay.

Paolo Bonzini <pbonzini@redhat.com> writes:

> Before rewriting the error recovery code to work in a push parsing
> setup, make sure that we have tests for it.
>
> Cover various cases of invalid JSON, to check that structural
> recovery based on balanced brackets and braces works; and
> lexer-based recovery which documents "\f" as a sure fire
> way to reset the lexer.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

How is this related to tests/unit/check-qjson.c?

Re: [PATCH] tests: add test for json-streamer.c error recovery

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

Il lun 20 apr 2026, 12:53 Markus Armbruster <armbru@redhat.com> ha scritto:

> > Before rewriting the error recovery code to work in a push parsing
> > setup, make sure that we have tests for it.
> >
> > Cover various cases of invalid JSON, to check that structural
> > recovery based on balanced brackets and braces works; and
> > lexer-based recovery which documents "\f" as a sure fire
> > way to reset the lexer.
>
> How is this related to tests/unit/check-qjson.c?
>

It tests at a lower level, talking directly to the json-streamer.c API.
check-qjson.c only uses qobject_from_json() so it never tests more than one
(attempted) JSON object in the input.

Paolo


>

[-- Attachment #2: Type: text/html, Size: 1373 bytes --]

Re: [PATCH] tests: add test for json-streamer.c error recovery

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il lun 20 apr 2026, 12:53 Markus Armbruster <armbru@redhat.com> ha scritto:
>
>> > Before rewriting the error recovery code to work in a push parsing
>> > setup, make sure that we have tests for it.
>> >
>> > Cover various cases of invalid JSON, to check that structural
>> > recovery based on balanced brackets and braces works; and
>> > lexer-based recovery which documents "\f" as a sure fire
>> > way to reset the lexer.
>>
>> How is this related to tests/unit/check-qjson.c?
>>
>
> It tests at a lower level, talking directly to the json-streamer.c API.
> check-qjson.c only uses qobject_from_json() so it never tests more than one
> (attempted) JSON object in the input.

I see.

There's overlap between the two.  More on that in the review I'm about
to send.

Thanks!

Re: [PATCH] tests: add test for json-streamer.c error recovery

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Before rewriting the error recovery code to work in a push parsing
> setup, make sure that we have tests for it.
>
> Cover various cases of invalid JSON, to check that structural
> recovery based on balanced brackets and braces works; and
> lexer-based recovery which documents "\f" as a sure fire
> way to reset the lexer.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/unit/check-json-parser.c | 145 +++++++++++++++++++++++++++++++++
>  tests/unit/meson.build         |   1 +
>  2 files changed, 146 insertions(+)
>  create mode 100644 tests/unit/check-json-parser.c
>
> diff --git a/tests/unit/check-json-parser.c b/tests/unit/check-json-parser.c
> new file mode 100644
> index 00000000000..ca2d5f41097
> --- /dev/null
> +++ b/tests/unit/check-json-parser.c
> @@ -0,0 +1,145 @@
> +/*
> + * Unit tests for JSON Parser error recovery
> + *
> + * Copyright 2026 Red Hat
> + * Author: Paolo Bonzini <pbonzini@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/osdep.h"
> +
> +#include "qapi/error.h"
> +#include "qobject/qbool.h"
> +#include "qobject/json-parser.h"
> +
> +typedef struct ParseResult {
> +    int errors;
> +    QObject *result;
> +} ParseResult;
> +
> +static void parse_emit(void *opaque, QObject *json, Error *err)
> +{
> +    ParseResult *r = opaque;
> +

Recommend

       assert(!json != !err);

> +    if (err) {
> +        r->errors++;
> +        error_free(err);

ParseResult.errors counts errors, and ...

> +    } else {
> +        qobject_unref(r->result);
> +        r->result = json;

.result is the last value successfully parsed.

Suggest to comment struct ParseResult accordingly.

> +    }
> +}
> +
> +static ParseResult do_parse(const char *input)
> +{
> +    ParseResult r = { 0, NULL };
> +    JSONMessageParser parser;
> +
> +    json_message_parser_init(&parser, parse_emit, &r, NULL);
> +    json_message_parser_feed(&parser, input, strlen(input));
> +    json_message_parser_flush(&parser);
> +    json_message_parser_destroy(&parser);
> +    return r;
> +}

This is similar to qobject_from_json(), but geared towards testing error
recovery:

* qobject_from_json() treats multiple JSON values as error, do_parse()
  returns the last one.

* qobject_from_json() returns the last error, do_parse() returns the
  number of errors.

Okay.

> +
> +static void check_result(const char *input, int expected_errors, QType expected_type)

Break the line after the second parameter, please.

> +{
> +    ParseResult r = do_parse(input);
> +
> +    g_assert_cmpint(r.errors, ==, expected_errors);
> +    g_assert_nonnull(r.result);
> +    g_assert_cmpint(qobject_type(r.result), ==, expected_type);
> +    qobject_unref(r.result);
> +}
> +
> +static void check_result_error(const char *input, int expected_errors)
> +{
> +    ParseResult r = do_parse(input);
> +
> +    g_assert_cmpint(r.errors, ==, expected_errors);
> +    g_assert_null(r.result);
> +}
> +
> +static void test_simple(void)
> +{
> +    check_result("false", 0, QTYPE_QBOOL);
> +}

This one isn't about error recovery.  It's arguably redundant with
check-qjson.c's keyword_literal().

> +
> +static void test_whitespace(void)
> +{
> +    check_result(" false", 0, QTYPE_QBOOL);
> +}

Also not about error recovery.  Sort of redundant with check-qjson.c's
simple_whitespace().

> +
> +static void test_extra_closing_braces(void)
> +{
> +    check_result("}}false", 2, QTYPE_QBOOL);
> +}

Overlap with check-qjson.c's multiple_values().

> +
> +static void test_bad_dict(void)
> +{
> +    check_result("{ 'abc' }false", 1, QTYPE_QBOOL);
> +}

Interesting: just one error, because error recovery eats false.

> +
> +static void test_trailing_comma(void)
> +{
> +    check_result("[ 'abc', ]false", 1, QTYPE_QBOOL);
> +}

Likewise.

> +
> +static void test_lexer_recovery(void)
> +{
> +    check_result("\f{}", 1, QTYPE_QDICT);
> +    check_result("\f[]", 1, QTYPE_QLIST);
> +    check_result("\f:false", 2, QTYPE_QBOOL);
> +    check_result("\f,false", 2, QTYPE_QBOOL);
> +
> +    /*
> +     * alphabetic characters do not start a new parsing, this is slightly weird
> +     * but it keeps the lexer simple and works well for QMP (where valid input
> +     * is a sequence of dictionaries)
> +     */

Please wrap comment lines a bit earlier for legibility:

       /*
        * Alphabetic characters do not start a new parsing.  This is
        * slightly weird but it keeps the lexer simple and works well for
        * QMP (where valid input is a sequence of dictionaries).
        */

> +    check_result_error("\ffalse", 1);
> +    check_result_error("\f'str'", 1);
> +    check_result_error("\f\"str\"", 1);
> +}
> +
> +static void test_lexer_recovery_nested(void)
> +{
> +    check_result("{[{\f{}", 1, QTYPE_QDICT);
> +    check_result("{[{\f[]", 1, QTYPE_QLIST);
> +    check_result("{[{\f:false", 2, QTYPE_QBOOL);
> +    check_result("{[{\f,false", 2, QTYPE_QBOOL);
> +
> +    /* same as test_lexer_recovery */

Really?

> +    check_result_error("{[{\ffalse", 1);
> +    check_result_error("{[{\f'str'", 1);
> +    check_result_error("{[{\f\"str\"", 1);
> +}
> +
> +static void test_nested(void)
> +{
> +    check_result("[{'a']}false", 1, QTYPE_QBOOL);
> +}
> +
> +static void test_nested_multiple(void)
> +{
> +    check_result("[{'a']}[{'a']}false", 2, QTYPE_QBOOL);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +
> +    g_test_add_func("/json-parser/simple", test_simple);
> +    g_test_add_func("/json-parser/whitespace", test_whitespace);
> +    g_test_add_func("/json-parser/error-recovery/extra-closing-braces", test_extra_closing_braces);
> +    g_test_add_func("/json-parser/error-recovery/bad-dict", test_bad_dict);
> +    g_test_add_func("/json-parser/error-recovery/trailing-comma", test_trailing_comma);
> +    g_test_add_func("/json-parser/error-recovery/lexer", test_lexer_recovery);
> +    g_test_add_func("/json-parser/error-recovery/lexer/nested", test_lexer_recovery_nested);
> +    g_test_add_func("/json-parser/error-recovery/nested", test_nested);
> +    g_test_add_func("/json-parser/error-recovery/nested/multiple", test_nested_multiple);
> +
> +    return g_test_run();
> +}
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index 41e8b06c339..03d36748c73 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -10,6 +10,7 @@ tests = {
>    'check-qnull': [],
>    'check-qobject': [],
>    'check-qjson': [],
> +  'check-json-parser': [],
>    'check-qlit': [],
>    'test-error-report': [],
>    'test-qobject-output-visitor': [testqapi],

check-json-parser.c and check-qjson.c both test the JSON parser.

check-json-parser.c tests at the json-parser.h level, and focuses on
error recovery.

check-qjson.c tests one level up at the qjson.h level, where error
recovery is not testable.

Perhaps the JSON parser tests should all be in one place.  This is not a
demand :)

Re: [PATCH] tests: add test for json-streamer.c error recovery

[Paolo Bonzini]

On Tue, Apr 21, 2026 at 12:29 PM Markus Armbruster <armbru@redhat.com> wrote:
> > +static void parse_emit(void *opaque, QObject *json, Error *err)
> > +{
> > +    ParseResult *r = opaque;
> > +
>
> Recommend
>
>        assert(!json != !err);

Makes sense.

> > +static void test_simple(void)
> > +{
> > +    check_result("false", 0, QTYPE_QBOOL);
> > +}
>
> This one isn't about error recovery.  It's arguably redundant with
> check-qjson.c's keyword_literal().

Or the other way round. :)  They test different APIs which makes both
of them worthwhile.

> > +
> > +static void test_extra_closing_braces(void)
> > +{
> > +    check_result("}}false", 2, QTYPE_QBOOL);
> > +}
>
> Overlap with check-qjson.c's multiple_values().

Overlapping input but completely different test, because this checks
for success (after error) whereas multiple_values() checks for an
error condition.  This also shows the difference between the two test
files and APIs.

> > +
> > +static void test_bad_dict(void)
> > +{
> > +    check_result("{ 'abc' }false", 1, QTYPE_QBOOL);
> > +}
>
> Interesting: just one error, because error recovery eats false.

Why would it eat false here? The error is recovered after "{ 'abc' }"
and then "false" is parsed successfully. One error and one successful
value, as indicated by the arguments to check_result().

> > +
> > +static void test_lexer_recovery(void)
> > +{
> > +    check_result("\f{}", 1, QTYPE_QDICT);
> > +    check_result("\f[]", 1, QTYPE_QLIST);
> > +    check_result("\f:false", 2, QTYPE_QBOOL);
> > +    check_result("\f,false", 2, QTYPE_QBOOL);
> > +
> > +    /*
> > +     * alphabetic characters do not start a new parsing, this is slightly weird
> > +     * but it keeps the lexer simple and works well for QMP (where valid input
> > +     * is a sequence of dictionaries)
> > +     */
>
> Please wrap comment lines a bit earlier for legibility:

79 characters are Just Fine.  While I appreciate careful reviews, and
I will apply your suggested edit, this is perhaps a bit too much?

> > +    /* same as test_lexer_recovery */
> > +    check_result_error("{[{\ffalse", 1);
>
> Really?

Same comment about alphabetic characters not starting a new parsing after \f.

> check-json-parser.c and check-qjson.c both test the JSON parser.
>
> check-json-parser.c tests at the json-parser.h level, and focuses on
> error recovery.

That's what I focused on _for now_, because that's where the push
parser changes are more tricky, but it doesn't have to be that way.

> check-qjson.c tests one level up at the qjson.h level, where error
> recovery is not testable.
>
> Perhaps the JSON parser tests should all be in one place.  This is not a
> demand :)

Objection, your honor - different APIs, different test files. :)

Paolo

Re: [PATCH] tests: add test for json-streamer.c error recovery

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On Tue, Apr 21, 2026 at 12:29 PM Markus Armbruster <armbru@redhat.com> wrote:
>> > +static void parse_emit(void *opaque, QObject *json, Error *err)
>> > +{
>> > +    ParseResult *r = opaque;
>> > +
>>
>> Recommend
>>
>>        assert(!json != !err);
>
> Makes sense.
>
>> > +static void test_simple(void)
>> > +{
>> > +    check_result("false", 0, QTYPE_QBOOL);
>> > +}
>>
>> This one isn't about error recovery.  It's arguably redundant with
>> check-qjson.c's keyword_literal().
>
> Or the other way round. :)  They test different APIs which makes both
> of them worthwhile.

See discussion at the end.

>> > +
>> > +static void test_extra_closing_braces(void)
>> > +{
>> > +    check_result("}}false", 2, QTYPE_QBOOL);
>> > +}
>>
>> Overlap with check-qjson.c's multiple_values().
>
> Overlapping input but completely different test, because this checks
> for success (after error) whereas multiple_values() checks for an
> error condition.  This also shows the difference between the two test
> files and APIs.
>
>> > +
>> > +static void test_bad_dict(void)
>> > +{
>> > +    check_result("{ 'abc' }false", 1, QTYPE_QBOOL);
>> > +}
>>
>> Interesting: just one error, because error recovery eats false.
>
> Why would it eat false here? The error is recovered after "{ 'abc' }"
> and then "false" is parsed successfully. One error and one successful
> value, as indicated by the arguments to check_result().

I got confused %-}

>> > +
>> > +static void test_lexer_recovery(void)
>> > +{
>> > +    check_result("\f{}", 1, QTYPE_QDICT);
>> > +    check_result("\f[]", 1, QTYPE_QLIST);
>> > +    check_result("\f:false", 2, QTYPE_QBOOL);
>> > +    check_result("\f,false", 2, QTYPE_QBOOL);
>> > +
>> > +    /*
>> > +     * alphabetic characters do not start a new parsing, this is slightly weird
>> > +     * but it keeps the lexer simple and works well for QMP (where valid input
>> > +     * is a sequence of dictionaries)
>> > +     */
>>
>> Please wrap comment lines a bit earlier for legibility:
>
> 79 characters are Just Fine.  While I appreciate careful reviews, and
> I will apply your suggested edit, this is perhaps a bit too much?

I asked nicely, because I do have real trouble reading long lines.  It's
not the right margin, it's the width of the text not counting
indentation.

Peace?

>> > +    /* same as test_lexer_recovery */
>> > +    check_result_error("{[{\ffalse", 1);
>>
>> Really?
>
> Same comment about alphabetic characters not starting a new parsing after \f.
>
>> check-json-parser.c and check-qjson.c both test the JSON parser.
>>
>> check-json-parser.c tests at the json-parser.h level, and focuses on
>> error recovery.
>
> That's what I focused on _for now_, because that's where the push
> parser changes are more tricky, but it doesn't have to be that way.
>
>> check-qjson.c tests one level up at the qjson.h level, where error
>> recovery is not testable.
>>
>> Perhaps the JSON parser tests should all be in one place.  This is not a
>> demand :)
>
> Objection, your honor - different APIs, different test files. :)

Separate APIs, yes, but one wraps around the other in a straightforward
manner.

I don't mind having a separate tests for both levels.

Ideally, the test at the lower level would be complete, and the test at
the upper level focused on what's added there.

As is, much functionality is only covered at the upper level.  Some is
covered at both.

I wanted to point that out.  I could've expressed myself more clearly,
though.  Whether and when to clean up this tangle is up for debate!

Re: [PATCH] tests: add test for json-streamer.c error recovery

[Markus Armbruster]

One more thing...

Paolo Bonzini <pbonzini@redhat.com> writes:

> Before rewriting the error recovery code to work in a push parsing
> setup, make sure that we have tests for it.
>
> Cover various cases of invalid JSON, to check that structural
> recovery based on balanced brackets and braces works; and
> lexer-based recovery which documents "\f" as a sure fire
> way to reset the lexer.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  tests/unit/check-json-parser.c | 145 +++++++++++++++++++++++++++++++++
>  tests/unit/meson.build         |   1 +
>  2 files changed, 146 insertions(+)
>  create mode 100644 tests/unit/check-json-parser.c
>
> diff --git a/tests/unit/check-json-parser.c b/tests/unit/check-json-parser.c
> new file mode 100644
> index 00000000000..ca2d5f41097
> --- /dev/null
> +++ b/tests/unit/check-json-parser.c
> @@ -0,0 +1,145 @@
> +/*
> + * Unit tests for JSON Parser error recovery
> + *
> + * Copyright 2026 Red Hat
> + * Author: Paolo Bonzini <pbonzini@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/osdep.h"
> +
> +#include "qapi/error.h"
> +#include "qobject/qbool.h"
> +#include "qobject/json-parser.h"
> +
> +typedef struct ParseResult {
> +    int errors;
> +    QObject *result;
> +} ParseResult;
> +
> +static void parse_emit(void *opaque, QObject *json, Error *err)
> +{
> +    ParseResult *r = opaque;
> +
> +    if (err) {
> +        r->errors++;
> +        error_free(err);
> +    } else {
> +        qobject_unref(r->result);

Unexpected calls to parse_emit() won't be detected as long as the last
one yields the expected expression.  Worth detecting?  I'm not quite
sure.  As far as I can tell, this is not reached at the moment.

The stupidest way to detect is to assert r->result is still null.

Can't do that if we want to cover parsing multiple JSON expressions in
sequence.  Collect all the results in an array?

Leave for later, with a short comment?

Up to you.

> +        r->result = json;
> +    }
> +}

[...]

Re: [PATCH] tests: add test for json-streamer.c error recovery

[Paolo Bonzini]

On Thu, Apr 23, 2026 at 8:56 AM Markus Armbruster <armbru@redhat.com> wrote:
> > +    } else {
> > +        qobject_unref(r->result);
>
> Unexpected calls to parse_emit() won't be detected as long as the last
> one yields the expected expression.  Worth detecting?  I'm not quite
> sure.  As far as I can tell, this is not reached at the moment.

It's only covered in /errors/multiple_values of tests/unit/check-qson.c, yes.

> The stupidest way to detect is to assert r->result is still null.

I'm all for simplicity.

> Can't do that if we want to cover parsing multiple JSON expressions in
> sequence.  Collect all the results in an array?

Yes, but not now. For now the assert will do.

Paolo

Re: [PATCH] tests: add test for json-streamer.c error recovery

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On Thu, Apr 23, 2026 at 8:56 AM Markus Armbruster <armbru@redhat.com> wrote:
>> > +    } else {
>> > +        qobject_unref(r->result);
>>
>> Unexpected calls to parse_emit() won't be detected as long as the last
>> one yields the expected expression.  Worth detecting?  I'm not quite
>> sure.  As far as I can tell, this is not reached at the moment.
>
> It's only covered in /errors/multiple_values of tests/unit/check-qson.c, yes.
>
>> The stupidest way to detect is to assert r->result is still null.
>
> I'm all for simplicity.
>
>> Can't do that if we want to cover parsing multiple JSON expressions in
>> sequence.  Collect all the results in an array?
>
> Yes, but not now. For now the assert will do.

Makes sense.  Thanks!