On 04/02/13 17:40, Martin Kletzander wrote:
On 04/02/2013 05:36 PM, Michal Privoznik wrote:
On 02.04.2013 17:32, Martin Kletzander wrote:
On 04/02/2013 05:20 PM, Michal Privoznik wrote:
The virsh domfstrim command was not freeing allocated domain,
leaving leaked references behind.
---
tools/virsh-domain.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 5ddcedc..5fbfeee 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10058,6 +10058,8 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd)
ret = true;
cleanup:
+ if (dom)
+ virDomainFree(dom);
return ret;
}
Alternatively, you could also get out of this with one line less:
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d32218f..99f19a4 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10057,7 +10057,7 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd)
unsigned int flags = 0;
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
- goto cleanup;
+ return false;
In fact, I prefer return ret here; as future changing of return value
will require less lines changed and I am used to that. I went with my
version.
This actually doesn't conform with the rest of the virsh that either
uses "goto cleanup" or "return false".
I'd personally prefer virDomainFree() to gracefully handle NULL as
parameter, but life just ain't that simple ;-)
I tried that some time ago, wasn't accepted warmly :(
[JK, there just wasn't much time to enjoy the April Fool's day.]
The Czech republic didn't have the chance to but some of the libvirt
folks had to work on monday unfortunately :)
Martin
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list