[libvirt] [PATCH] virsh: Call virDomainFree in cmdDomFSTrim
Martin Kletzander
mkletzan at redhat.com
Tue Apr 2 15:40:26 UTC 2013
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
More information about the libvir-list
mailing list