The dump functions could be generally useful for any qobject user or for
debugging etc.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qapi/qmp/qdict.h   |  2 ++
 include/qapi/qmp/qlist.h   |  2 ++
 include/qapi/qmp/qobject.h |  7 ++++
 block/qapi.c               | 90 +++-------------------------------------------
 qobject/qdict.c            | 30 ++++++++++++++++
 qobject/qlist.c            | 23 ++++++++++++
 qobject/qobject.c          | 19 ++++++++++
 tests/check-qjson.c        | 19 ++++++++++
 8 files changed, 106 insertions(+), 86 deletions(-)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 8c7c2b762b..1ef3bc8cda 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -91,4 +91,6 @@ QObject *qdict_crumple(const QDict *src, Error **errp);
 
 void qdict_join(QDict *dest, QDict *src, bool overwrite);
 
+char *qdict_to_string(QDict *dict, int indent);
+
 #endif /* QDICT_H */
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index c4b5fdad9b..c93ac3e15b 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -60,6 +60,8 @@ size_t qlist_size(const QList *qlist);
 QList *qobject_to_qlist(const QObject *obj);
 void qlist_destroy_obj(QObject *obj);
 
+char *qlist_to_string(QList *list, int indent);
+
 static inline const QListEntry *qlist_first(const QList *qlist)
 {
     return QTAILQ_FIRST(&qlist->head);
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index b8ddbca405..0d6ae5048a 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -101,4 +101,11 @@ static inline QObject *qnull(void)
     return &qnull_;
 }
 
+char *qobject_to_string_indent(QObject *obj, int indent);
+
+static inline char *qobject_to_string(QObject *obj)
+{
+    return qobject_to_string_indent(obj, 0);
+}
+
 #endif /* QOBJECT_H */
diff --git a/block/qapi.c b/block/qapi.c
index 2050df29e4..9b7d42e50a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -586,101 +586,19 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
     }
 }
 
-static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
-                       QDict *dict);
-static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
-                       QList *list);
-
-static void dump_qobject(fprintf_function func_fprintf, void *f,
-                         int comp_indent, QObject *obj)
-{
-    switch (qobject_type(obj)) {
-        case QTYPE_QNUM: {
-            QNum *value = qobject_to_qnum(obj);
-            char *tmp = qnum_to_string(value);
-            func_fprintf(f, "%s", tmp);
-            g_free(tmp);
-            break;
-        }
-        case QTYPE_QSTRING: {
-            QString *value = qobject_to_qstring(obj);
-            func_fprintf(f, "%s", qstring_get_str(value));
-            break;
-        }
-        case QTYPE_QDICT: {
-            QDict *value = qobject_to_qdict(obj);
-            dump_qdict(func_fprintf, f, comp_indent, value);
-            break;
-        }
-        case QTYPE_QLIST: {
-            QList *value = qobject_to_qlist(obj);
-            dump_qlist(func_fprintf, f, comp_indent, value);
-            break;
-        }
-        case QTYPE_QBOOL: {
-            QBool *value = qobject_to_qbool(obj);
-            func_fprintf(f, "%s", qbool_get_bool(value) ? "true" : "false");
-            break;
-        }
-        default:
-            abort();
-    }
-}
-
-static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
-                       QList *list)
-{
-    const QListEntry *entry;
-    int i = 0;
-
-    for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
-        QType type = qobject_type(entry->value);
-        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-        func_fprintf(f, "%*s[%i]:%c", indentation * 4, "", i,
-                     composite ? '\n' : ' ');
-        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
-        if (!composite) {
-            func_fprintf(f, "\n");
-        }
-    }
-}
-
-static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
-                       QDict *dict)
-{
-    const QDictEntry *entry;
-
-    for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
-        QType type = qobject_type(entry->value);
-        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-        char *key = g_malloc(strlen(entry->key) + 1);
-        int i;
-
-        /* replace dashes with spaces in key (variable) names */
-        for (i = 0; entry->key[i]; i++) {
-            key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
-        }
-        key[i] = 0;
-        func_fprintf(f, "%*s%s:%c", indentation * 4, "", key,
-                     composite ? '\n' : ' ');
-        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
-        if (!composite) {
-            func_fprintf(f, "\n");
-        }
-        g_free(key);
-    }
-}
-
 void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
                                    ImageInfoSpecific *info_spec)
 {
     QObject *obj, *data;
     Visitor *v = qobject_output_visitor_new(&obj);
+    char *tmp;
 
     visit_type_ImageInfoSpecific(v, NULL, &info_spec, &error_abort);
     visit_complete(v, &obj);
     data = qdict_get(qobject_to_qdict(obj), "data");
-    dump_qobject(func_fprintf, f, 1, data);
+    tmp = qobject_to_string_indent(data, 1);
+    func_fprintf(f, "%s", tmp);
+    g_free(tmp);
     qobject_decref(obj);
     visit_free(v);
 }
diff --git a/qobject/qdict.c b/qobject/qdict.c
index 65069baa1b..7e5a945c5e 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -1055,3 +1055,33 @@ void qdict_join(QDict *dest, QDict *src, bool overwrite)
         entry = next;
     }
 }
