Re: [PATCH] Fix virQEMUCapsLoadCache leaks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]