Re: [Qemu-devel] [PATCH v3 07/18] qapi: Add json output visitor

[Markus Armbruster]

Title: s/json/JSON/

Eric Blake <address@hidden> writes:

> We have several places that want to go from qapi to JSON; right now,
> they have to create an intermediate QObject to do the work.  That
> also has the drawback that the JSON formatting of a QDict will
> rearrange keys (according to a deterministic, but unpredictable,
> hash), when humans have an easier time if dicts are produced in
> the same order as the qapi type.

There's a drawback, though: more code.

Could the JSON output visitor replace the QMP output visitor?

Aside: the QMP visitors are really QObject visitors.

> For these reasons, it is time to add a new JSON output visitor.
> This patch just adds the basic visitor and tests that it works;
> later patches will add pretty-printing, and convert clients to
> use the visitor.
>
> Design choices: Unlike the QMP output visitor, the JSON visitor
> refuses to visit a required string with a NULL value (abort), as
> well as a non-finite number (raises an error message).  Reusing
> QString to grow the contents means that we easily share code with
> both qobject-json.c and qjson.c; although it might be nice to
> enhance things to take an optional output callback function so
> that the output can truly be streamed instead of collected in
> memory.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v3: retitle, rebase to master, minor cleanups
> v2: rebase to qapi subset E v8; add test of error outputting
> infinity; use unsigned depth
> ---
>  include/qapi/visitor.h             |  20 +-
>  include/qapi/json-output-visitor.h |  29 +++
>  qapi/json-output-visitor.c         | 202 ++++++++++++++++++
>  tests/test-json-output-visitor.c   | 418 
> +++++++++++++++++++++++++++++++++++++
>  qapi/Makefile.objs                 |   2 +-
>  tests/.gitignore                   |   1 +
>  tests/Makefile                     |   4 +
>  7 files changed, 665 insertions(+), 11 deletions(-)
>  create mode 100644 include/qapi/json-output-visitor.h
>  create mode 100644 qapi/json-output-visitor.c
>  create mode 100644 tests/test-json-output-visitor.c
>
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index a430c19..e8a4403 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -26,16 +26,16 @@
>   *
>   * There are three kinds of visitor classes: input visitors (QMP,
>   * string, and QemuOpts) parse an external representation and build
> - * the corresponding QAPI graph, output visitors (QMP and string) take
> - * a completed QAPI graph and generate an external representation, and
> - * the dealloc visitor can take a QAPI graph (possibly partially
> - * constructed) and recursively free its resources.  While the dealloc
> - * and QMP input/output visitors are general, the string and QemuOpts
> - * visitors have some implementation limitations; see the
> - * documentation for each visitor for more details on what it
> - * supports.  Also, see visitor-impl.h for the callback contracts
> - * implemented by each visitor, and docs/qapi-code-gen.txt for more
> - * about the QAPI code generator.
> + * the corresponding QAPI graph, output visitors (QMP, string, and
> + * JSON) take a completed QAPI graph and generate an external
> + * representation, and the dealloc visitor can take a QAPI graph
> + * (possibly partially constructed) and recursively free its
> + * resources.  While the dealloc and QMP input/output visitors are
> + * general, the string and QemuOpts visitors have some implementation
> + * limitations; see the documentation for each visitor for more
> + * details on what it supports.  Also, see visitor-impl.h for the
> + * callback contracts implemented by each visitor, and
> + * docs/qapi-code-gen.txt for more about the QAPI code generator.
>   *
>   * All QAPI types have a corresponding function with a signature
>   * roughly compatible with this:
> diff --git a/include/qapi/json-output-visitor.h 
> b/include/qapi/json-output-visitor.h
> new file mode 100644
> index 0000000..ac03da1
> --- /dev/null
> +++ b/include/qapi/json-output-visitor.h
> @@ -0,0 +1,29 @@
> +/*
> + * JSON Output Visitor
> + *
> + * Copyright (C) 2015-2016 Red Hat, Inc.
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef JSON_OUTPUT_VISITOR_H
> +#define JSON_OUTPUT_VISITOR_H
> +
> +#include "qapi/visitor.h"
> +
> +typedef struct JsonOutputVisitor JsonOutputVisitor;
> +
> +/*
> + * The JSON output visitor does not accept Infinity or NaN to
> + * visit_type_number().
> + */
> +JsonOutputVisitor *json_output_visitor_new(void);
> +void json_output_visitor_cleanup(JsonOutputVisitor *v);
> +void json_output_visitor_reset(JsonOutputVisitor *v);