+
+char *qdict_to_string(QDict *dict, int indent)
+{
+    const QDictEntry *entry;
+    GString *str = g_string_new(NULL);
+
+    for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
+        QType type = qobject_type(entry->value);
+        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
+        char *key = g_malloc(strlen(entry->key) + 1);
+        char *val = qobject_to_string_indent(entry->value, indent + 1);
+        int i;
+
+        /* replace dashes with spaces in key (variable) names */
+        for (i = 0; entry->key[i]; i++) {
+            key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
+        }
+        key[i] = 0;
+        g_string_append_printf(str, "%*s%s:", indent * 4, "", key);
+        g_string_append_c(str, composite ? '\n' : ' ');
+        g_string_append(str, val);
+        if (!composite) {
+            g_string_append_c(str, '\n');
+        }
+        g_free(val);
+        g_free(key);
+    }
+
+    return g_string_free(str, false);
+}
diff --git a/qobject/qlist.c b/qobject/qlist.c
index 86b60cb88c..b769248290 100644
--- a/qobject/qlist.c
+++ b/qobject/qlist.c
@@ -158,3 +158,26 @@ void qlist_destroy_obj(QObject *obj)
 
     g_free(qlist);
 }
+
+char *qlist_to_string(QList *list, int indent)
+{
+    GString *str = g_string_new(NULL);
+    const QListEntry *entry;
+    int i = 0;
+
+    for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
+        QType type = qobject_type(entry->value);
+        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
+        char *val = qobject_to_string_indent(entry->value, indent + 1);
+
+        g_string_append_printf(str, "%*s[%i]:", indent * 4, "", i);
+        g_string_append_c(str, composite ? '\n' : ' ');
+        g_string_append(str, val);
+        if (!composite) {
+            g_string_append_c(str, '\n');
+        }
+        g_free(val);
+    }
+
+    return g_string_free(str, false);
+}
diff --git a/qobject/qobject.c b/qobject/qobject.c
index b0cafb66f1..64e959c54f 100644
--- a/qobject/qobject.c
+++ b/qobject/qobject.c
@@ -27,3 +27,22 @@ void qobject_destroy(QObject *obj)
     assert(QTYPE_QNULL < obj->type && obj->type < QTYPE__MAX);
     qdestroy[obj->type](obj);
 }
+
+char *qobject_to_string_indent(QObject *obj, int indent)
+{
+    switch (qobject_type(obj)) {
+    case QTYPE_QNUM:
+        return qnum_to_string(qobject_to_qnum(obj));
+    case QTYPE_QSTRING:
+        return g_strdup(qstring_get_str(qobject_to_qstring(obj)));
+    case QTYPE_QDICT:
+        return qdict_to_string(qobject_to_qdict(obj), indent);
+    case QTYPE_QLIST:
+        return qlist_to_string(qobject_to_qlist(obj), indent);
+    case QTYPE_QBOOL:
+        return g_strdup(qbool_get_bool(qobject_to_qbool(obj)) ?
+                        "true" : "false");
+    default:
+        abort();
+    }
+}
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 53f2275b9b..dd3c102bc0 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -1372,6 +1372,23 @@ static void simple_whitespace(void)
     }
 }
 
+static void qobject_to_string_test(void)
+{
+    QObject *obj;
+    char *tmp;
+
+    obj = qobject_from_json("[ 43, { 'c': { 'd' : 12 } }, [ 1, 2 ], 42 ]",
+                            &error_abort);
+    tmp = qobject_to_string(obj);
+    g_assert_cmpstr(tmp, ==,
+                    "[0]: 43\n"
+                    "[1]:\n    c:\n        d: 12\n"
+                    "[2]:\n    [0]: 1\n    [1]: 2\n"
+                    "[3]: 42\n");
+    g_free(tmp);
+    qobject_decref(obj);
+}
+
 static void simple_varargs(void)
 {
     QObject *embedded_obj;
@@ -1545,5 +1562,7 @@ int main(int argc, char **argv)
     g_test_add_func("/errors/unterminated/literal", unterminated_literal);
     g_test_add_func("/errors/limits/nesting", limits_nesting);
 
+    g_test_add_func("/qobject/to_string", qobject_to_string_test);
+
     return g_test_run();
 }
