Re: [libvirt] [PATCH] virsh: Call virDomainFree in cmdDomFSTrim

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.