[Qemu-devel] [PATCH 0/2] qdict_flatten() fixes

[Qemu-devel] [PATCH 0/2] qdict_flatten() fixes

[Kevin Wolf]

Kevin Wolf (2):
  qdict: Fix memory leak in qdict_do_flatten()
  qdict: Optimise qdict_do_flatten()

 qobject/qdict.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/2] qdict: Fix memory leak in qdict_do_flatten()

[Kevin Wolf]

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qobject/qdict.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/qobject/qdict.c b/qobject/qdict.c
index 0f3e0a6..60d6cd5 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -481,7 +481,7 @@ static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix)
 {
     QObject *value;
     const QDictEntry *entry, *next;
-    const char *new_key;
+    char *new_key;
     bool delete;
 
     entry = qdict_first(qdict);
@@ -506,6 +506,8 @@ static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix)
             delete = true;
         }
 
+        g_free(new_key);
+
         if (delete) {
             qdict_del(qdict, entry->key);
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/2] qdict: Optimise qdict_do_flatten()

[Kevin Wolf]

Nested QDicts used to be both entered recursively in order to move their
entries to the target QDict and also be moved themselves to the target
QDict like all other objects. This is harmless because for the top
level, qdict_do_flatten() will encounter the (now empty) QDict for a
second time and then delete it, but at the same time it's obviously
unnecessary overhead. Just delete nested QDicts directly after moving
all of their entries.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qobject/qdict.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/qobject/qdict.c b/qobject/qdict.c
index 60d6cd5..17e14f0 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -494,16 +494,20 @@ static void qdict_do_flatten(QDict *qdict, QDict *target, const char *prefix)
         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) {
+            /* 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);
             delete = true;
+        } else if (prefix) {
+            /* All other objects are moved to the target unchanged. */
+            qobject_incref(value);
+            qdict_put_obj(target, new_key, value);
+            delete = true;
         }
 
         g_free(new_key);
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 0/2] qdict_flatten() fixes

[Laszlo Ersek]

On 11/20/13 13:09, Kevin Wolf wrote:
> Kevin Wolf (2):
>   qdict: Fix memory leak in qdict_do_flatten()
>   qdict: Optimise qdict_do_flatten()
> 
>  qobject/qdict.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 

Had to look at my earlier review to page-in this stuff. The series looks
good to me.

series
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

Re: [Qemu-devel] [PATCH 0/2] qdict_flatten() fixes

[Stefan Hajnoczi]

On Wed, Nov 20, 2013 at 01:09:19PM +0100, Kevin Wolf wrote:
> Kevin Wolf (2):
>   qdict: Fix memory leak in qdict_do_flatten()
>   qdict: Optimise qdict_do_flatten()
> 
>  qobject/qdict.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)

Thanks, applied to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan