On 03/28/2012 04:12 PM, Osier Yang wrote:
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@xxxxxxxxxx>
---
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,
Yeah, I forgot 'ret' has a 'false' initial value.
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.
Yeah, I should include previous change in here.
}
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;"
Thanks,
Alex
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list