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.
>
> 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, support for visit_type_any(),
> and conversion of 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, via abort().
> 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>
>
> ---
> v4: retitle, split off inf/NaN handling, rebase to visit_complete
> and visit_free changes, defer type_any, address other findings from
> Markus
> 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 | 33 ++--
> include/qapi/json-output-visitor.h | 28 +++
> qapi/json-output-visitor.c | 193 +++++++++++++++++++
> tests/test-json-output-visitor.c | 383
> +++++++++++++++++++++++++++++++++++++
> tests/test-qmp-output-visitor.c | 5 +
> qapi/Makefile.objs | 2 +-
> tests/.gitignore | 1 +
> tests/Makefile | 5 +-
> 8 files changed, 632 insertions(+), 18 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 3f46921..c097507 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -26,17 +26,17 @@
> *
> * There are four 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, the
> - * dealloc visitor can take a QAPI graph (possibly partially
> - * constructed) and recursively free its resources, and the clone
> - * visitor performs a deep clone of one QAPI object to another. While
> - * the dealloc and QMP input/output visitors are general, the string,
> - * QemuOpts, and clone 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, the dealloc visitor can take a QAPI graph (possibly
> + * partially constructed) and recursively free its resources, and the
> + * clone visitor performs a deep clone of one QAPI object to another.
> + * While the dealloc, JSON output, and QMP input/output visitors are
> + * general, the string, QemuOpts, and clone 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 of the visitors are created via:
> *
> @@ -59,11 +59,12 @@
> * visitors are declared here; the remaining visitors are generated in
> * qapi-visit.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.
> + * The @name parameter of visit_type_FOO() and visit_start_OBJECT()
> + * 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.
Not related to this patch, just came up in review of v3. Splitting it
out into its own patch doesn't seem worthwhile, so let's keep it here.
> *
> * FIXME: Clients must pass NULL for @name when visiting a member of a
> * list, but this leads to poor error messages; it might be nicer to
> diff --git a/include/qapi/json-output-visitor.h
> b/include/qapi/json-output-visitor.h
> new file mode 100644
> index 0000000..41c79f4
> --- /dev/null
> +++ b/include/qapi/json-output-visitor.h
> @@ -0,0 +1,28 @@
> +/*
> + * JSON Output Visitor
> + *
> + * 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.
> + *
> + */
> +
> +#ifndef JSON_OUTPUT_VISITOR_H
> +#define JSON_OUTPUT_VISITOR_H
> +
> +#include "qapi/visitor.h"
> +
> +typedef struct JsonOutputVisitor JsonOutputVisitor;
> +
> +/*
> + * Create a new JSON output visitor.
> + *
> + * If everything else succeeds, pass @result to visit_complete() to
> + * collect the result of the visit.
> + *
> + * For now, this cannot be used to visit the 'any' type.
> + */
> +Visitor *json_output_visitor_new(char **result);
> +
> +#endif
> diff --git a/qapi/json-output-visitor.c b/qapi/json-output-visitor.c
> new file mode 100644
> index 0000000..7010ff8
> --- /dev/null
> +++ b/qapi/json-output-visitor.c
> @@ -0,0 +1,193 @@
> +/*
> + * Convert QAPI to JSON
> + *
> + * 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 "qapi/json-output-visitor.h"
> +#include "qapi/visitor-impl.h"
> +#include "qapi/qmp/qstring.h"
> +#include "qapi/qmp/qobject-json.h"
> +
> +struct JsonOutputVisitor {
> + Visitor visitor;
> + QString *str;
> + bool comma;
> + unsigned int depth;
> + char **result;
> +};
> +
> +static JsonOutputVisitor *to_jov(Visitor *v)
> +{
> + return container_of(v, JsonOutputVisitor, visitor);
> +}
> +
> +static void json_output_name(JsonOutputVisitor *jov, const char *name)
> +{
> + if (jov->comma) {
> + qstring_append(jov->str, ", ");
> + } else {
> + jov->comma = true;
> + }
> + if (name && jov->depth) {
> + qstring_append_json_string(jov->str, name);
> + qstring_append(jov->str, ": ");
> + }
> +}
> +
> +static void json_output_start_struct(Visitor *v, const char *name, void
> **obj,
> + size_t unused, Error **errp)
> +{
> + JsonOutputVisitor *jov = to_jov(v);
> +
> + json_output_name(jov, name);
> + qstring_append(jov->str, "{");
> + jov->comma = false;
> + jov->depth++;
> +}
> +
> +static void json_output_end_struct(Visitor *v, void **obj)
> +{
> + 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, void **obj)
> +{
> + JsonOutputVisitor *jov = to_jov(v);
> +
> + assert(jov->depth);
> + jov->depth--;
> + qstring_append(jov->str, "]");
> + jov->comma = true;
> +}
> +
> +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_printf(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_printf(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);
> + /* FIXME: report invalid UTF-8 encoding */
> + 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);
> + /* FIXME: report Inf/NaN problems */
> + qstring_append_json_number(jov->str, *obj);
> +}
> +
> +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");
> +}
> +
> +static void json_output_complete(Visitor *v, void *result)
> +{
> + JsonOutputVisitor *jov = to_jov(v);
> +
> + assert(!jov->depth);
> + assert(qstring_get_length(jov->str));
> + assert(jov->result == result);
> + *jov->result = qstring_consume_str(jov->str);
> + jov->str = NULL;
> + jov->result = NULL;
> +}
Related: discussion of complete() semantics in review of PATCH 12.
Non-idempotent semantics save us a copy, like in
string_output_complete().
> +
> +static void json_output_free(Visitor *v)
> +{
> + JsonOutputVisitor *jov = to_jov(v);
> +
> + QDECREF(jov->str);
> + g_free(jov);
I'm afraid this leaks jov->result when the visitor is destroyed without
calling its complete() method.
string_output_free() appears to have the same leak.
> +}
> +
> +Visitor *json_output_visitor_new(char **result)
> +{
> + JsonOutputVisitor *v;
> +
> + v = g_malloc0(sizeof(*v));
> + v->result = result;
> + *result = NULL;
> + v->str = qstring_new();
> +
> + 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_null = json_output_type_null;
> + v->visitor.complete = json_output_complete;
> + v->visitor.free = json_output_free;
> +
> + return &v->visitor;
> +}
[...]
