| Message ID | 1374584606-5615-18-git-send-email-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 07/23/2013 07:03 AM, Kevin Wolf wrote: > qdict_flatten(): For each nested QDict with key x, all fields with key y > are moved to this QDict and their key is renamed to "x.y". This operation > is applied recursively for nested QDicts. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > include/qapi/qmp/qdict.h | 1 + > qobject/qdict.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 51 insertions(+) > + while (entry != NULL) { > + > + next = qdict_next(qdict, entry); next points to a position in the unmodified qdict... > + value = qdict_entry_value(entry); > + new_key = NULL; > + delete = false; > + > + if (prefix) { > + qobject_incref(value); > + new_key = g_strdup_printf("%s.%s", prefix, entry->key); > + qdict_put_obj(target, new_key, value); > + delete = true; > + } > + > + if (qobject_type(value) == QTYPE_QDICT) { > + qdict_do_flatten(qobject_to_qdict(value), target, > + new_key ? new_key : entry->key); > + delete = true; > + } > + > + if (delete) { > + qdict_del(qdict, entry->key); > + > + /* Restart loop after modifying the iterated QDict */ > + entry = qdict_first(qdict); ...now entry points to the head of the modified qdict, since the modification may have re-arranged where we would iterate next... > + } > + > + entry = next; ...oops, we just undid that, and pointed it back to the old qdict iteration location. I think you're missing a continue statement inside 'if (delete)'. If you agree with my analysis and incorporate my suggested fix, then I'm okay if you add: Reviewed-by: Eric Blake <eblake@redhat.com>
Am 26.07.2013 um 18:40 hat Eric Blake geschrieben: > On 07/23/2013 07:03 AM, Kevin Wolf wrote: > > qdict_flatten(): For each nested QDict with key x, all fields with key y > > are moved to this QDict and their key is renamed to "x.y". This operation > > is applied recursively for nested QDicts. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > include/qapi/qmp/qdict.h | 1 + > > qobject/qdict.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 51 insertions(+) > > > > + while (entry != NULL) { > > + > > + next = qdict_next(qdict, entry); > > next points to a position in the unmodified qdict... > > > + value = qdict_entry_value(entry); > > + new_key = NULL; > > + delete = false; > > + > > + if (prefix) { > > + qobject_incref(value); > > + new_key = g_strdup_printf("%s.%s", prefix, entry->key); > > + qdict_put_obj(target, new_key, value); > > + delete = true; > > + } > > + > > + if (qobject_type(value) == QTYPE_QDICT) { > > + qdict_do_flatten(qobject_to_qdict(value), target, > > + new_key ? new_key : entry->key); > > + delete = true; > > + } > > + > > + if (delete) { > > + qdict_del(qdict, entry->key); > > + > > + /* Restart loop after modifying the iterated QDict */ > > + entry = qdict_first(qdict); > > ...now entry points to the head of the modified qdict, since the > modification may have re-arranged where we would iterate next... > > > + } > > + > > + entry = next; > > ...oops, we just undid that, and pointed it back to the old qdict > iteration location. I think you're missing a continue statement inside > 'if (delete)'. Gah, this is getting embarrassing. Who stole my continue? :-) You're right of course, thanks for catching that. I'll fix it. Kevin
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h index 685b2e3..d6855d1 100644 --- a/include/qapi/qmp/qdict.h +++ b/include/qapi/qmp/qdict.h @@ -65,5 +65,6 @@ int qdict_get_try_bool(const QDict *qdict, const char *key, int def_value); const char *qdict_get_try_str(const QDict *qdict, const char *key); QDict *qdict_clone_shallow(const QDict *src); +void qdict_flatten(QDict *qdict); #endif /* QDICT_H */ diff --git a/qobject/qdict.c b/qobject/qdict.c index ed381f9..108db47 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -476,3 +476,53 @@ static void qdict_destroy_obj(QObject *obj) g_free(qdict); } + +static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix) +{ + QObject *value; + const QDictEntry *entry, *next; + const char *new_key; + bool delete; + + entry = qdict_first(qdict); + + while (entry != NULL) { + + next = qdict_next(qdict, entry); + value = qdict_entry_value(entry); + new_key = NULL; + delete = false; + + if (prefix) { + qobject_incref(value); + new_key = g_strdup_printf("%s.%s", prefix, entry->key); + qdict_put_obj(target, new_key, value); + delete = true; + } + + if (qobject_type(value) == QTYPE_QDICT) { + qdict_do_flatten(qobject_to_qdict(value), target, + new_key ? new_key : entry->key); + delete = true; + } + + if (delete) { + qdict_del(qdict, entry->key); + + /* Restart loop after modifying the iterated QDict */ + entry = qdict_first(qdict); + } + + entry = next; + } +} + +/** + * qdict_flatten(): For each nested QDict with key x, all fields with key y + * are moved to this QDict and their key is renamed to "x.y". This operation + * is applied recursively for nested QDicts. + */ +void qdict_flatten(QDict *qdict) +{ + qdict_do_flatten(qdict, qdict, NULL); +}
qdict_flatten(): For each nested QDict with key x, all fields with key y are moved to this QDict and their key is renamed to "x.y". This operation is applied recursively for nested QDicts. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- include/qapi/qmp/qdict.h | 1 + qobject/qdict.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+)