|
From: | Max Reitz |
Subject: | Re: [Qemu-devel] [PATCH v3 04/21] qapi: extend qdict_flatten() for QLists |
Date: | Fri, 13 Dec 2013 16:52:22 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.1 |
On 12.12.2013 07:17, Fam Zheng wrote:
On 2013年12月12日 02:10, Max Reitz wrote:Reversing qdict_array_split(), qdict_flatten() should flatten QLists as well by interpreting them as QDicts where every entry's key is its index. This allows bringing QDicts with QLists from QMP commands to the same form as they would be given as command-line options, thereby allowing them to be parsed the same way. Signed-off-by: Max Reitz <address@hidden> ---The logic looks fine, just a few questions about assertions below.qobject/qdict.c | 45 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 41 insertions(+), 4 deletions(-) diff --git a/qobject/qdict.c b/qobject/qdict.c index fca1902..d7ce4b3 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -477,7 +477,40 @@ static void qdict_destroy_obj(QObject *obj) g_free(qdict); }-static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix)+static void qdict_flatten_qdict(QDict *qdict, QDict *target, + const char *prefix); ++static void qdict_flatten_qlist(QList *qlist, QDict *target, const char *prefix)+{ + QObject *value; + const QListEntry *entry; + char *new_key; + int i; ++ /* This function is never called with prefix == NULL, i.e., it is always + * called from within qdict_flatten_q(list|dict)(). Therefore, it does not + * need to remove list entries during the iteration (the whole list will be + * deleted eventually anyway from qdict_flatten_qdict()). Also, prefix can+ * never be NULL. */Then maybe assert this?
Yes, I'll do that.
+ + entry = qlist_first(qlist); + + for (i = 0; entry; entry = qlist_next(entry), i++) { + value = qlist_entry_obj(entry); + + qobject_incref(value); + new_key = g_strdup_printf("%s.%i", prefix, i); + qdict_put_obj(target, new_key, value); + + if (qobject_type(value) == QTYPE_QDICT) { + qdict_flatten_qdict(qobject_to_qdict(value), target, new_key); + } else {And maybe assert the type are not something other than dict and list?
I think I rather forgot to support other types. Those should be moved to the target QDict, just as qdict_flatten_qdict() does it.
+ qdict_flatten_qlist(qobject_to_qlist(value), target, new_key); + } + } +} ++static void qdict_flatten_qdict(QDict *qdict, QDict *target, const char *prefix){ QObject *value; const QDictEntry *entry, *next;@@ -500,8 +533,12 @@ static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix)if (qobject_type(value) == QTYPE_QDICT) { /* Entries of QDicts are processed recursively, the QDict object * itself disappears. */ - qdict_do_flatten(qobject_to_qdict(value), target, - new_key ? new_key : entry->key); + qdict_flatten_qdict(qobject_to_qdict(value), target, + new_key ? new_key : entry->key); + delete = true; + } else if (qobject_type(value) == QTYPE_QLIST) { + qdict_flatten_qlist(qobject_to_qlist(value), target, + new_key ? new_key : entry->key); delete = true; } else if (prefix) { /* All other objects are moved to the target unchanged. */@@ -531,7 +568,7 @@ static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix)*/ void qdict_flatten(QDict *qdict) { - qdict_do_flatten(qdict, qdict, NULL); + qdict_flatten_qdict(qdict, qdict, NULL);If there was an assumption that the top level is a qdict, also assert that here?
Well, the type is already QDict *, therefore I think we are safe to assume it is indeed one – if that is what you meant. ;-)
The question to me would be whether we should be supporting QLists here, too. Since this would make qdict_flatten_qlist() more complex, I think we should not do that until it is required.
Thank you, Max
[Prev in Thread] | Current Thread | [Next in Thread] |