[libvirt] [PATCH] virsh: Call virDomainFree in cmdDomFSTrim
Peter Krempa
pkrempa at redhat.com
Tue Apr 2 15:46:25 UTC 2013
On 04/02/13 17:40, Martin Kletzander wrote:
> 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.
This actually doesn't conform with the rest of the virsh that either
uses "goto cleanup" or "return false".
>>
>
> I'd personally prefer virDomainFree() to gracefully handle NULL as
> parameter, but life just ain't that simple ;-)
I tried that some time ago, wasn't accepted warmly :(
>
> [JK, there just wasn't much time to enjoy the April Fool's day.]
The Czech republic didn't have the chance to but some of the libvirt
folks had to work on monday unfortunately :)
>
> Martin
Peter
More information about the libvir-list
mailing list