On Wed, Mar 19, 2014 at 05:17:40PM +0100, Ján Tomko wrote:
Valgrind reported leaking of maxCpus and arch strings from
virXPathString, as well as the leak of the machineMaxCpus array.
Use 'tmp' for the strings we don't want to free, to allow freeing
of 'str' in the cleanup label and free machineMaxCpus
in virCapsReset too.
---
src/qemu/qemu_capabilities.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2914200..e742c03 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2376,7 +2376,8 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char
*filename,
int n;
xmlNodePtr *nodes = NULL;
xmlXPathContextPtr ctxt = NULL;
- char *str;
+ char *str = NULL;
+ char *tmp;
long long int l;
if (!(doc = virXMLParseFile(filename)))
@@ -2432,7 +2433,6 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char
*filename,
if (flag < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unknown qemu capabilities flag %s"), str);
- VIR_FREE(str);
goto cleanup;
}
VIR_FREE(str);
@@ -2463,6 +2463,7 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char
*filename,
_("unknown arch %s in QEMU capabilities cache"), str);
goto cleanup;
}
+ VIR_FREE(str);
if ((n = virXPathNodeSet("./cpu", ctxt, &nodes)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -2476,12 +2477,12 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char
*filename,
goto cleanup;
for (i = 0; i < n; i++) {
- if (!(str = virXMLPropString(nodes[i], "name"))) {
+ if (!(tmp = virXMLPropString(nodes[i], "name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing cpu name in QEMU capabilities
cache"));
goto cleanup;
}
- qemuCaps->cpuDefinitions[i] = str;
+ qemuCaps->cpuDefinitions[i] = tmp;
Wouldn't it be easier to throw away tmp and do:
qemuCaps->cpuDefinitions[i] = virXMLPropString(nodes[i], "name");
if (!qemuCaps->cpuDefinitions[i])
as with aliases etc.?
}
}
VIR_FREE(nodes);
@@ -2503,12 +2504,12 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char
*filename,
goto cleanup;
for (i = 0; i < n; i++) {
- if (!(str = virXMLPropString(nodes[i], "name"))) {
+ if (!(tmp = virXMLPropString(nodes[i], "name"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("missing machine name in QEMU capabilities
cache"));
goto cleanup;
}
- qemuCaps->machineTypes[i] = str;
+ qemuCaps->machineTypes[i] = tmp;
Same here?
qemuCaps->machineAliases[i] =
virXMLPropString(nodes[i], "alias");
@@ -2519,12 +2520,14 @@ virQEMUCapsLoadCache(virQEMUCapsPtr qemuCaps, const char
*filename,
_("malformed machine cpu count in QEMU capabilities
cache"));
goto cleanup;
}
+ VIR_FREE(str);
}
}
VIR_FREE(nodes);
ret = 0;
- cleanup:
+cleanup:
I wish someone would add a HACKING rule about required spaces before
labels and enforce them in syntax-check. This makes git diffs a bit
less usable :(
ACK with the suggestions being optional to squash in.
Martin
+ VIR_FREE(str);
VIR_FREE(nodes);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(doc);
@@ -2668,6 +2671,7 @@ virQEMUCapsReset(virQEMUCapsPtr qemuCaps)
}
VIR_FREE(qemuCaps->machineTypes);
VIR_FREE(qemuCaps->machineAliases);
+ VIR_FREE(qemuCaps->machineMaxCpus);
qemuCaps->nmachineTypes = 0;
}
--
1.8.3.2
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list