-- 
2.13.0.91.g00982b8dd

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> The dump functions could be generally useful for any qobject user or for
> debugging etc.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qapi/qmp/qdict.h   |  2 ++
>  include/qapi/qmp/qlist.h   |  2 ++
>  include/qapi/qmp/qobject.h |  7 ++++
>  block/qapi.c               | 90 +++-------------------------------------------
>  qobject/qdict.c            | 30 ++++++++++++++++
>  qobject/qlist.c            | 23 ++++++++++++
>  qobject/qobject.c          | 19 ++++++++++
>  tests/check-qjson.c        | 19 ++++++++++
>  8 files changed, 106 insertions(+), 86 deletions(-)
>
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index 8c7c2b762b..1ef3bc8cda 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -91,4 +91,6 @@ QObject *qdict_crumple(const QDict *src, Error **errp);
>  
>  void qdict_join(QDict *dest, QDict *src, bool overwrite);
>  
> +char *qdict_to_string(QDict *dict, int indent);
> +
>  #endif /* QDICT_H */
> diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
> index c4b5fdad9b..c93ac3e15b 100644
> --- a/include/qapi/qmp/qlist.h
> +++ b/include/qapi/qmp/qlist.h
> @@ -60,6 +60,8 @@ size_t qlist_size(const QList *qlist);
>  QList *qobject_to_qlist(const QObject *obj);
>  void qlist_destroy_obj(QObject *obj);
>  
> +char *qlist_to_string(QList *list, int indent);
> +
>  static inline const QListEntry *qlist_first(const QList *qlist)
>  {
>      return QTAILQ_FIRST(&qlist->head);
> diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
> index b8ddbca405..0d6ae5048a 100644
> --- a/include/qapi/qmp/qobject.h
> +++ b/include/qapi/qmp/qobject.h
> @@ -101,4 +101,11 @@ static inline QObject *qnull(void)
>      return &qnull_;
>  }
>  
> +char *qobject_to_string_indent(QObject *obj, int indent);
> +
> +static inline char *qobject_to_string(QObject *obj)
> +{
> +    return qobject_to_string_indent(obj, 0);
> +}
> +
>  #endif /* QOBJECT_H */
> diff --git a/block/qapi.c b/block/qapi.c
> index 2050df29e4..9b7d42e50a 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -586,101 +586,19 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
>      }
>  }
>  
> -static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
> -                       QDict *dict);
> -static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
> -                       QList *list);
> -
> -static void dump_qobject(fprintf_function func_fprintf, void *f,
> -                         int comp_indent, QObject *obj)
> -{
> -    switch (qobject_type(obj)) {
> -        case QTYPE_QNUM: {
> -            QNum *value = qobject_to_qnum(obj);
> -            char *tmp = qnum_to_string(value);
> -            func_fprintf(f, "%s", tmp);
> -            g_free(tmp);
> -            break;
> -        }
> -        case QTYPE_QSTRING: {
> -            QString *value = qobject_to_qstring(obj);
> -            func_fprintf(f, "%s", qstring_get_str(value));
> -            break;
> -        }
> -        case QTYPE_QDICT: {
> -            QDict *value = qobject_to_qdict(obj);
> -            dump_qdict(func_fprintf, f, comp_indent, value);
> -            break;
> -        }
> -        case QTYPE_QLIST: {
> -            QList *value = qobject_to_qlist(obj);
> -            dump_qlist(func_fprintf, f, comp_indent, value);
> -            break;
> -        }
> -        case QTYPE_QBOOL: {
> -            QBool *value = qobject_to_qbool(obj);
> -            func_fprintf(f, "%s", qbool_get_bool(value) ? "true" : "false");
> -            break;
> -        }
> -        default:
> -            abort();
> -    }
> -}
> -
> -static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
> -                       QList *list)
> -{
> -    const QListEntry *entry;
> -    int i = 0;
> -
> -    for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
> -        QType type = qobject_type(entry->value);
> -        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> -        func_fprintf(f, "%*s[%i]:%c", indentation * 4, "", i,
> -                     composite ? '\n' : ' ');
> -        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
> -        if (!composite) {
> -            func_fprintf(f, "\n");
> -        }
> -    }
> -}
> -
> -static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
> -                       QDict *dict)
> -{
> -    const QDictEntry *entry;
> -
> -    for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
> -        QType type = qobject_type(entry->value);
> -        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> -        char *key = g_malloc(strlen(entry->key) + 1);
> -        int i;
> -
> -        /* replace dashes with spaces in key (variable) names */
> -        for (i = 0; entry->key[i]; i++) {
> -            key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
> -        }
> -        key[i] = 0;
> -        func_fprintf(f, "%*s%s:%c", indentation * 4, "", key,
> -                     composite ? '\n' : ' ');
> -        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
> -        if (!composite) {
> -            func_fprintf(f, "\n");
> -        }
> -        g_free(key);
> -    }
> -}
> -
>  void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
>                                     ImageInfoSpecific *info_spec)
>  {
>      QObject *obj, *data;
>      Visitor *v = qobject_output_visitor_new(&obj);
> +    char *tmp;
>  
>      visit_type_ImageInfoSpecific(v, NULL, &info_spec, &error_abort);
>      visit_complete(v, &obj);
>      data = qdict_get(qobject_to_qdict(obj), "data");
> -    dump_qobject(func_fprintf, f, 1, data);
> +    tmp = qobject_to_string_indent(data, 1);
> +    func_fprintf(f, "%s", tmp);
> +    g_free(tmp);
>      qobject_decref(obj);
>      visit_free(v);
>  }

The title claims "move dump_qobject() from block/ to qobject/", but
that's not what the patch does.  It *replaces* dump_qobject() by
qobject_to_string().  The former dumps to a callback, the latter to a
dynamic string buffer.

Providing dump functionality in one way doesn't preclude the other way:
given callback, one could define a callback that builds up a string
buffer, and given buffer, one could (and you actually do) pass the
buffer to a callback.  That's less efficient, though.

Trading efficiency for ease-of-use should be okay here, but I'm cc'ing
Max and Kevin to double-check.

Two ways forward:

1. Get Max / Kevin's blessing, change the commit message to match the
code.

2. Change the code to match the commit message.

