[libvirt] [PATCH] Domain/Net object cleanups after remote error

[Cole Robinson]

Daniel P. Berrange wrote:
> On Tue, May 20, 2008 at 10:44:52AM +0100, Richard W.M. Jones wrote:
>> On Mon, May 19, 2008 at 04:58:38PM -0400, Cole Robinson wrote:
>>> diff --git a/src/remote_internal.c b/src/remote_internal.c
>>> index 51e8eb7..80f6ce6 100644
>>> --- a/src/remote_internal.c
>>> +++ b/src/remote_internal.c
>>> @@ -4606,6 +4606,10 @@ server_error (virConnectPtr conn, remote_error *err)
>>>                       err->str3 ? *err->str3 : NULL,
>>>                       err->int1, err->int2,
>>>                       "%s", err->message ? *err->message : NULL);
>>> +    if (dom)
>>> +        virDomainFree(dom);
>>> +    if (net)
>>> +        virNetworkFree(net);
>>>  }
>> Extra long hmmmmmmmmmmm ...
>>
>> This is right, the domain and network are leaked.
>>
>> However the virterror structure containing these pointers will be used
>> later.  (__virRaiseError saves it and callers access it later).
>>
>> However^2 these entries in the virterror structure are deprecated.  I
>> added a patch last month which adds a big warning about using these
>> entries.  At most callers should look at the pointers themselves and
>> shouldn't dereference them (which would be the only safe thing to do
>> given that the pointers have just been freed).
> 
> Yes, that should be safe - the only domain / network object set in an error
> condition is one that is passed in via the original caller. So there should
> still be a reference count held on these objects further up the call stack
> and thus this code won't actually free them immediately, merely decrement
> the ref count. Can probably confirm this by turning on debug mode and checking
> the messages.
> 
> Dan.

I just tested without this patch, but with the other destroy leak fixes I
sent, and everything looks to be okay. So this patch can be ignored.

Thanks,
Cole