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.
--
|: Red Hat, Engineering, Boston -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|