> diff --git a/qobject/qdict.c b/qobject/qdict.c
> index 65069baa1b..7e5a945c5e 100644
> --- a/qobject/qdict.c
> +++ b/qobject/qdict.c
> @@ -1055,3 +1055,33 @@ void qdict_join(QDict *dest, QDict *src, bool overwrite)
>          entry = next;
>      }
>  }
> +
> +char *qdict_to_string(QDict *dict, int indent)
> +{
> +    const QDictEntry *entry;
> +    GString *str = g_string_new(NULL);
> +
> +    for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
> +        QType type = qobject_type(entry->value);
> +        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> +        char *key = g_malloc(strlen(entry->key) + 1);
> +        char *val = qobject_to_string_indent(entry->value, indent + 1);
> +        int i;
> +
> +        /* replace dashes with spaces in key (variable) names */
> +        for (i = 0; entry->key[i]; i++) {
> +            key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
> +        }
> +        key[i] = 0;
> +        g_string_append_printf(str, "%*s%s:", indent * 4, "", key);
> +        g_string_append_c(str, composite ? '\n' : ' ');
> +        g_string_append(str, val);
> +        if (!composite) {
> +            g_string_append_c(str, '\n');
> +        }
> +        g_free(val);
> +        g_free(key);
> +    }
> +
> +    return g_string_free(str, false);
> +}
> diff --git a/qobject/qlist.c b/qobject/qlist.c
> index 86b60cb88c..b769248290 100644
> --- a/qobject/qlist.c
> +++ b/qobject/qlist.c
> @@ -158,3 +158,26 @@ void qlist_destroy_obj(QObject *obj)
>  
>      g_free(qlist);
>  }
> +
> +char *qlist_to_string(QList *list, int indent)
> +{
> +    GString *str = g_string_new(NULL);
> +    const QListEntry *entry;
> +    int i = 0;
> +
> +    for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
> +        QType type = qobject_type(entry->value);
> +        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> +        char *val = qobject_to_string_indent(entry->value, indent + 1);
> +
> +        g_string_append_printf(str, "%*s[%i]:", indent * 4, "", i);
> +        g_string_append_c(str, composite ? '\n' : ' ');
> +        g_string_append(str, val);
> +        if (!composite) {
> +            g_string_append_c(str, '\n');
> +        }
> +        g_free(val);
> +    }
> +
> +    return g_string_free(str, false);
> +}
> diff --git a/qobject/qobject.c b/qobject/qobject.c
> index b0cafb66f1..64e959c54f 100644
> --- a/qobject/qobject.c
> +++ b/qobject/qobject.c
> @@ -27,3 +27,22 @@ void qobject_destroy(QObject *obj)
>      assert(QTYPE_QNULL < obj->type && obj->type < QTYPE__MAX);
>      qdestroy[obj->type](obj);
>  }
> +
> +char *qobject_to_string_indent(QObject *obj, int indent)
> +{
> +    switch (qobject_type(obj)) {
> +    case QTYPE_QNUM:
> +        return qnum_to_string(qobject_to_qnum(obj));
> +    case QTYPE_QSTRING:
> +        return g_strdup(qstring_get_str(qobject_to_qstring(obj)));
> +    case QTYPE_QDICT:
> +        return qdict_to_string(qobject_to_qdict(obj), indent);
> +    case QTYPE_QLIST:
> +        return qlist_to_string(qobject_to_qlist(obj), indent);
> +    case QTYPE_QBOOL:
> +        return g_strdup(qbool_get_bool(qobject_to_qbool(obj)) ?
> +                        "true" : "false");
> +    default:
> +        abort();
> +    }
> +}
> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> index 53f2275b9b..dd3c102bc0 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qjson.c
> @@ -1372,6 +1372,23 @@ static void simple_whitespace(void)
>      }
>  }
>  
> +static void qobject_to_string_test(void)
> +{
> +    QObject *obj;
> +    char *tmp;
> +
> +    obj = qobject_from_json("[ 43, { 'c': { 'd' : 12 } }, [ 1, 2 ], 42 ]",
> +                            &error_abort);
> +    tmp = qobject_to_string(obj);
> +    g_assert_cmpstr(tmp, ==,
> +                    "[0]: 43\n"
> +                    "[1]:\n    c:\n        d: 12\n"
> +                    "[2]:\n    [0]: 1\n    [1]: 2\n"
> +                    "[3]: 42\n");
> +    g_free(tmp);
> +    qobject_decref(obj);
> +}
> +
>  static void simple_varargs(void)
>  {
>      QObject *embedded_obj;
> @@ -1545,5 +1562,7 @@ int main(int argc, char **argv)
>      g_test_add_func("/errors/unterminated/literal", unterminated_literal);
>      g_test_add_func("/errors/limits/nesting", limits_nesting);
>  
> +    g_test_add_func("/qobject/to_string", qobject_to_string_test);
> +
>      return g_test_run();
>  }

