Title: s/json/JSON/ Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com> > > --- > 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)