[Qemu-devel] [RESEND v2 0/3] Fix some memory leaks about query memdev

[Qemu-devel] [RESEND v2 0/3] Fix some memory leaks about query memdev

[Chen Fan]

when using valgrind to test the command "query memdev", I had
found some memory leaks. the test result:

==13802== 4 bytes in 1 blocks are definitely lost in loss record 125 of 8,508
==13802==    at 0x4A08934: malloc (vg_replace_malloc.c:291)
==13802==    by 0x4A08AA8: realloc (vg_replace_malloc.c:687)
==13802==    by 0x64C5736: g_realloc (in /usr/lib64/libglib-2.0.so.0.3400.2)
==13802==    by 0x64DE226: ??? (in /usr/lib64/libglib-2.0.so.0.3400.2)
==13802==    by 0x64DE279: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.3400.2)
==13802==    by 0x47CFBB: string_output_visitor_new (string-output-visitor.c:341)
==13802==    by 0x3F456F: object_property_get_uint16List (object.c:970)
==13802==    by 0x1E8764: query_memdev (numa.c:361)
==13802==    by 0x3F3248: object_child_foreach (object.c:686)
==13802==    by 0x1E9141: qmp_query_memdev (numa.c:389)
==13802==    by 0x2D79A0: qmp_marshal_input_query_memdev (qmp-marshal.c:5057)
==13802==    by 0x1DD7D7: handle_qmp_command (monitor.c:5038)

==15046== 48 (16 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 4,722 of 8,549
==15046==    at 0x4A08934: malloc (vg_replace_malloc.c:291)
==15046==    by 0x64C541D: ??? (in /usr/lib64/libglib-2.0.so.0.3400.2)
==15046==    by 0x64C56E6: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.3400.2)
==15046==    by 0x1E868C: query_memdev (numa.c:325)
==15046==    by 0x3F3258: object_child_foreach (object.c:686)
==15046==    by 0x1E9141: qmp_query_memdev (numa.c:389)
==15046==    by 0x2DDFF3: hmp_info_memdev (hmp.c:1687)
==15046==    by 0x1E4B08: handle_user_command (monitor.c:4119)
==15046==    by 0x1E4E7A: monitor_command_cb (monitor.c:5156)
==15046==    by 0x496056: readline_handle_byte (readline.c:391)
==15046==    by 0x1E4BCE: monitor_read (monitor.c:5139)
==15046==    by 0x2BCDEF: fd_chr_read (qemu-char.c:213)


Chen Fan (3):
  query-memdev: fix potential memory leaks
  qom/object.c: fix string_output_get_string() memory leak
  hmp: fix MemdevList memory leak

 hmp.c        |  8 ++++++--
 numa.c       | 15 +++++++--------
 qom/object.c | 12 ++++++++++--
 3 files changed, 23 insertions(+), 12 deletions(-)

-- 
1.9.3

[Qemu-devel] [RESEND v2 1/3] query-memdev: fix potential memory leaks

[Chen Fan]

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Hu Tao <hutao@cn.fujitsu.com>

---
 numa.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/numa.c b/numa.c
index c78cec9..aa772aa 100644
--- a/numa.c
+++ b/numa.c
@@ -318,10 +318,11 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
 static int query_memdev(Object *obj, void *opaque)
 {
     MemdevList **list = opaque;
+    MemdevList *m = NULL;
     Error *err = NULL;
 
     if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
-        MemdevList *m = g_malloc0(sizeof(*m));
+        m = g_malloc0(sizeof(*m));
 
         m->value = g_malloc0(sizeof(*m->value));
 
@@ -369,6 +370,9 @@ static int query_memdev(Object *obj, void *opaque)
 
     return 0;
 error:
+    g_free(m->value);
+    g_free(m);
+
     return -1;
 }
 
-- 
1.9.3

[Qemu-devel] [RESEND v2 2/3] qom/object.c: fix string_output_get_string() memory leak

[Chen Fan]

string_output_get_string() uses g_string_free(str, false) to
transfer the 'str' pointer to callers and never free it.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Hu Tao <hutao@cn.fujitsu.com>

---
 hmp.c        |  6 ++++--
 qom/object.c | 12 ++++++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/hmp.c b/hmp.c
index 4d1838e..ba40c75 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1687,6 +1687,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
     MemdevList *memdev_list = qmp_query_memdev(&err);
     MemdevList *m = memdev_list;
     StringOutputVisitor *ov;
+    char *str;
     int i = 0;
 
 
@@ -1704,9 +1705,10 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
                        m->value->prealloc ? "true" : "false");
         monitor_printf(mon, "  policy: %s\n",
                        HostMemPolicy_lookup[m->value->policy]);
-        monitor_printf(mon, "  host nodes: %s\n",
-                       string_output_get_string(ov));
+        str = string_output_get_string(ov);
+        monitor_printf(mon, "  host nodes: %s\n", str);
 
+        g_free(str);
         string_output_visitor_cleanup(ov);
         m = m->next;
         i++;