----- Original Message -----
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
> > The dump functions could be generally useful for any qobject user or for
> > debugging etc.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/qapi/qmp/qdict.h   |  2 ++
> >  include/qapi/qmp/qlist.h   |  2 ++
> >  include/qapi/qmp/qobject.h |  7 ++++
> >  block/qapi.c               | 90
> >  +++-------------------------------------------
> >  qobject/qdict.c            | 30 ++++++++++++++++
> >  qobject/qlist.c            | 23 ++++++++++++
> >  qobject/qobject.c          | 19 ++++++++++
> >  tests/check-qjson.c        | 19 ++++++++++
> >  8 files changed, 106 insertions(+), 86 deletions(-)
> >
> > diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> > index 8c7c2b762b..1ef3bc8cda 100644
> > --- a/include/qapi/qmp/qdict.h
> > +++ b/include/qapi/qmp/qdict.h
> > @@ -91,4 +91,6 @@ QObject *qdict_crumple(const QDict *src, Error **errp);
> >  
> >  void qdict_join(QDict *dest, QDict *src, bool overwrite);
> >  
> > +char *qdict_to_string(QDict *dict, int indent);
> > +
> >  #endif /* QDICT_H */
> > diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
> > index c4b5fdad9b..c93ac3e15b 100644
> > --- a/include/qapi/qmp/qlist.h
> > +++ b/include/qapi/qmp/qlist.h
> > @@ -60,6 +60,8 @@ size_t qlist_size(const QList *qlist);
> >  QList *qobject_to_qlist(const QObject *obj);
> >  void qlist_destroy_obj(QObject *obj);
> >  
> > +char *qlist_to_string(QList *list, int indent);
> > +
> >  static inline const QListEntry *qlist_first(const QList *qlist)
> >  {
> >      return QTAILQ_FIRST(&qlist->head);
> > diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
> > index b8ddbca405..0d6ae5048a 100644
> > --- a/include/qapi/qmp/qobject.h
> > +++ b/include/qapi/qmp/qobject.h
> > @@ -101,4 +101,11 @@ static inline QObject *qnull(void)
> >      return &qnull_;
> >  }
> >  
> > +char *qobject_to_string_indent(QObject *obj, int indent);
> > +
> > +static inline char *qobject_to_string(QObject *obj)
> > +{
> > +    return qobject_to_string_indent(obj, 0);
> > +}
> > +
> >  #endif /* QOBJECT_H */
> > diff --git a/block/qapi.c b/block/qapi.c
> > index 2050df29e4..9b7d42e50a 100644
> > --- a/block/qapi.c
> > +++ b/block/qapi.c
> > @@ -586,101 +586,19 @@ void bdrv_snapshot_dump(fprintf_function
> > func_fprintf, void *f,
> >      }
> >  }
> >  
> > -static void dump_qdict(fprintf_function func_fprintf, void *f, int
> > indentation,
> > -                       QDict *dict);
> > -static void dump_qlist(fprintf_function func_fprintf, void *f, int
> > indentation,
> > -                       QList *list);
> > -
> > -static void dump_qobject(fprintf_function func_fprintf, void *f,
> > -                         int comp_indent, QObject *obj)
> > -{
> > -    switch (qobject_type(obj)) {
> > -        case QTYPE_QNUM: {
> > -            QNum *value = qobject_to_qnum(obj);
> > -            char *tmp = qnum_to_string(value);
> > -            func_fprintf(f, "%s", tmp);
> > -            g_free(tmp);
> > -            break;
> > -        }
> > -        case QTYPE_QSTRING: {
> > -            QString *value = qobject_to_qstring(obj);
> > -            func_fprintf(f, "%s", qstring_get_str(value));
> > -            break;
> > -        }
> > -        case QTYPE_QDICT: {
> > -            QDict *value = qobject_to_qdict(obj);
> > -            dump_qdict(func_fprintf, f, comp_indent, value);
> > -            break;
> > -        }
> > -        case QTYPE_QLIST: {
> > -            QList *value = qobject_to_qlist(obj);
> > -            dump_qlist(func_fprintf, f, comp_indent, value);
> > -            break;
> > -        }
> > -        case QTYPE_QBOOL: {
> > -            QBool *value = qobject_to_qbool(obj);
> > -            func_fprintf(f, "%s", qbool_get_bool(value) ? "true" :
> > "false");
> > -            break;
> > -        }
> > -        default:
> > -            abort();
> > -    }
> > -}
> > -
> > -static void dump_qlist(fprintf_function func_fprintf, void *f, int
> > indentation,
> > -                       QList *list)
> > -{
> > -    const QListEntry *entry;
> > -    int i = 0;
> > -
> > -    for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++)
> > {
> > -        QType type = qobject_type(entry->value);
> > -        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> > -        func_fprintf(f, "%*s[%i]:%c", indentation * 4, "", i,
> > -                     composite ? '\n' : ' ');
> > -        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
> > -        if (!composite) {
> > -            func_fprintf(f, "\n");
> > -        }
> > -    }
> > -}
> > -
> > -static void dump_qdict(fprintf_function func_fprintf, void *f, int
> > indentation,
> > -                       QDict *dict)
> > -{
> > -    const QDictEntry *entry;
> > -
> > -    for (entry = qdict_first(dict); entry; entry = qdict_next(dict,
> > entry)) {
> > -        QType type = qobject_type(entry->value);
> > -        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> > -        char *key = g_malloc(strlen(entry->key) + 1);
> > -        int i;
> > -
> > -        /* replace dashes with spaces in key (variable) names */
> > -        for (i = 0; entry->key[i]; i++) {
> > -            key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
> > -        }
> > -        key[i] = 0;
> > -        func_fprintf(f, "%*s%s:%c", indentation * 4, "", key,
> > -                     composite ? '\n' : ' ');
> > -        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
> > -        if (!composite) {
> > -            func_fprintf(f, "\n");
> > -        }
> > -        g_free(key);
> > -    }
> > -}
> > -
> >  void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
> >                                     ImageInfoSpecific *info_spec)
> >  {
> >      QObject *obj, *data;
> >      Visitor *v = qobject_output_visitor_new(&obj);
> > +    char *tmp;
> >  
> >      visit_type_ImageInfoSpecific(v, NULL, &info_spec, &error_abort);
> >      visit_complete(v, &obj);
> >      data = qdict_get(qobject_to_qdict(obj), "data");
> > -    dump_qobject(func_fprintf, f, 1, data);
> > +    tmp = qobject_to_string_indent(data, 1);
> > +    func_fprintf(f, "%s", tmp);
> > +    g_free(tmp);
> >      qobject_decref(obj);
> >      visit_free(v);
> >  }
> 
> The title claims "move dump_qobject() from block/ to qobject/", but
> that's not what the patch does.  It *replaces* dump_qobject() by
> qobject_to_string().  The former dumps to a callback, the latter to a
> dynamic string buffer.
> 
> Providing dump functionality in one way doesn't preclude the other way:
> given callback, one could define a callback that builds up a string
> buffer, and given buffer, one could (and you actually do) pass the
> buffer to a callback.  That's less efficient, though.
> 
> Trading efficiency for ease-of-use should be okay here, but I'm cc'ing
> Max and Kevin to double-check.

I believe convenience is more important than efficiency here. It's easy to call qobject_to_string(foo) from gdb for example, with a callback, it's less easy.

(fprintf or monitor_fprintf will both build an internal buffer anyway, efficiency is probably similar)

