8:09 p.m.
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
8:09 p.m.
New subject: [PATCH 1/2] libxkutil: Plug memory leaks in device parsing
Fixed a number of memory leaks detected while running xml_parse_test
under valgrind.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)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
5:14 a.m.
New subject: [PATCH 1/2] libxkutil: Plug memory leaks in device parsing
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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)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);
4:27 p.m.
New subject: [PATCH 1/2] libxkutil: Plug memory leaks in device parsing
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
8:09 p.m.
New subject: [PATCH 2/2] xml_parse_test: Call cleanup_dominfo before exiting
This avoids misleading valgrind output running xml_parse_test.
Signed-off-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)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
5:15 a.m.
New subject: [PATCH 2/2] xml_parse_test: Call cleanup_dominfo before exiting
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(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
---
libxkutil/xml_parse_test.c | 2 ++
1 file changed, 2 insertions(+)
ACK (and pushed)
John
4:28 p.m.
New subject: [PATCH 2/2] xml_parse_test: Call cleanup_dominfo before exiting
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(a)linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy(a)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
4013
days inactive
4019
days old
6 comments
2 participants
participants (2)
-
John Ferlan -
Viktor Mihajlovski