On 2017-06-08 19:43, Markus Armbruster wrote: > Marc-André Lureau <marcandre.lur...@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.lur...@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.
signature.asc
Description: OpenPGP digital signature