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

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.

Michal


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