diff --git a/qom/object.c b/qom/object.c
index 0e8267b..e5aed60 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -948,14 +948,18 @@ int object_property_get_enum(Object *obj, const char *name,
 {
     StringOutputVisitor *sov;
     StringInputVisitor *siv;
+    char *str;
     int ret;
 
     sov = string_output_visitor_new(false);
     object_property_get(obj, string_output_get_visitor(sov), name, errp);
-    siv = string_input_visitor_new(string_output_get_string(sov));
+    str = string_output_get_string(sov);
+    siv = string_input_visitor_new(str);
     string_output_visitor_cleanup(sov);
     visit_type_enum(string_input_get_visitor(siv),
                     &ret, strings, NULL, name, errp);
+
+    g_free(str);
     string_input_visitor_cleanup(siv);
 
     return ret;
@@ -966,13 +970,17 @@ void object_property_get_uint16List(Object *obj, const char *name,
 {
     StringOutputVisitor *ov;
     StringInputVisitor *iv;
+    char *str;
 
     ov = string_output_visitor_new(false);
     object_property_get(obj, string_output_get_visitor(ov),
                         name, errp);
-    iv = string_input_visitor_new(string_output_get_string(ov));
+    str = string_output_get_string(ov);
+    iv = string_input_visitor_new(str);
     visit_type_uint16List(string_input_get_visitor(iv),
                           list, NULL, errp);
+
+    g_free(str);
     string_output_visitor_cleanup(ov);
     string_input_visitor_cleanup(iv);
 }
-- 
1.9.3

[Qemu-devel] [RESEND v2 3/3] hmp: fix MemdevList memory leak

[Chen Fan]

the memdev_list in hmp_info_memdev() is never freed.
so we use existent method qapi_free_MemdevList() to free it.
and also we can use qapi_free_MemdevList() to replace list loops
to clean up the memdev list in error path.

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Hu Tao <hutao@cn.fujitsu.com>

---
 hmp.c  | 2 ++
 numa.c | 9 ++-------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/hmp.c b/hmp.c
index ba40c75..40a90da 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1715,4 +1715,6 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
     }
 
     monitor_printf(mon, "\n");
+
+    qapi_free_MemdevList(memdev_list);
 }
diff --git a/numa.c b/numa.c
index aa772aa..f07149b 100644
--- a/numa.c
+++ b/numa.c
@@ -379,7 +379,7 @@ error:
 MemdevList *qmp_query_memdev(Error **errp)
 {
     Object *obj;
-    MemdevList *list = NULL, *m;
+    MemdevList *list = NULL;
 
     obj = object_resolve_path("/objects", NULL);
     if (obj == NULL) {
@@ -393,11 +393,6 @@ MemdevList *qmp_query_memdev(Error **errp)
     return list;
 
 error:
-    while (list) {
-        m = list;
-        list = list->next;
-        g_free(m->value);
-        g_free(m);
-    }
+    qapi_free_MemdevList(list);
     return NULL;
 }
-- 
1.9.3

Re: [Qemu-devel] [RESEND v2 0/3] Fix some memory leaks about query memdev

[chen.fan.fnst]

ping...

This patches have been reviewed-by, only need someone ack it.

Thanks,
Chen

On Mon, 2014-08-18 at 14:46 +0800, Chen Fan wrote: 
> when using valgrind to test the command "query memdev", I had
> found some memory leaks. the test result:
> 
> ==13802== 4 bytes in 1 blocks are definitely lost in loss record 125 of 8,508
> ==13802==    at 0x4A08934: malloc (vg_replace_malloc.c:291)
> ==13802==    by 0x4A08AA8: realloc (vg_replace_malloc.c:687)
> ==13802==    by 0x64C5736: g_realloc (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==13802==    by 0x64DE226: ??? (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==13802==    by 0x64DE279: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==13802==    by 0x47CFBB: string_output_visitor_new (string-output-visitor.c:341)
> ==13802==    by 0x3F456F: object_property_get_uint16List (object.c:970)
> ==13802==    by 0x1E8764: query_memdev (numa.c:361)
> ==13802==    by 0x3F3248: object_child_foreach (object.c:686)
> ==13802==    by 0x1E9141: qmp_query_memdev (numa.c:389)
> ==13802==    by 0x2D79A0: qmp_marshal_input_query_memdev (qmp-marshal.c:5057)
> ==13802==    by 0x1DD7D7: handle_qmp_command (monitor.c:5038)
> 
> ==15046== 48 (16 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 4,722 of 8,549
> ==15046==    at 0x4A08934: malloc (vg_replace_malloc.c:291)
> ==15046==    by 0x64C541D: ??? (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==15046==    by 0x64C56E6: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==15046==    by 0x1E868C: query_memdev (numa.c:325)
> ==15046==    by 0x3F3258: object_child_foreach (object.c:686)
> ==15046==    by 0x1E9141: qmp_query_memdev (numa.c:389)
> ==15046==    by 0x2DDFF3: hmp_info_memdev (hmp.c:1687)
> ==15046==    by 0x1E4B08: handle_user_command (monitor.c:4119)
> ==15046==    by 0x1E4E7A: monitor_command_cb (monitor.c:5156)
> ==15046==    by 0x496056: readline_handle_byte (readline.c:391)
> ==15046==    by 0x1E4BCE: monitor_read (monitor.c:5139)
> ==15046==    by 0x2BCDEF: fd_chr_read (qemu-char.c:213)
> 
> 
> Chen Fan (3):
>   query-memdev: fix potential memory leaks
>   qom/object.c: fix string_output_get_string() memory leak
>   hmp: fix MemdevList memory leak
> 
>  hmp.c        |  8 ++++++--
>  numa.c       | 15 +++++++--------
>  qom/object.c | 12 ++++++++++--
>  3 files changed, 23 insertions(+), 12 deletions(-)
> 

Re: [Qemu-devel] [RESEND v2 0/3] Fix some memory leaks about query memdev

[chen.fan.fnst]

ping ?

On Mon, 2014-08-18 at 14:46 +0800, Chen Fan wrote: 
> when using valgrind to test the command "query memdev", I had
> found some memory leaks. the test result:
> 
> ==13802== 4 bytes in 1 blocks are definitely lost in loss record 125 of 8,508
> ==13802==    at 0x4A08934: malloc (vg_replace_malloc.c:291)
> ==13802==    by 0x4A08AA8: realloc (vg_replace_malloc.c:687)
> ==13802==    by 0x64C5736: g_realloc (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==13802==    by 0x64DE226: ??? (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==13802==    by 0x64DE279: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==13802==    by 0x47CFBB: string_output_visitor_new (string-output-visitor.c:341)
> ==13802==    by 0x3F456F: object_property_get_uint16List (object.c:970)
> ==13802==    by 0x1E8764: query_memdev (numa.c:361)
> ==13802==    by 0x3F3248: object_child_foreach (object.c:686)
> ==13802==    by 0x1E9141: qmp_query_memdev (numa.c:389)
> ==13802==    by 0x2D79A0: qmp_marshal_input_query_memdev (qmp-marshal.c:5057)
> ==13802==    by 0x1DD7D7: handle_qmp_command (monitor.c:5038)
> 
> ==15046== 48 (16 direct, 32 indirect) bytes in 1 blocks are definitely lost in loss record 4,722 of 8,549
> ==15046==    at 0x4A08934: malloc (vg_replace_malloc.c:291)
> ==15046==    by 0x64C541D: ??? (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==15046==    by 0x64C56E6: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.3400.2)
> ==15046==    by 0x1E868C: query_memdev (numa.c:325)
> ==15046==    by 0x3F3258: object_child_foreach (object.c:686)
> ==15046==    by 0x1E9141: qmp_query_memdev (numa.c:389)
> ==15046==    by 0x2DDFF3: hmp_info_memdev (hmp.c:1687)
> ==15046==    by 0x1E4B08: handle_user_command (monitor.c:4119)
> ==15046==    by 0x1E4E7A: monitor_command_cb (monitor.c:5156)
> ==15046==    by 0x496056: readline_handle_byte (readline.c:391)
> ==15046==    by 0x1E4BCE: monitor_read (monitor.c:5139)
> ==15046==    by 0x2BCDEF: fd_chr_read (qemu-char.c:213)
> 
> 
> Chen Fan (3):
>   query-memdev: fix potential memory leaks
>   qom/object.c: fix string_output_get_string() memory leak
>   hmp: fix MemdevList memory leak
> 
>  hmp.c        |  8 ++++++--
>  numa.c       | 15 +++++++--------
>  qom/object.c | 12 ++++++++++--
>  3 files changed, 23 insertions(+), 12 deletions(-)
> 

Re: [Qemu-devel] [Qemu-trivial] [RESEND v2 0/3] Fix some memory leaks about query memdev

[Michael Tokarev]

18.08.2014 10:46, Chen Fan wrote:
> when using valgrind to test the command "query memdev", I had
> found some memory leaks. the test result:
...

Applied all 3 to -trivial.  Usually these are not really trivial.
Please note that your 2nd patch - "qom/object.c: fix string_output_get_string()
memory leak" - also touches hmp.c (which has a maintainer), but you
didn't mention this in patch description.  I've added it.

Thanks,

/mjt

Re: [Qemu-devel] [Qemu-trivial] [RESEND v2 0/3] Fix some memory leaks about query memdev

[chen.fan.fnst]

On Fri, 2014-08-29 at 20:29 +0400, Michael Tokarev wrote: 
> 18.08.2014 10:46, Chen Fan wrote:
> > when using valgrind to test the command "query memdev", I had
> > found some memory leaks. the test result:
> ...
> 
> Applied all 3 to -trivial.  Usually these are not really trivial.
> Please note that your 2nd patch - "qom/object.c: fix string_output_get_string()
> memory leak" - also touches hmp.c (which has a maintainer), but you
> didn't mention this in patch description.  I've added it.
Thanks,

Chen


> 
> Thanks,
> 
> /mjt