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

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



On 02/02/2012 07:25 AM, ajia redhat com wrote:
From: Alex Jia<ajia 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



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