> 
> Two ways forward:
> 
> 1. Get Max / Kevin's blessing, change the commit message to match the
> code.
> 
> 2. Change the code to match the commit message.
> 
> > diff --git a/qobject/qdict.c b/qobject/qdict.c
> > index 65069baa1b..7e5a945c5e 100644
> > --- a/qobject/qdict.c
> > +++ b/qobject/qdict.c
> > @@ -1055,3 +1055,33 @@ void qdict_join(QDict *dest, QDict *src, bool
> > overwrite)
> >          entry = next;
> >      }
> >  }
> > +
> > +char *qdict_to_string(QDict *dict, int indent)
> > +{
> > +    const QDictEntry *entry;
> > +    GString *str = g_string_new(NULL);
> > +
> > +    for (entry = qdict_first(dict); entry; entry = qdict_next(dict,
> > entry)) {
> > +        QType type = qobject_type(entry->value);
> > +        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> > +        char *key = g_malloc(strlen(entry->key) + 1);
> > +        char *val = qobject_to_string_indent(entry->value, indent + 1);
> > +        int i;
> > +
> > +        /* replace dashes with spaces in key (variable) names */
> > +        for (i = 0; entry->key[i]; i++) {
> > +            key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
> > +        }
> > +        key[i] = 0;
> > +        g_string_append_printf(str, "%*s%s:", indent * 4, "", key);
> > +        g_string_append_c(str, composite ? '\n' : ' ');
> > +        g_string_append(str, val);
> > +        if (!composite) {
> > +            g_string_append_c(str, '\n');
> > +        }
> > +        g_free(val);
> > +        g_free(key);
> > +    }
> > +
> > +    return g_string_free(str, false);
> > +}
> > diff --git a/qobject/qlist.c b/qobject/qlist.c
> > index 86b60cb88c..b769248290 100644
> > --- a/qobject/qlist.c
> > +++ b/qobject/qlist.c
> > @@ -158,3 +158,26 @@ void qlist_destroy_obj(QObject *obj)
> >  
> >      g_free(qlist);
> >  }
> > +
> > +char *qlist_to_string(QList *list, int indent)
> > +{
> > +    GString *str = g_string_new(NULL);
> > +    const QListEntry *entry;
> > +    int i = 0;
> > +
> > +    for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++)
> > {
> > +        QType type = qobject_type(entry->value);
> > +        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
> > +        char *val = qobject_to_string_indent(entry->value, indent + 1);
> > +
> > +        g_string_append_printf(str, "%*s[%i]:", indent * 4, "", i);
> > +        g_string_append_c(str, composite ? '\n' : ' ');
> > +        g_string_append(str, val);
> > +        if (!composite) {
> > +            g_string_append_c(str, '\n');
> > +        }
> > +        g_free(val);
> > +    }
> > +
> > +    return g_string_free(str, false);
> > +}
> > diff --git a/qobject/qobject.c b/qobject/qobject.c
> > index b0cafb66f1..64e959c54f 100644
> > --- a/qobject/qobject.c
> > +++ b/qobject/qobject.c
> > @@ -27,3 +27,22 @@ void qobject_destroy(QObject *obj)
> >      assert(QTYPE_QNULL < obj->type && obj->type < QTYPE__MAX);
> >      qdestroy[obj->type](obj);
> >  }
> > +
> > +char *qobject_to_string_indent(QObject *obj, int indent)
> > +{
> > +    switch (qobject_type(obj)) {
> > +    case QTYPE_QNUM:
> > +        return qnum_to_string(qobject_to_qnum(obj));
> > +    case QTYPE_QSTRING:
> > +        return g_strdup(qstring_get_str(qobject_to_qstring(obj)));
> > +    case QTYPE_QDICT:
> > +        return qdict_to_string(qobject_to_qdict(obj), indent);
> > +    case QTYPE_QLIST:
> > +        return qlist_to_string(qobject_to_qlist(obj), indent);
> > +    case QTYPE_QBOOL:
> > +        return g_strdup(qbool_get_bool(qobject_to_qbool(obj)) ?
> > +                        "true" : "false");
> > +    default:
> > +        abort();
> > +    }
> > +}
> > diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> > index 53f2275b9b..dd3c102bc0 100644
> > --- a/tests/check-qjson.c
> > +++ b/tests/check-qjson.c
> > @@ -1372,6 +1372,23 @@ static void simple_whitespace(void)
> >      }
> >  }
> >  
> > +static void qobject_to_string_test(void)
> > +{
> > +    QObject *obj;
> > +    char *tmp;
> > +
> > +    obj = qobject_from_json("[ 43, { 'c': { 'd' : 12 } }, [ 1, 2 ], 42 ]",
> > +                            &error_abort);
> > +    tmp = qobject_to_string(obj);
> > +    g_assert_cmpstr(tmp, ==,
> > +                    "[0]: 43\n"
> > +                    "[1]:\n    c:\n        d: 12\n"
> > +                    "[2]:\n    [0]: 1\n    [1]: 2\n"
> > +                    "[3]: 42\n");
> > +    g_free(tmp);
> > +    qobject_decref(obj);
> > +}
> > +
> >  static void simple_varargs(void)
> >  {
> >      QObject *embedded_obj;
> > @@ -1545,5 +1562,7 @@ int main(int argc, char **argv)
> >      g_test_add_func("/errors/unterminated/literal", unterminated_literal);
> >      g_test_add_func("/errors/limits/nesting", limits_nesting);
> >  
> > +    g_test_add_func("/qobject/to_string", qobject_to_string_test);
> > +
> >      return g_test_run();
> >  }
> 

Hi

