[libvirt] [PATCH] virsh: Plug memory leak on cmdUndefine
Peter Krempa
pkrempa at redhat.com
Thu Feb 2 10:48:36 UTC 2012
On 02/02/2012 07:25 AM, ajia at redhat.com wrote:
> From: Alex Jia<ajia at redhat.com>
>
> ---
> tools/virsh.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/virsh.c b/tools/virsh.c
> index c8fd448..73c2192 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -2787,10 +2787,6 @@ out:
> ctxt->node = vol_nodes[vol_i];
> VIR_FREE(target);
> VIR_FREE(source);
> - if (vol) {
> - virStorageVolFree(vol);
> - vol = NULL;
> - }
Actually, this code is needed, as error paths in the loop are handled
gracefuly with a 'continue;' so we need to free the volume on such path;
>
> /* get volume source and target paths */
> if (!(target = virXPathString("string(./target/@dev)", ctxt))) {
> @@ -2852,6 +2848,10 @@ out:
> vol_del_failed = true;
> }
> vshPrint(ctl, _("Volume '%s' removed.\n"), volume_tok?volume_tok:source);
> +
> + /* cleanup */
> + virStorageVolFree(vol);
> + vol = NULL;
> }
Yeah, I actualy forgot to clean up the volume after the loop. I modified
your patch to correct this in the final cleanup:
diff --git a/tools/virsh.c b/tools/virsh.c
index c8fd448..af78102 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2875,6 +2875,8 @@ cleanup:
VIR_FREE(volume_tokens);
VIR_FREE(def);
VIR_FREE(vol_nodes);
+ if (vol)
+ virStorageVolFree(vol);
xmlFreeDoc(doc);
xmlXPathFreeContext(ctxt);
virDomainFree(dom);
and pushed. Thanks for catching this bug.
Peter
More information about the libvir-list
mailing list