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. > I'd personally prefer virDomainFree() to gracefully handle NULL as parameter, but life just ain't that simple ;-) [JK, there just wasn't much time to enjoy the April Fool's day.] Martin -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list