----- Original Message -----
> > 
> > The title claims "move dump_qobject() from block/ to qobject/", but
> > that's not what the patch does.  It *replaces* dump_qobject() by
> > qobject_to_string().  The former dumps to a callback, the latter to a
> > dynamic string buffer.
> > 
> > Providing dump functionality in one way doesn't preclude the other way:
> > given callback, one could define a callback that builds up a string
> > buffer, and given buffer, one could (and you actually do) pass the
> > buffer to a callback.  That's less efficient, though.
> > 
> > Trading efficiency for ease-of-use should be okay here, but I'm cc'ing
> > Max and Kevin to double-check.
> 
> I believe convenience is more important than efficiency here. It's easy to
> call qobject_to_string(foo) from gdb for example, with a callback, it's less
> easy.
> 
> (fprintf or monitor_fprintf will both build an internal buffer anyway,
> efficiency is probably similar)
> 

Hmm, there are more allocations in qobject_to_string() though

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> ----- Original Message -----
>> > 
>> > The title claims "move dump_qobject() from block/ to qobject/", but
>> > that's not what the patch does.  It *replaces* dump_qobject() by
>> > qobject_to_string().  The former dumps to a callback, the latter to a
>> > dynamic string buffer.
>> > 
>> > Providing dump functionality in one way doesn't preclude the other way:
>> > given callback, one could define a callback that builds up a string
>> > buffer, and given buffer, one could (and you actually do) pass the
>> > buffer to a callback.  That's less efficient, though.
>> > 
>> > Trading efficiency for ease-of-use should be okay here, but I'm cc'ing
>> > Max and Kevin to double-check.
>> 
>> I believe convenience is more important than efficiency here. It's easy to
>> call qobject_to_string(foo) from gdb for example, with a callback, it's less
>> easy.
>> 
>> (fprintf or monitor_fprintf will both build an internal buffer anyway,
>> efficiency is probably similar)

monitor_vprintf() formats to a dynamic buffer, which it passes to
monitor_puts().  monitor_puts() uses a line buffer.

fprintf() can be unbuffered, line-buffered, or fully buffered.

Converting everything to a string first is different: the string buffer
holds *everything* rather than just a line or some fixed size.

> Hmm, there are more allocations in qobject_to_string() though

On 06/09/2017 07:40 AM, Markus Armbruster wrote:
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
>> Hi
>>
>> ----- Original Message -----
>>>>
>>>> The title claims "move dump_qobject() from block/ to qobject/", but
>>>> that's not what the patch does.  It *replaces* dump_qobject() by
>>>> qobject_to_string().  The former dumps to a callback, the latter to a
>>>> dynamic string buffer.
>>>>
>>>> Providing dump functionality in one way doesn't preclude the other way:
>>>> given callback, one could define a callback that builds up a string
>>>> buffer, and given buffer, one could (and you actually do) pass the
>>>> buffer to a callback.  That's less efficient, though.
>>>>
>>>> Trading efficiency for ease-of-use should be okay here, but I'm cc'ing
>>>> Max and Kevin to double-check.
>>>
>>> I believe convenience is more important than efficiency here. It's easy to
>>> call qobject_to_string(foo) from gdb for example, with a callback, it's less
>>> easy.
>>>
>>> (fprintf or monitor_fprintf will both build an internal buffer anyway,
>>> efficiency is probably similar)
> 
> monitor_vprintf() formats to a dynamic buffer, which it passes to
> monitor_puts().  monitor_puts() uses a line buffer.
> 
> fprintf() can be unbuffered, line-buffered, or fully buffered.
> 
> Converting everything to a string first is different: the string buffer
> holds *everything* rather than just a line or some fixed size.