Hmm.  Why is "reset" not a Visitor method?

I think this would let us put the things enforced by your "qmp: Tighten
output visitor rules" in the Visitor contract.

> +
> +char *json_output_get_string(JsonOutputVisitor *v);
> +Visitor *json_output_get_visitor(JsonOutputVisitor *v);
> +
> +#endif
> diff --git a/qapi/json-output-visitor.c b/qapi/json-output-visitor.c
> new file mode 100644
> index 0000000..3539db5
> --- /dev/null
> +++ b/qapi/json-output-visitor.c
> @@ -0,0 +1,202 @@
> +/*
> + * Convert QAPI to JSON
> + *
> + * Copyright (C) 2015-2016 Red Hat, Inc.
> + *
> + * 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/json-output-visitor.h"
> +#include "qapi/visitor-impl.h"
> +#include "qemu/queue.h"
> +#include "qemu-common.h"

qemu/queue.h and qemu-common.h are superfluous.

> +#include "qapi/qmp/qstring.h"
> +#include "qapi/qmp/qobject-json.h"
> +
> +struct JsonOutputVisitor {
> +    Visitor visitor;
> +    QString *str;
> +    bool comma;
> +    unsigned int depth;
> +};
> +
> +static JsonOutputVisitor *to_jov(Visitor *v)
> +{
> +    return container_of(v, JsonOutputVisitor, visitor);
> +}
> +
> +static void json_output_name(JsonOutputVisitor *jov, const char *name)

This is called for every value, by its visit_start_FOO() or
visit_type_FOO() function.  Quote visitor.h:

 * The @name parameter of visit_type_FOO() describes the relation
 * between this QAPI value and its parent container.  When visiting
 * the root of a tree, @name is ignored; when visiting a member of an
 * object, @name is the key associated with the value; and when
 * visiting a member of a list, @name is NULL.

Aside: it should mention visit_start_FOO() in addition to
visit_type_FOO().

> +{
> +    if (!jov->str) {
> +        jov->str = qstring_new();

This happens on the first call after creation or reset of jov.

If you call json_output_get_string() after an empty visit, it chokes on
null jov->str.  Could be declared a feature.  The Visitor contract is
silent on it, but the QMP output visitor rejects it since "qmp: Tighten
output visitor rules".

Regardless: why not create jov->str in json_output_visitor_new(), and
truncate it in json_output_visitor_reset()?

To retain the "feature", you'd assert qstring_get_length(jov->str).

> +    }
> +    if (jov->comma) {
> +        qstring_append(jov->str, ", ");

Must be in an array or object.

We get here only when called multiple times without a reset of
jov->comma in between, i.e. a call of json_output_start_struct(),
json_output_start_list(), or json_output_visitor_reset().

In other words, if we get here outside an array or object, the caller
screwed up.  Okay, although the Visitor contract is silent on it.

> +    } else {
> +        jov->comma = true;
> +    }
> +    if (name && jov->depth) {
> +        qstring_append_json_string(jov->str, name);
> +        qstring_append(jov->str, ": ");

Must be in an object.  Covered by the Visitor contract quoted above.

> +    }
> +}
> +
> +static void json_output_start_struct(Visitor *v, const char *name, void 
> **obj,
> +                                     size_t unused, Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);

Blank line between declarations and statements, please.  More of the
same below.

> +    json_output_name(jov, name);
> +    qstring_append(jov->str, "{");
> +    jov->comma = false;
> +    jov->depth++;
> +}
> +
> +static void json_output_end_struct(Visitor *v)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    assert(jov->depth);
> +    jov->depth--;
> +    qstring_append(jov->str, "}");
> +    jov->comma = true;
> +}
> +
> +static void json_output_start_list(Visitor *v, const char *name,
> +                                   GenericList **listp, size_t size,
> +                                   Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    json_output_name(jov, name);
> +    qstring_append(jov->str, "[");
> +    jov->comma = false;
> +    jov->depth++;
> +}
> +
> +static GenericList *json_output_next_list(Visitor *v, GenericList *tail,
> +                                          size_t size)
> +{
> +    return tail->next;
> +}
> +
> +static void json_output_end_list(Visitor *v)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    assert(jov->depth);
> +    jov->depth--;
> +    qstring_append(jov->str, "]");
> +    jov->comma = true;
> +}

The nesting checks are less thorough than the QMP visitor's, because we
don't use a stack.  Okay.

> +
> +static void json_output_type_int64(Visitor *v, const char *name, int64_t 
> *obj,
> +                                   Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    json_output_name(jov, name);
> +    qstring_append_format(jov->str, "%" PRId64, *obj);
> +}
> +
> +static void json_output_type_uint64(Visitor *v, const char *name,
> +                                    uint64_t *obj, Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    json_output_name(jov, name);
> +    qstring_append_format(jov->str, "%" PRIu64, *obj);
> +}
> +
> +static void json_output_type_bool(Visitor *v, const char *name, bool *obj,
> +                                  Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    json_output_name(jov, name);
> +    qstring_append(jov->str, *obj ? "true" : "false");
> +}
> +
> +static void json_output_type_str(Visitor *v, const char *name, char **obj,
> +                                 Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    assert(*obj);
> +    json_output_name(jov, name);
> +    qstring_append_json_string(jov->str, *obj);
> +}
> +
> +static void json_output_type_number(Visitor *v, const char *name, double 
> *obj,
> +                                    Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    json_output_name(jov, name);
> +    qstring_append_json_number(jov->str, *obj, errp);
> +}
> +
> +static void json_output_type_any(Visitor *v, const char *name, QObject **obj,
> +                                 Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    QString *str = qobject_to_json(*obj);
> +    assert(str);

Can't happen.

> +    json_output_name(jov, name);
> +    qstring_append(jov->str, qstring_get_str(str));
> +    QDECREF(str);
> +}
> +
> +static void json_output_type_null(Visitor *v, const char *name, Error **errp)
> +{
> +    JsonOutputVisitor *jov = to_jov(v);
> +    json_output_name(jov, name);
> +    qstring_append(jov->str, "null");
> +}
> +
> +/* Finish building, and return the resulting string. Will not be NULL. */
> +char *json_output_get_string(JsonOutputVisitor *jov)
> +{
> +    char *result;
> +
> +    assert(!jov->depth);
> +    result = g_strdup(qstring_get_str(jov->str));
> +    json_output_visitor_reset(jov);

Could avoid the strdup() if we wanted to.  Needs a way to convert
jov->str to char * destructively, like g_string_free() can do for a
GString.  Your choice.

> +    return result;
> +}
> +
> +Visitor *json_output_get_visitor(JsonOutputVisitor *v)
> +{
> +    return &v->visitor;
> +}
> +
> +void json_output_visitor_reset(JsonOutputVisitor *v)
> +{
> +    QDECREF(v->str);
> +    v->str = NULL;
> +    v->depth = 0;
> +    v->comma = false;
> +}
> +
> +void json_output_visitor_cleanup(JsonOutputVisitor *v)
> +{
> +    json_output_visitor_reset(v);
> +    g_free(v);
> +}
> +
> +JsonOutputVisitor *json_output_visitor_new(void)
> +{
> +    JsonOutputVisitor *v;
> +
> +    v = g_malloc0(sizeof(*v));
> +
> +    v->visitor.type = VISITOR_OUTPUT;
> +    v->visitor.start_struct = json_output_start_struct;
> +    v->visitor.end_struct = json_output_end_struct;
> +    v->visitor.start_list = json_output_start_list;
> +    v->visitor.next_list = json_output_next_list;
> +    v->visitor.end_list = json_output_end_list;
> +    v->visitor.type_int64 = json_output_type_int64;
> +    v->visitor.type_uint64 = json_output_type_uint64;
> +    v->visitor.type_bool = json_output_type_bool;
> +    v->visitor.type_str = json_output_type_str;
> +    v->visitor.type_number = json_output_type_number;
> +    v->visitor.type_any = json_output_type_any;
> +    v->visitor.type_null = json_output_type_null;
> +
> +    return v;
> +}
> diff --git a/tests/test-json-output-visitor.c 
> b/tests/test-json-output-visitor.c
> new file mode 100644
> index 0000000..57fe1c6
> --- /dev/null
> +++ b/tests/test-json-output-visitor.c
> @@ -0,0 +1,418 @@
> +/*
> + * JSON Output Visitor unit-tests.
> + *
> + * Copyright (C) 2015-2016 Red Hat Inc.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include <glib.h>
> +#include <math.h>
> +
> +#include "qemu-common.h"
> +#include "qapi/json-output-visitor.h"
> +#include "test-qapi-types.h"
> +#include "test-qapi-visit.h"
> +#include "qapi/qmp/types.h"
> +
> +typedef struct TestOutputVisitorData {
> +    JsonOutputVisitor *jov;
> +    Visitor *ov;
> +} TestOutputVisitorData;
> +
> +static void visitor_output_setup(TestOutputVisitorData *data,
> +                                 const void *unused)
> +{
> +    data->jov = json_output_visitor_new();
> +    g_assert(data->jov);
> +
> +    data->ov = json_output_get_visitor(data->jov);
> +    g_assert(data->ov);
> +}
> +
> +static void visitor_output_teardown(TestOutputVisitorData *data,
> +                                    const void *unused)
> +{
> +    json_output_visitor_cleanup(data->jov);
> +    data->jov = NULL;
> +    data->ov = NULL;
> +}
> +
> +static void test_visitor_out_int(TestOutputVisitorData *data,
> +                                 const void *unused)
> +{
> +    int64_t value = -42;
> +    char *out;
> +
> +    visit_type_int(data->ov, NULL, &value, &error_abort);
> +
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==, "-42");
> +    g_free(out);
> +}
> +
> +static void test_visitor_out_bool(TestOutputVisitorData *data,
> +                                  const void *unused)
> +{
> +    bool value = true;
> +    char *out;
> +
> +    visit_type_bool(data->ov, NULL, &value, &error_abort);
> +
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==, "true");
> +    g_free(out);
> +}
> +
> +static void test_visitor_out_number(TestOutputVisitorData *data,
> +                                    const void *unused)
> +{
> +    double value = 3.14;
> +    char *out;
> +    Error *err = NULL;
> +
> +    visit_type_number(data->ov, NULL, &value, &error_abort);
> +
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==, "3.14");
> +    g_free(out);
> +
> +    /* JSON requires finite numbers */
> +    value = INFINITY;
> +    visit_type_number(data->ov, NULL, &value, &err);
> +    g_assert(err);
> +    error_free(err);

