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

Re: [libvirt] [PATCH] virsh: plug memory leaks on failure path



On 03/28/2012 02:58 PM, Alex Jia wrote:
Leaks are introduced in commit 1cf0e3d and fe383bb.

* tools/virsh.c (cmdSnapshotList): fix memory leaks.

* How to reproduce?

% virsh snapshot-list foo --parent --roots
error: --parent and --roots are mutually exclusive

error: Failed to disconnect from the hypervisor, 1 leaked reference(s)

% virsh snapshot-list foo --parent --tree
error: --parent and --tree are mutually exclusive

error: Failed to disconnect from the hypervisor, 1 leaked reference(s)

% virsh snapshot-list foo --roots --tree
error: --roots and --tree are mutually exclusive

error: Failed to disconnect from the hypervisor, 1 leaked reference(s)

......

Signed-off-by: Alex Jia<ajia redhat com>
---
  tools/virsh.c |    5 +++++
  1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 5009b6b..f5c3b60 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -16357,11 +16357,13 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
          if (vshCommandOptBool(cmd, "roots")) {
              vshError(ctl, "%s",
                       _("--parent and --roots are mutually exclusive"));
+            virDomainFree(dom);
              return false;

Why not "goto cleanup;" There is a "goto cleanup" right before,
so either change the previous "goto cleanup" into
"virDomainFree(dom); return false;" too, or uses the "goto cleanup;"
following.


          }
          if (tree) {
              vshError(ctl, "%s",
                       _("--parent and --tree are mutually exclusive"));
+            virDomainFree(dom);
              return false;
          }
          parent_filter = 1;
@@ -16369,11 +16371,13 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
          if (tree) {
              vshError(ctl, "%s",
                       _("--roots and --tree are mutually exclusive"));
+            virDomainFree(dom);
              return false;
          }
          if (from) {
              vshError(ctl, "%s",
                       _("--roots and --from are mutually exclusive"));
+            virDomainFree(dom);

This should be a bug, "return" or "goto cleanup;" is missed. But it
can be a seperate patch.


          }
          flags |= VIR_DOMAIN_SNAPSHOT_LIST_ROOTS;
      }
@@ -16381,6 +16385,7 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
          if (tree) {
              vshError(ctl, "%s",
                       _("--leaves and --tree are mutually exclusive"));
+            virDomainFree(dom);
              return false;
          }
          flags |= VIR_DOMAIN_SNAPSHOT_LIST_LEAVES;


ACK with "goto cleanup;"


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