[libvirt] [PATCH] virsh: Plug memory leak on cmdUndefine

[Peter Krempa]

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