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

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.