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

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



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


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