error_free_or_abort()

> +}
> +
> +static void test_visitor_out_string(TestOutputVisitorData *data,
> +                                    const void *unused)
> +{
> +    char *string = (char *) "Q E M U";
> +    char *out;
> +
> +    visit_type_str(data->ov, NULL, &string, &error_abort);
> +
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==, "\"Q E M U\"");
> +    g_free(out);
> +}
> +
> +static void test_visitor_out_enum(TestOutputVisitorData *data,
> +                                  const void *unused)
> +{
> +    char *out;
> +    char *tmp;
> +    EnumOne i;
> +    size_t len;
> +
> +    for (i = 0; i < ENUM_ONE__MAX; i++) {
> +        visit_type_EnumOne(data->ov, "unused", &i, &error_abort);
> +
> +        out = json_output_get_string(data->jov);
> +        g_assert(*out == '"');
> +        len = strlen(out);
> +        g_assert(out[len - 1] == '"');
> +        tmp = out + 1;
> +        out[len - 1] = 0;
> +        g_assert_cmpstr(tmp, ==, EnumOne_lookup[i]);
> +        g_free(out);

Unlike in test-qmp-output-visitor.c, you don't need
qmp_output_visitor_reset() here, because json_output_get_string() does
it automatically.

Is the difference between json_output_get_string() and
qmp_output_get_qobject() a good idea?

> +    }
> +}
> +
> +static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
> +                                         const void *unused)
> +{
> +    EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
> +    Error *err;
> +
> +    for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
> +        err = NULL;
> +        visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
> +        g_assert(err);
> +        error_free(err);

error_free_or_abort()

Why no json_output_visitor_reset() here?

Running out of time, skipping the rest of the test for now.

> +    }
> +}
> +
> +
> +static void test_visitor_out_struct(TestOutputVisitorData *data,
> +                                    const void *unused)
> +{
> +    TestStruct test_struct = { .integer = 42,
> +                               .boolean = false,
> +                               .string = (char *) "foo"};
> +    TestStruct *p = &test_struct;
> +    char *out;
> +
> +    visit_type_TestStruct(data->ov, NULL, &p, &error_abort);
> +
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==,
> +                    "{"
> +                     "\"integer\": 42, "
> +                     "\"boolean\": false, "
> +                     "\"string\": \"foo\""
> +                    "}");
> +    g_free(out);
> +}
> +
> +static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
> +                                           const void *unused)
> +{
> +    int64_t value = 42;
> +    UserDefTwo *ud2;
> +    const char *string = "user def string";
> +    const char *strings[] = { "forty two", "forty three", "forty four",
> +                              "forty five" };
> +    char *out;
> +
> +    ud2 = g_malloc0(sizeof(*ud2));
> +    ud2->string0 = g_strdup(strings[0]);
> +
> +    ud2->dict1 = g_malloc0(sizeof(*ud2->dict1));
> +    ud2->dict1->string1 = g_strdup(strings[1]);
> +
> +    ud2->dict1->dict2 = g_malloc0(sizeof(*ud2->dict1->dict2));
> +    ud2->dict1->dict2->userdef = g_new0(UserDefOne, 1);
> +    ud2->dict1->dict2->userdef->string = g_strdup(string);
> +    ud2->dict1->dict2->userdef->integer = value;
> +    ud2->dict1->dict2->string = g_strdup(strings[2]);
> +
> +    ud2->dict1->dict3 = g_malloc0(sizeof(*ud2->dict1->dict3));
> +    ud2->dict1->has_dict3 = true;
> +    ud2->dict1->dict3->userdef = g_new0(UserDefOne, 1);
> +    ud2->dict1->dict3->userdef->string = g_strdup(string);
> +    ud2->dict1->dict3->userdef->integer = value;
> +    ud2->dict1->dict3->string = g_strdup(strings[3]);
> +
> +    visit_type_UserDefTwo(data->ov, "unused", &ud2, &error_abort);
> +
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==,
> +                    "{"
> +                     "\"string0\": \"forty two\", "
> +                     "\"dict1\": {"
> +                      "\"string1\": \"forty three\", "
> +                      "\"dict2\": {"
> +                       "\"userdef\": {"
> +                        "\"integer\": 42, "
> +                        "\"string\": \"user def string\""
> +                        "}, "
> +                       "\"string\": \"forty four\""
> +                       "}, "
> +                      "\"dict3\": {"
> +                       "\"userdef\": {"
> +                        "\"integer\": 42, "
> +                        "\"string\": \"user def string\""
> +                        "}, "
> +                      "\"string\": \"forty five\""
> +                      "}"
> +                     "}"
> +                    "}");
> +    qapi_free_UserDefTwo(ud2);
> +    g_free(out);
> +}
> +
> +static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
> +                                           const void *unused)
> +{
> +    EnumOne bad_values[] = { ENUM_ONE__MAX, -1 };
> +    UserDefOne u = { .string = (char *)"" };
> +    UserDefOne *pu = &u;
> +    Error *err;
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
> +        err = NULL;
> +        u.has_enum1 = true;
> +        u.enum1 = bad_values[i];
> +        visit_type_UserDefOne(data->ov, "unused", &pu, &err);
> +        g_assert(err);
> +        error_free(err);

error_free_or_abort()

> +        json_output_visitor_reset(data->jov);
> +    }
> +}
> +
> +
> +static void test_visitor_out_list(TestOutputVisitorData *data,
> +                                  const void *unused)
> +{
> +    const char *value_str = "list value";
> +    TestStructList *p, *head = NULL;
> +    const int max_items = 10;
> +    bool value_bool = true;
> +    int value_int = 10;
> +    int i;
> +    char *out;
> +
> +    for (i = 0; i < max_items; i++) {
> +        p = g_malloc0(sizeof(*p));
> +        p->value = g_malloc0(sizeof(*p->value));
> +        p->value->integer = value_int + (max_items - i - 1);
> +        p->value->boolean = value_bool;
> +        p->value->string = g_strdup(value_str);
> +
> +        p->next = head;
> +        head = p;
> +    }
> +
> +    visit_type_TestStructList(data->ov, NULL, &head, &error_abort);
> +
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==,
> +                    "["
> +                     "{\"integer\": 10, \"boolean\": true, "
> +                      "\"string\": \"list value\"}, "
> +                     "{\"integer\": 11, \"boolean\": true, "
> +                      "\"string\": \"list value\"}, "
> +                     "{\"integer\": 12, \"boolean\": true, "
> +                      "\"string\": \"list value\"}, "
> +                     "{\"integer\": 13, \"boolean\": true, "
> +                      "\"string\": \"list value\"}, "
> +                     "{\"integer\": 14, \"boolean\": true, "
> +                      "\"string\": \"list value\"}, "
> +                     "{\"integer\": 15, \"boolean\": true, "
> +                      "\"string\": \"list value\"}, "
> +                     "{\"integer\": 16, \"boolean\": true, "
> +                      "\"string\": \"list value\"}, "
> +                     "{\"integer\": 17, \"boolean\": true, "
> +                      "\"string\": \"list value\"}, "
> +                     "{\"integer\": 18, \"boolean\": true, "
> +                      "\"string\": \"list value\"}, "
> +                     "{\"integer\": 19, \"boolean\": true, "
> +                      "\"string\": \"list value\"}"
> +                    "]");
> +    qapi_free_TestStructList(head);
> +    g_free(out);
> +}
> +
> +static void test_visitor_out_any(TestOutputVisitorData *data,
> +                                 const void *unused)
> +{
> +    QObject *qobj;
> +    QDict *qdict;
> +    char *out;
> +
> +    qobj = QOBJECT(qint_from_int(-42));
> +    visit_type_any(data->ov, NULL, &qobj, &error_abort);
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==, "-42");
> +    qobject_decref(qobj);
> +    g_free(out);
> +
> +    qdict = qdict_new();
> +    qdict_put(qdict, "integer", qint_from_int(-42));
> +    qdict_put(qdict, "boolean", qbool_from_bool(true));
> +    qdict_put(qdict, "string", qstring_from_str("foo"));
> +    qobj = QOBJECT(qdict);
> +    visit_type_any(data->ov, NULL, &qobj, &error_abort);
> +    qobject_decref(qobj);
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==,
> +                    "{"
> +                     "\"integer\": -42, "
> +                     "\"boolean\": true, "
> +                     "\"string\": \"foo\""
> +                    "}");
> +    g_free(out);
> +}
> +
> +static void test_visitor_out_union_flat(TestOutputVisitorData *data,
> +                                        const void *unused)
> +{
> +    char *out;
> +    UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
> +
> +    tmp->enum1 = ENUM_ONE_VALUE1;
> +    tmp->string = g_strdup("str");
> +    tmp->integer = 41;
> +    tmp->u.value1.boolean = true;
> +
> +    visit_type_UserDefFlatUnion(data->ov, NULL, &tmp, &error_abort);
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==,
> +                    "{"
> +                     "\"integer\": 41, "
> +                     "\"string\": \"str\", "
> +                     "\"enum1\": \"value1\", "
> +                     "\"boolean\": true"
> +                    "}");
> +    g_free(out);
> +    qapi_free_UserDefFlatUnion(tmp);
> +}
> +
> +static void test_visitor_out_alternate(TestOutputVisitorData *data,
> +                                       const void *unused)
> +{
> +    UserDefAlternate *tmp;
> +    char *out;
> +
> +    tmp = g_new0(UserDefAlternate, 1);
> +    tmp->type = QTYPE_QINT;
> +    tmp->u.i = 42;
> +
> +    visit_type_UserDefAlternate(data->ov, NULL, &tmp, &error_abort);
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==, "42");
> +    g_free(out);
> +    qapi_free_UserDefAlternate(tmp);
> +
> +    tmp = g_new0(UserDefAlternate, 1);
> +    tmp->type = QTYPE_QSTRING;
> +    tmp->u.s = g_strdup("hello");
> +
> +    visit_type_UserDefAlternate(data->ov, NULL, &tmp, &error_abort);
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==, "\"hello\"");
> +    g_free(out);
> +    qapi_free_UserDefAlternate(tmp);
> +}
> +
> +static void test_visitor_out_null(TestOutputVisitorData *data,
> +                                  const void *unused)
> +{
> +    char *out;
> +
> +    visit_type_null(data->ov, NULL, &error_abort);
> +    out = json_output_get_string(data->jov);
> +    g_assert_cmpstr(out, ==, "null");
> +    g_free(out);
> +}
> +
> +static void output_visitor_test_add(const char *testpath,
> +                                    void (*test_func)(TestOutputVisitorData 
> *,
> +                                                      const void *))
> +{
> +    g_test_add(testpath, TestOutputVisitorData, NULL, visitor_output_setup,
> +               test_func, visitor_output_teardown);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +
> +    output_visitor_test_add("/visitor/json/int", test_visitor_out_int);
> +    output_visitor_test_add("/visitor/json/bool", test_visitor_out_bool);
> +    output_visitor_test_add("/visitor/json/number", test_visitor_out_number);
> +    output_visitor_test_add("/visitor/json/string", test_visitor_out_string);
> +    output_visitor_test_add("/visitor/json/enum", test_visitor_out_enum);
> +    output_visitor_test_add("/visitor/json/enum-errors",
> +                            test_visitor_out_enum_errors);
> +    output_visitor_test_add("/visitor/json/struct", test_visitor_out_struct);
> +    output_visitor_test_add("/visitor/json/struct-nested",
> +                            test_visitor_out_struct_nested);
> +    output_visitor_test_add("/visitor/json/struct-errors",
> +                            test_visitor_out_struct_errors);
> +    output_visitor_test_add("/visitor/json/list", test_visitor_out_list);
> +    output_visitor_test_add("/visitor/json/any", test_visitor_out_any);
> +    output_visitor_test_add("/visitor/json/union-flat",
> +                            test_visitor_out_union_flat);
> +    output_visitor_test_add("/visitor/json/alternate",
> +                            test_visitor_out_alternate);
> +    output_visitor_test_add("/visitor/json/null", test_visitor_out_null);
> +
> +    g_test_run();
> +
> +    return 0;
> +}

