Re: [Qemu-devel] [PATCH v4 26/28] qobject: Implement qobject_to_json() atop JSON visitor

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> Rather than open-code two different JSON visitors, it's nicer to
> make qobject_to_json() reuse the JSON output visitor.  This also
> lets us pass QObject to any output visitor (passing it to the
> string output visitor will fail if structs are involved, but
> passing it to the QMP output visitor would provide a way to
> deep-clone a QObject).

If I understand this correctly, the basic idea is as follows.

The JSON ouput visitor is normally used via QAPI-generated visit
functions to convert a QAPI object (in general a tree) to JSON.  This is
a "real" visit in QAPI visitor parlance.

Note that the tree walk is in the QAPI-generated visit functions, and
the actual conversion is in the JSON output visitor.  This lets us reuse
the latter with a different tree walk, namely one of a QObject (also a
tree).  This is a "virtual" visit in QAPI visitor parlance.  The result
is a replacement for qobject_to_json() that is shorter and avoids
duplicating the conversion.

We can also use this QObject tree walk with other output visitors.  The
string output visitor isn't terribly interesting.  With the (misnamed)
QMP output visitor, we get a QObject deep clone.

Makes me wonder whether we have any QObject deep-clones in the tree we
could replace now.

> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v4: new patch, inspired by discussion on v3 12/18
> ---
>  include/qapi/qmp/qobject-json.h |   9 ++++
>  qobject/qobject-json.c          | 108 
> +++++++++++-----------------------------
>  2 files changed, 39 insertions(+), 78 deletions(-)
>
> diff --git a/include/qapi/qmp/qobject-json.h b/include/qapi/qmp/qobject-json.h
> index e4d11cf..28e44b2 100644
> --- a/include/qapi/qmp/qobject-json.h
> +++ b/include/qapi/qmp/qobject-json.h
> @@ -24,6 +24,15 @@ QObject *qobject_from_jsonv(const char *string, va_list 
> *ap) GCC_FMT_ATTR(1, 0);
>  /* Convert the object to QString; does not fail. */
>  QString *qobject_to_json(const QObject *obj, bool pretty);
>
> +/*
> + * Visit the object with an output visitor @v.
> + *
> + * @name represents the relationship of this object to any parent
> + * (ignored for a top-level visit, must be set if part of a
> + * dictionary, should be NULL if part of a list).
> + */
> +void qobject_visit_output(Visitor *v, const char *name, const QObject *obj);
> +
>  int qstring_append_json_string(QString *qstring, const char *str);
>  int qstring_append_json_number(QString *qstring, double number);
>
> diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c
> index 9ace92b..05b020a 100644
> --- a/qobject/qobject-json.c
> +++ b/qobject/qobject-json.c
> @@ -18,6 +18,9 @@
>  #include "qapi/qmp/qobject-json.h"
>  #include "qemu/unicode.h"
>  #include "qapi/qmp/types.h"
> +#include "qapi/error.h"
> +#include "qapi/visitor.h"
> +#include "qapi/json-output-visitor.h"
>  #include <math.h>
>
>  typedef struct JSONParsingState
> @@ -69,115 +72,62 @@ QObject *qobject_from_jsonf(const char *string, ...)
>      return obj;
>  }
>
> -typedef struct ToJsonIterState
> -{
> -    int indent;
> -    bool pretty;
> -    int count;
> -    QString *str;
> -} ToJsonIterState;
> -
> -static void to_json(const QObject *obj, QString *str, bool pretty, int 
> indent);
> -
>  static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
>  {
> -    ToJsonIterState *s = opaque;
> +    Visitor *v = opaque;
>
> -    if (s->count) {
> -        qstring_append(s->str, s->pretty ? "," : ", ");
> -    }
> -
> -    if (s->pretty) {
> -        qstring_append_printf(s->str, "\n%*s", 4 * s->indent, "");
> -    }
> -
> -    qstring_append_json_string(s->str, key);
> -
> -    qstring_append(s->str, ": ");
> -    to_json(obj, s->str, s->pretty, s->indent);
> -    s->count++;
> +    qobject_visit_output(v, key, obj);
>  }

This function is now a trivial wrapper.  It's used like this:

       qdict_iter(val, to_json_dict_iter, v);

You might want to replace it by something like

       for (ent = qdict_first(val); ent; ent = qdict_next(val, ent)) {
           qobject_visit_output(v, ent->key, ent->value);
       }

I'd find that easier to understand.

