On Fri, Dec 20, 2013 at 06:23:26PM +0100, Max Reitz wrote: > On 20.12.2013 18:19, Max Reitz wrote: > >On 20.12.2013 10:46, Stefan Hajnoczi wrote: > >>>diff --git a/qobject/qdict.c b/qobject/qdict.c > >>>index fca1902..7b6b08a 100644 > >>>--- a/qobject/qdict.c > >>>+++ b/qobject/qdict.c > >>>@@ -477,7 +477,46 @@ 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()). */ > >>>+ assert(prefix); > >>>+ > >>>+ 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); > >>It seems this operation is clobbered by what follows and should be > >>deleted. Is the incref also superfluous or... > >> > >>>+ > >>>+ if (qobject_type(value) == QTYPE_QDICT) { > >>>+ qdict_flatten_qdict(qobject_to_qdict(value), > >>>target, new_key); > >>>+ } else if (qobject_type(value) == QTYPE_QLIST) { > >>>+ qdict_flatten_qlist(qobject_to_qlist(value), > >>>target, new_key); > >>>+ } else { > >>>+ /* All other types are moved to the target unchanged. */ > >>>+ qobject_incref(value); > >>...should this one be deleted? > > > >Oh great, I've been fixing working code, then. *g* > > > >I added the else branch in v5, I guess, it was correct not to have > >it (however, the condition in the else if was missing in v4). I'll > >drop it again, thanks. > > On second thought, that would be even more wrong. I'll leave the > else branch and drop the qobject_incref() and qdict_put_obj() above > the condition.
I agree, that sounds like the right solution.