Re: [libvirt] [PATCH] conf: Fix memory leaks in virStoragePoolDefParseSource
On 05/09/2012 08:15 AM, Alex Jia wrote:
Detected by valgrind. Leaks are introduced in commit 122fa379.
src/conf/storage_conf.c: fix memory leaks.
How to reproduce?
$ make&& make -C tests check TESTS=storagepoolxml2xmltest
$ cd tests&& valgrind -v --leak-check=full ./storagepoolxml2xmltest
actual result:
==28571== LEAK SUMMARY:
==28571== definitely lost: 40 bytes in 5 blocks
==28571== indirectly lost: 0 bytes in 0 blocks
==28571== possibly lost: 0 bytes in 0 blocks
==28571== still reachable: 1,054 bytes in 21 blocks
==28571== suppressed: 0 bytes in 0 blocks
Signed-off-by: Alex Jia<ajia(a)redhat.com>
---
src/conf/storage_conf.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 0b34f28..668e679 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -462,6 +462,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
}
}
+ VIR_FREE(nodeset);
}
}
The problem with the added line is not visible with this context, so I'm adding
some more:
source->nhost = virXPathNodeSet("./host", ctxt, &nodeset);
if (source->nhost) {
if (VIR_ALLOC_N(source->hosts, source->nhost) < 0) {
virReportOOMError();
goto cleanup;
}
for (i = 0 ; i < source->nhost ; i++) {
name = virXMLPropString(nodeset[i], "name");
if (name == NULL) {
virStorageReportError(VIR_ERR_XML_ERROR,
"%s", _("missing storage pool host name"));
goto cleanup;
}
source->hosts[i].name = name;
port = virXMLPropString(nodeset[i], "port");
if (port) {
if (virStrToLong_i(port, NULL, 10, &source->hosts[i].port) < 0)
{
virStorageReportError(VIR_ERR_XML_ERROR,
_("Invalid port number: %s"),
port);
goto cleanup;
}
}
+ VIR_FREE(nodeset);
}
}
You added the VIR_FREE inside the for loop, so it gets freed before the next iteration
and might cause a segfault in the second iteration.
NACK
Peter