>
>  static void to_json_list_iter(QObject *obj, void *opaque)
>  {
> -    ToJsonIterState *s = opaque;
> +    Visitor *v = opaque;
>
> -    if (s->count) {
> -        qstring_append(s->str, s->pretty ? "," : ", ");
> -    }
> -
> -    if (s->pretty) {
> -        qstring_append_printf(s->str, "\n%*s", 4 * s->indent, "");
> -    }
> -
> -    to_json(obj, s->str, s->pretty, s->indent);
> -    s->count++;
> +    qobject_visit_output(v, NULL, obj);
>  }

Likewise.

>
> -static void to_json(const QObject *obj, QString *str, bool pretty, int 
> indent)
> +void qobject_visit_output(Visitor *v, const char *name, const QObject *obj)
>  {
>      switch (qobject_type(obj)) {
>      case QTYPE_QNULL:
> -        qstring_append(str, "null");
> +        visit_type_null(v, name, &error_abort);
>          break;
>      case QTYPE_QINT: {
> -        QInt *val = qobject_to_qint(obj);
> -        qstring_append_printf(str, "%" PRId64, qint_get_int(val));
> +        int64_t val = qint_get_int(qobject_to_qint(obj));
> +        visit_type_int64(v, name, &val, &error_abort);
>          break;
>      }
>      case QTYPE_QSTRING: {
> -        QString *val = qobject_to_qstring(obj);
> +        const char *str = qstring_get_str(qobject_to_qstring(obj));
>          /* FIXME: no way inform user if we modified the string to
>           * avoid encoding errors */

More precise: no way to inform the user if we replaced invalid UTF-8
sequences in the QString.

> -        qstring_append_json_string(str, qstring_get_str(val));
> +        visit_type_str(v, name, (char **)&str, &error_abort);
>          break;
>      }
>      case QTYPE_QDICT: {
> -        ToJsonIterState s;
>          QDict *val = qobject_to_qdict(obj);
> -
> -        s.count = 0;
> -        s.str = str;
> -        s.indent = indent + 1;
> -        s.pretty = pretty;
> -        qstring_append_chr(str, '{');
> -        qdict_iter(val, to_json_dict_iter, &s);
> -        if (pretty) {
> -            qstring_append_printf(str, "\n%*s", 4 * indent, "");
> -        }
> -        qstring_append_chr(str, '}');
> +        visit_start_struct(v, name, NULL, 0, &error_abort);
> +        qdict_iter(val, to_json_dict_iter, v);
> +        visit_check_struct(v, &error_abort);
> +        visit_end_struct(v, NULL);
>          break;
>      }
>      case QTYPE_QLIST: {
> -        ToJsonIterState s;
>          QList *val = qobject_to_qlist(obj);
> -
> -        s.count = 0;
> -        s.str = str;
> -        s.indent = indent + 1;
> -        s.pretty = pretty;
> -        qstring_append_chr(str, '[');
> -        qlist_iter(val, (void *)to_json_list_iter, &s);
> -        if (pretty) {
> -            qstring_append_printf(str, "\n%*s", 4 * indent, "");
> -        }
> -        qstring_append_chr(str, ']');
> +        visit_start_list(v, name, NULL, 0, &error_abort);
> +        qlist_iter(val, to_json_list_iter, v);
> +        visit_end_list(v, NULL);
>          break;
>      }
>      case QTYPE_QFLOAT: {
> -        QFloat *val = qobject_to_qfloat(obj);
> +        double val = qfloat_get_double(qobject_to_qfloat(obj));
>          /* FIXME: no way inform user if we generated invalid JSON */
> -        qstring_append_json_number(str, qfloat_get_double(val));
> +        visit_type_number(v, name, &val, NULL);
>          break;
>      }
>      case QTYPE_QBOOL: {
> -        QBool *val = qobject_to_qbool(obj);
> -
> -        if (qbool_get_bool(val)) {
> -            qstring_append(str, "true");
> -        } else {
> -            qstring_append(str, "false");
> -        }
> +        bool val = qbool_get_bool(qobject_to_qbool(obj));
> +        visit_type_bool(v, name, &val, &error_abort);
>          break;
>      }
>      default:
> @@ -187,11 +137,13 @@ static void to_json(const QObject *obj, QString *str, 
> bool pretty, int indent)
>
>  QString *qobject_to_json(const QObject *obj, bool pretty)
>  {
> -    QString *str = qstring_new();
> +    char *str;
> +    Visitor *v = json_output_visitor_new(pretty, &str);
>
> -    to_json(obj, str, pretty, 0);
> -
> -    return str;
> +    qobject_visit_output(v, NULL, obj);
> +    visit_complete(v, &str);
> +    visit_free(v);
> +    return qstring_wrap_str(str);
>  }
>
>  /**