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 -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list