[PATCH 0/2] Remove memory leaks in XML device parsing

Discovered a few leaks in device_parsing.c running xml_parse_test under valgrind. The series applies on master as well as on top of the last series I sent out. Viktor Mihajlovski (2): libxkutil: Plug memory leaks in device parsing xml_parse_test: Call cleanup_dominfo before exiting libxkutil/device_parsing.c | 30 ++++++++++++++++++++---------- libxkutil/xml_parse_test.c | 2 ++ 2 files changed, 22 insertions(+), 10 deletions(-) -- 1.7.9.5

Fixed a number of memory leaks detected while running xml_parse_test under valgrind. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- libxkutil/device_parsing.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index 06acbac..59186a3 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -105,6 +105,8 @@ static void cleanup_net_device(struct net_device *dev) free(dev->device); free(dev->net_mode); free(dev->filter_ref); + free(dev->poolid); + cleanup_vsi_device(&dev->vsi); } static void cleanup_emu_device(struct emu_device *dev) @@ -712,6 +714,8 @@ static int parse_mem_device(xmlNode *node, struct virt_device **vdevs) struct virt_device *vdev = NULL; struct mem_device *mdev = NULL; char *content = NULL; + char *tmpval = NULL; + int ret = 0; vdev = calloc(1, sizeof(*vdev)); if (vdev == NULL) @@ -725,27 +729,25 @@ static int parse_mem_device(xmlNode *node, struct virt_device **vdevs) sscanf(content, "%" PRIu64, &mdev->size); else if (XSTREQ(node->name, "memory")) { sscanf(content, "%" PRIu64, &mdev->maxsize); - content = get_attr_value(node, "dumpCore"); - if (content && XSTREQ(content, "on")) { + tmpval = get_attr_value(node, "dumpCore"); + if (tmpval && XSTREQ(tmpval, "on")) { mdev->dumpCore = MEM_DUMP_CORE_ON; - } else if (content && XSTREQ(content, "off")) { + } else if (tmpval && XSTREQ(content, "off")) { mdev->dumpCore = MEM_DUMP_CORE_OFF; } else { mdev->dumpCore = MEM_DUMP_CORE_NOT_SET; } } - free(content); - *vdevs = vdev; - - return 1; + ret = 1; err: free(content); + free(tmpval); free(vdev); - return 0; + return ret; } static char *get_attr_value_default(xmlNode *node, char *attrname, @@ -1079,7 +1081,10 @@ static int do_parse(xmlNodeSet *nsv, dev_parse_func_t do_real_parse, } out: - *l = list; + if (list) { + free(*l); + *l = list; + } return lstidx; } @@ -1497,7 +1502,7 @@ static int parse_features(struct domain *dominfo, xmlNode *features) static void set_action(int *val, xmlNode *child) { - const char *action = (char *)xmlNodeGetContent(child); + char *action = (char *)xmlNodeGetContent(child); if (action == NULL) *val = CIM_VSSD_RECOVERY_NONE; @@ -1509,6 +1514,8 @@ static void set_action(int *val, xmlNode *child) *val = CIM_VSSD_RECOVERY_RESTART; else *val = CIM_VSSD_RECOVERY_NONE; + + xmlFree(action); } static int parse_domain(xmlNodeSet *nsv, struct domain *dominfo) @@ -1663,9 +1670,11 @@ void cleanup_dominfo(struct domain **dominfo) dom = *dominfo; free(dom->name); + free(dom->typestr); free(dom->uuid); free(dom->bootloader); free(dom->bootloader_args); + free(dom->clock); if (dom->type == DOMAIN_XENPV) { free(dom->os_info.pv.type); @@ -1687,6 +1696,7 @@ void cleanup_dominfo(struct domain **dominfo) CU_DEBUG("Unknown domain type %i", dom->type); } + cleanup_virt_devices(&dom->dev_emu, 1); cleanup_virt_devices(&dom->dev_mem, dom->dev_mem_ct); cleanup_virt_devices(&dom->dev_net, dom->dev_net_ct); cleanup_virt_devices(&dom->dev_disk, dom->dev_disk_ct); -- 1.7.9.5