This is obviously patterned after test-qmp-output-visitor.c.  Should we
link the two with comments, to improve our chances of them being kept in
sync?

> diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
> index 2278970..b60e11b 100644
> --- a/qapi/Makefile.objs
> +++ b/qapi/Makefile.objs
> @@ -1,6 +1,6 @@
>  util-obj-y = qapi-visit-core.o qapi-dealloc-visitor.o qmp-input-visitor.o
>  util-obj-y += qmp-output-visitor.o qmp-registry.o qmp-dispatch.o
>  util-obj-y += string-input-visitor.o string-output-visitor.o
> -util-obj-y += opts-visitor.o
> +util-obj-y += opts-visitor.o json-output-visitor.o
>  util-obj-y += qmp-event.o
>  util-obj-y += qapi-util.o
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 2f8c7ea..c2aad79 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -40,6 +40,7 @@ test-io-channel-file.txt
>  test-io-channel-socket
>  test-io-channel-tls
>  test-io-task
> +test-json-output-visitor
>  test-logging
>  test-mul64
>  test-opts-visitor
> diff --git a/tests/Makefile b/tests/Makefile
> index f71ed1c..0b5a7a8 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -22,6 +22,8 @@ check-unit-y += tests/check-qobject-json$(EXESUF)
>  gcov-files-check-qobject-json-y = qobject/qobject-json.c
>  check-unit-y += tests/test-qmp-output-visitor$(EXESUF)
>  gcov-files-test-qmp-output-visitor-y = qapi/qmp-output-visitor.c
> +check-unit-y += tests/test-json-output-visitor$(EXESUF)
> +gcov-files-test-json-output-visitor-y = qapi/json-output-visitor.c
>  check-unit-y += tests/test-qmp-input-visitor$(EXESUF)
>  gcov-files-test-qmp-input-visitor-y = qapi/qmp-input-visitor.c
>  check-unit-y += tests/test-qmp-input-strict$(EXESUF)
> @@ -388,6 +390,7 @@ test-obj-y = tests/check-qint.o tests/check-qstring.o 
> tests/check-qdict.o \
>       tests/check-qobject-json.o \
>       tests/test-coroutine.o tests/test-string-output-visitor.o \
>       tests/test-string-input-visitor.o tests/test-qmp-output-visitor.o \
> +     tests/test-json-output-visitor.o \
>       tests/test-qmp-input-visitor.o tests/test-qmp-input-strict.o \
>       tests/test-qmp-commands.o tests/test-visitor-serialization.o \
>       tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
> @@ -478,6 +481,7 @@ tests/test-string-output-visitor$(EXESUF): 
> tests/test-string-output-visitor.o $(
>  tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o 
> $(test-qapi-obj-y)
>  tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y)
>  tests/test-qmp-output-visitor$(EXESUF): tests/test-qmp-output-visitor.o 
> $(test-qapi-obj-y)
> +tests/test-json-output-visitor$(EXESUF): tests/test-json-output-visitor.o 
> $(test-qapi-obj-y)
>  tests/test-qmp-input-visitor$(EXESUF): tests/test-qmp-input-visitor.o 
> $(test-qapi-obj-y)
>  tests/test-qmp-input-strict$(EXESUF): tests/test-qmp-input-strict.o 
> $(test-qapi-obj-y)
>  tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o 
> tests/test-qmp-marshal.o $(test-qapi-obj-y)