Message ID | 20180509165530.29561-10-mreitz@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 05/09/2018 11:55 AM, Max Reitz wrote: > In its current form, qdict_flatten() removes all entries from nested > QDicts that are moved to the root QDict. It is completely sufficient to > remove all old entries from the root QDict, however. If the nested > dicts have a refcount of 1, this will automatically delete them, too. > And if they have a greater refcount, we probably do not want to modify > them in the first place. > > The latter observation means that it was currently (in general) > impossible to qdict_flatten() a shallowly cloned dict because that would > empty nested QDicts in the original dict as well. This patch changes > this, so you can now use qdict_flatten(qdict_shallow_clone(dict)) to get > a flattened copy without disturbing the original. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > qobject/qdict.c | 23 ++++++++++++++++------- > 1 file changed, 16 insertions(+), 7 deletions(-) > > > g_free(new_key); > > - if (delete) { > + if (copied_value && qdict == target) { > + /* If we have copied a value, and if we are on the root > + * level, we need to remove the old entry. Otherwise, we > + * do not, because by removing these entries on the root > + * level, the reference counts of nested dicts and listed s/listed/lists/ > + * will be reduced automatically. In fact, we probably do > + * not want to modify nested dicts and lists with > + * refcounts greater than 1 anyway. */ > qdict_del(qdict, entry->key); > > - /* Restart loop after modifying the iterated QDict */ > + /* Restart loop after modifying the iterated QDict. We > + * only need to do this if qdict == target, because > + * otherwise copying the value did not affect qdict. */ > entry = qdict_first(qdict); > continue; > } > With the typo fix, Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/qobject/qdict.c b/qobject/qdict.c index 83678c4951..63dcb43e73 100644 --- a/qobject/qdict.c +++ b/qobject/qdict.c @@ -537,7 +537,7 @@ static void qdict_flatten_qdict(QDict *qdict, QDict *target, const char *prefix) QObject *value; const QDictEntry *entry, *next; char *new_key; - bool delete; + bool copied_value; entry = qdict_first(qdict); @@ -546,7 +546,7 @@ static void qdict_flatten_qdict(QDict *qdict, QDict *target, const char *prefix) next = qdict_next(qdict, entry); value = qdict_entry_value(entry); new_key = NULL; - delete = false; + copied_value = false; if (prefix) { new_key = g_strdup_printf("%s.%s", prefix, entry->key); @@ -557,23 +557,32 @@ static void qdict_flatten_qdict(QDict *qdict, QDict *target, const char *prefix) * itself disappears. */ qdict_flatten_qdict(qobject_to(QDict, value), target, new_key ? new_key : entry->key); - delete = true; + copied_value = true; } else if (qobject_type(value) == QTYPE_QLIST) { qdict_flatten_qlist(qobject_to(QList, value), target, new_key ? new_key : entry->key); - delete = true; + copied_value = true; } else if (prefix) { /* All other objects are moved to the target unchanged. */ qdict_put_obj(target, new_key, qobject_ref(value)); - delete = true; + copied_value = true; } g_free(new_key); - if (delete) { + if (copied_value && qdict == target) { + /* If we have copied a value, and if we are on the root + * level, we need to remove the old entry. Otherwise, we + * do not, because by removing these entries on the root + * level, the reference counts of nested dicts and listed + * will be reduced automatically. In fact, we probably do + * not want to modify nested dicts and lists with + * refcounts greater than 1 anyway. */ qdict_del(qdict, entry->key); - /* Restart loop after modifying the iterated QDict */ + /* Restart loop after modifying the iterated QDict. We + * only need to do this if qdict == target, because + * otherwise copying the value did not affect qdict. */ entry = qdict_first(qdict); continue; }
In its current form, qdict_flatten() removes all entries from nested QDicts that are moved to the root QDict. It is completely sufficient to remove all old entries from the root QDict, however. If the nested dicts have a refcount of 1, this will automatically delete them, too. And if they have a greater refcount, we probably do not want to modify them in the first place. The latter observation means that it was currently (in general) impossible to qdict_flatten() a shallowly cloned dict because that would empty nested QDicts in the original dict as well. This patch changes this, so you can now use qdict_flatten(qdict_shallow_clone(dict)) to get a flattened copy without disturbing the original. Signed-off-by: Max Reitz <mreitz@redhat.com> --- qobject/qdict.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-)