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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list