My patch series to create a QAPI-to-JSON output visitor, and convert the
existing qobject-to-json conversion to be a thin wrapper over the JSON
visitor, is a natural place to create a visitor constructor that can
take a callback (where the callback either appends to a QString or uses
pwrite() (no need to spend time with the slower printf()) to go straight
to stdout.  I'll have to revive that series on top of your work.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 2017-06-08 19:43, Markus Armbruster wrote:
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
>> The dump functions could be generally useful for any qobject user or for
>> debugging etc.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  include/qapi/qmp/qdict.h   |  2 ++
>>  include/qapi/qmp/qlist.h   |  2 ++
>>  include/qapi/qmp/qobject.h |  7 ++++
>>  block/qapi.c               | 90 +++-------------------------------------------
>>  qobject/qdict.c            | 30 ++++++++++++++++
>>  qobject/qlist.c            | 23 ++++++++++++
>>  qobject/qobject.c          | 19 ++++++++++
>>  tests/check-qjson.c        | 19 ++++++++++
>>  8 files changed, 106 insertions(+), 86 deletions(-)
>>
>> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
>> index 8c7c2b762b..1ef3bc8cda 100644
>> --- a/include/qapi/qmp/qdict.h
>> +++ b/include/qapi/qmp/qdict.h
>> @@ -91,4 +91,6 @@ QObject *qdict_crumple(const QDict *src, Error **errp);
>>  
>>  void qdict_join(QDict *dest, QDict *src, bool overwrite);
>>  
>> +char *qdict_to_string(QDict *dict, int indent);
>> +
>>  #endif /* QDICT_H */
>> diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
>> index c4b5fdad9b..c93ac3e15b 100644
>> --- a/include/qapi/qmp/qlist.h
>> +++ b/include/qapi/qmp/qlist.h
>> @@ -60,6 +60,8 @@ size_t qlist_size(const QList *qlist);
>>  QList *qobject_to_qlist(const QObject *obj);
>>  void qlist_destroy_obj(QObject *obj);
>>  
>> +char *qlist_to_string(QList *list, int indent);
>> +
>>  static inline const QListEntry *qlist_first(const QList *qlist)
>>  {
>>      return QTAILQ_FIRST(&qlist->head);
>> diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
>> index b8ddbca405..0d6ae5048a 100644
>> --- a/include/qapi/qmp/qobject.h
>> +++ b/include/qapi/qmp/qobject.h
>> @@ -101,4 +101,11 @@ static inline QObject *qnull(void)
>>      return &qnull_;
>>  }
>>  
>> +char *qobject_to_string_indent(QObject *obj, int indent);
>> +
>> +static inline char *qobject_to_string(QObject *obj)
>> +{
>> +    return qobject_to_string_indent(obj, 0);
>> +}
>> +
>>  #endif /* QOBJECT_H */
>> diff --git a/block/qapi.c b/block/qapi.c
>> index 2050df29e4..9b7d42e50a 100644
>> --- a/block/qapi.c
>> +++ b/block/qapi.c
>> @@ -586,101 +586,19 @@ void bdrv_snapshot_dump(fprintf_function func_fprintf, void *f,
>>      }
>>  }
>>  
>> -static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
>> -                       QDict *dict);
>> -static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
>> -                       QList *list);
>> -
>> -static void dump_qobject(fprintf_function func_fprintf, void *f,
>> -                         int comp_indent, QObject *obj)
>> -{
>> -    switch (qobject_type(obj)) {
>> -        case QTYPE_QNUM: {
>> -            QNum *value = qobject_to_qnum(obj);
>> -            char *tmp = qnum_to_string(value);
>> -            func_fprintf(f, "%s", tmp);
>> -            g_free(tmp);
>> -            break;
>> -        }
>> -        case QTYPE_QSTRING: {
>> -            QString *value = qobject_to_qstring(obj);
>> -            func_fprintf(f, "%s", qstring_get_str(value));
>> -            break;
>> -        }
>> -        case QTYPE_QDICT: {
>> -            QDict *value = qobject_to_qdict(obj);
>> -            dump_qdict(func_fprintf, f, comp_indent, value);
>> -            break;
>> -        }
>> -        case QTYPE_QLIST: {
>> -            QList *value = qobject_to_qlist(obj);
>> -            dump_qlist(func_fprintf, f, comp_indent, value);
>> -            break;
>> -        }
>> -        case QTYPE_QBOOL: {
>> -            QBool *value = qobject_to_qbool(obj);
>> -            func_fprintf(f, "%s", qbool_get_bool(value) ? "true" : "false");
>> -            break;
>> -        }
>> -        default:
>> -            abort();
>> -    }
>> -}
>> -
>> -static void dump_qlist(fprintf_function func_fprintf, void *f, int indentation,
>> -                       QList *list)
>> -{
>> -    const QListEntry *entry;
>> -    int i = 0;
>> -
>> -    for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
>> -        QType type = qobject_type(entry->value);
>> -        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
>> -        func_fprintf(f, "%*s[%i]:%c", indentation * 4, "", i,
>> -                     composite ? '\n' : ' ');
>> -        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
>> -        if (!composite) {
>> -            func_fprintf(f, "\n");
>> -        }
>> -    }
>> -}
>> -
>> -static void dump_qdict(fprintf_function func_fprintf, void *f, int indentation,
>> -                       QDict *dict)
>> -{
>> -    const QDictEntry *entry;
>> -
>> -    for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
>> -        QType type = qobject_type(entry->value);
>> -        bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
>> -        char *key = g_malloc(strlen(entry->key) + 1);
>> -        int i;
>> -
>> -        /* replace dashes with spaces in key (variable) names */
>> -        for (i = 0; entry->key[i]; i++) {
>> -            key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
>> -        }
>> -        key[i] = 0;
>> -        func_fprintf(f, "%*s%s:%c", indentation * 4, "", key,
>> -                     composite ? '\n' : ' ');
>> -        dump_qobject(func_fprintf, f, indentation + 1, entry->value);
>> -        if (!composite) {
>> -            func_fprintf(f, "\n");
>> -        }
>> -        g_free(key);
>> -    }
>> -}
>> -
>>  void bdrv_image_info_specific_dump(fprintf_function func_fprintf, void *f,
>>                                     ImageInfoSpecific *info_spec)
>>  {
>>      QObject *obj, *data;
>>      Visitor *v = qobject_output_visitor_new(&obj);
>> +    char *tmp;
>>  
>>      visit_type_ImageInfoSpecific(v, NULL, &info_spec, &error_abort);
>>      visit_complete(v, &obj);
>>      data = qdict_get(qobject_to_qdict(obj), "data");
>> -    dump_qobject(func_fprintf, f, 1, data);
>> +    tmp = qobject_to_string_indent(data, 1);
>> +    func_fprintf(f, "%s", tmp);
>> +    g_free(tmp);
>>      qobject_decref(obj);
>>      visit_free(v);
>>  }
> 
> The title claims "move dump_qobject() from block/ to qobject/", but
> that's not what the patch does.  It *replaces* dump_qobject() by
> qobject_to_string().  The former dumps to a callback, the latter to a
> dynamic string buffer.
> 
> Providing dump functionality in one way doesn't preclude the other way:
> given callback, one could define a callback that builds up a string
> buffer, and given buffer, one could (and you actually do) pass the
> buffer to a callback.  That's less efficient, though.
> 
> Trading efficiency for ease-of-use should be okay here, but I'm cc'ing
> Max and Kevin to double-check.

Given that this function is called only by HMP's "info block -v" and
qemu-img, I don't think efficiency is too important. In the former case,
it's your own fault for using HMP, in the latter you'll have only a
single image anyway.

So I'm OK with this change.

Max

> 
> Two ways forward:
> 
> 1. Get Max / Kevin's blessing, change the commit message to match the
> code.
> 
> 2. Change the code to match the commit message.