On 09/06/2013 08:09 AM, Viktor Mihajlovski wrote:
Fixed a number of memory leaks detected while running xml_parse_test under valgrind.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- libxkutil/device_parsing.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/libxkutil/device_parsing.c b/libxkutil/device_parsing.c index 06acbac..59186a3 100644 --- a/libxkutil/device_parsing.c +++ b/libxkutil/device_parsing.c @@ -105,6 +105,8 @@ static void cleanup_net_device(struct net_device *dev) free(dev->device); free(dev->net_mode); free(dev->filter_ref); + free(dev->poolid); + cleanup_vsi_device(&dev->vsi); }
static void cleanup_emu_device(struct emu_device *dev) @@ -712,6 +714,8 @@ static int parse_mem_device(xmlNode *node, struct virt_device **vdevs) struct virt_device *vdev = NULL; struct mem_device *mdev = NULL; char *content = NULL; + char *tmpval = NULL; + int ret = 0;
vdev = calloc(1, sizeof(*vdev)); if (vdev == NULL) @@ -725,27 +729,25 @@ static int parse_mem_device(xmlNode *node, struct virt_device **vdevs) sscanf(content, "%" PRIu64, &mdev->size); else if (XSTREQ(node->name, "memory")) { sscanf(content, "%" PRIu64, &mdev->maxsize); - content = get_attr_value(node, "dumpCore"); - if (content && XSTREQ(content, "on")) { + tmpval = get_attr_value(node, "dumpCore"); + if (tmpval && XSTREQ(tmpval, "on")) { mdev->dumpCore = MEM_DUMP_CORE_ON; - } else if (content && XSTREQ(content, "off")) { + } else if (tmpval && XSTREQ(content, "off")) { mdev->dumpCore = MEM_DUMP_CORE_OFF; } else { mdev->dumpCore = MEM_DUMP_CORE_NOT_SET; } }
- free(content); - *vdevs = vdev;
Since vdev is going to be returned - we'll need to : vdev = NULL; here, which I've done and tested successfully. I'll squash before push. John
- - return 1; + ret = 1;
err: free(content); + free(tmpval); free(vdev);
- return 0; + return ret; }
static char *get_attr_value_default(xmlNode *node, char *attrname, @@ -1079,7 +1081,10 @@ static int do_parse(xmlNodeSet *nsv, dev_parse_func_t do_real_parse, }
out: - *l = list; + if (list) { + free(*l); + *l = list; + } return lstidx; }
@@ -1497,7 +1502,7 @@ static int parse_features(struct domain *dominfo, xmlNode *features)
static void set_action(int *val, xmlNode *child) { - const char *action = (char *)xmlNodeGetContent(child); + char *action = (char *)xmlNodeGetContent(child);
if (action == NULL) *val = CIM_VSSD_RECOVERY_NONE; @@ -1509,6 +1514,8 @@ static void set_action(int *val, xmlNode *child) *val = CIM_VSSD_RECOVERY_RESTART; else *val = CIM_VSSD_RECOVERY_NONE; + + xmlFree(action); }
static int parse_domain(xmlNodeSet *nsv, struct domain *dominfo) @@ -1663,9 +1670,11 @@ void cleanup_dominfo(struct domain **dominfo)
dom = *dominfo; free(dom->name); + free(dom->typestr); free(dom->uuid); free(dom->bootloader); free(dom->bootloader_args); + free(dom->clock);
if (dom->type == DOMAIN_XENPV) { free(dom->os_info.pv.type); @@ -1687,6 +1696,7 @@ void cleanup_dominfo(struct domain **dominfo) CU_DEBUG("Unknown domain type %i", dom->type); }
+ cleanup_virt_devices(&dom->dev_emu, 1); cleanup_virt_devices(&dom->dev_mem, dom->dev_mem_ct); cleanup_virt_devices(&dom->dev_net, dom->dev_net_ct); cleanup_virt_devices(&dom->dev_disk, dom->dev_disk_ct);

On 09/11/2013 11:14 PM, John Ferlan wrote: [...]
*vdevs = vdev;
Since vdev is going to be returned - we'll need to :
vdev = NULL;
here, which I've done and tested successfully. I'll squash before push. embarrassing ... I'm really glad you caught this (no wonder the leaks were gone :-)
-- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

This avoids misleading valgrind output running xml_parse_test. Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- libxkutil/xml_parse_test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libxkutil/xml_parse_test.c b/libxkutil/xml_parse_test.c index af5e508..374bcf6 100644 --- a/libxkutil/xml_parse_test.c +++ b/libxkutil/xml_parse_test.c @@ -521,6 +521,8 @@ int main(int argc, char **argv) return 4; } + cleanup_dominfo(&dominfo); + return 0; } -- 1.7.9.5

On 09/06/2013 08:09 AM, Viktor Mihajlovski wrote:
This avoids misleading valgrind output running xml_parse_test.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- libxkutil/xml_parse_test.c | 2 ++ 1 file changed, 2 insertions(+)
ACK (and pushed) John

On 09/11/2013 11:15 PM, John Ferlan wrote:
On 09/06/2013 08:09 AM, Viktor Mihajlovski wrote:
This avoids misleading valgrind output running xml_parse_test.
Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- libxkutil/xml_parse_test.c | 2 ++ 1 file changed, 2 insertions(+)
ACK (and pushed)
John
Thanks, appreciate your support very much. -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (2)
-
John Ferlan
-
Viktor Mihajlovski