"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
While testing Cole's series of patches I identified a couple more
places
where we leak memory.
In libvirt.c, the default authentication callback uses uninitialized
data, and indeed strdup()'s it and this is then never released. This
simply disables that bit of code.
In qparams.c when free'ing the struct the 'p' struct field was not
released. I took the opportunity to switch it over to using the new
style memory.h functions
In remote.c there were a couple of handlers which forgot to free the
virDomainPtr object when they were done.
qemud/remote.c | 18 ++++++++++++++----
src/libvirt.c | 16 +++++++++-------
src/qparams.c | 31 +++++++++++++------------------
3 files changed, 36 insertions(+), 29 deletions(-)
Looks good.
I suppose the tests that exposed the leaks aren't yet run
as part of "make check". It'd be nice...
Index: qemud/remote.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/remote.c,v
retrieving revision 1.32
diff -u -p -r1.32 remote.c
--- qemud/remote.c 21 May 2008 20:53:30 -0000 1.32
+++ qemud/remote.c 21 May 2008 21:16:42 -0000
@@ -792,8 +792,11 @@ remoteDispatchDomainBlockStats (struct q
}
path = args->path;
- if (virDomainBlockStats (dom, path, &stats, sizeof stats) == -1)
+ if (virDomainBlockStats (dom, path, &stats, sizeof stats) == -1) {
+ virDomainFree (dom);
return -1;
+ }
+ virDomainFree (dom);
Recently, I've come to prefer using a single free call,
snuggled right up against the last use (as long as you don't
mind the additional variable). E.g.,
fail = (virDomainBlockStats (dom, path, &stats, sizeof stats) == -1);
virDomainFree (dom);
if (fail)
return -1;
ret->rd_req = stats.rd_req;
ret->rd_bytes = stats.rd_bytes;
@@ -823,8 +826,11 @@ remoteDispatchDomainInterfaceStats (stru
}
path = args->path;
- if (virDomainInterfaceStats (dom, path, &stats, sizeof stats) == -1)
+ if (virDomainInterfaceStats (dom, path, &stats, sizeof stats) == -1) {
+ virDomainFree (dom);
return -1;
+ }
+ virDomainFree (dom);
ret->rx_bytes = stats.rx_bytes;
ret->rx_packets = stats.rx_packets;
@@ -1258,7 +1264,11 @@ remoteDispatchDomainMigratePerform (stru
args->cookie.cookie_len,
args->uri,
args->flags, dname, args->resource);
- if (r == -1) return -1;
+ if (r == -1) {
+ virDomainFree (dom);
+ return -1;
+ }
+ virDomainFree (dom);
Here, you can call virDomainFree just once, before the "if", regardless.