Re: [libvirt] virStoragePoolSourceFree(S) does not free S
On Fri, Feb 05, 2010 at 05:26:35PM +0100, Jim Meyering wrote:
I was surprised to see that virStoragePoolSourceFree(S) does not free
S.
The other three vir*Free functions in storage_conf *do* free S.
[snip]
One fix might be to call VIR_FREE(def) in the "if
(def)..."
block, but that seems short-sighted, since the name
virStoragePoolSourceFree makes me think *it* should be
doing the whole job.
It is a bad name - it should be renamed to virStoragePoolSourceClear()
However, if I make the logical change to virStoragePoolSourceFree,
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 62b8394..ffe38cc 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -291,6 +291,7 @@ virStoragePoolSourceFree(virStoragePoolSourcePtr source) {
VIR_FREE(source->auth.chap.login);
VIR_FREE(source->auth.chap.passwd);
}
+ VIR_FREE(source);
}
that causes "make check" to fail miserably due to heap corruption,
as reported by valgrind:
I tracked the problem down to this definition in
src/conf/storage_conf.h:
typedef struct _virStoragePoolDef virStoragePoolDef;
typedef virStoragePoolDef *virStoragePoolDefPtr;
struct _virStoragePoolDef {
/* General metadata */
char *name;
unsigned char uuid[VIR_UUID_BUFLEN];
int type; /* virStoragePoolType */
unsigned long long allocation;
unsigned long long capacity;
unsigned long long available;
virStoragePoolSource source; <== this is a *STRUCT*, not a ptr
virStoragePoolTarget target; <== Likewise
};
Yep, the 'virStoragePoolSource' object is embedded directly in other
structs, not referenced via a pointer, so you can't free this object
directly most of the time... except we later added an internal API
which lets you use virStoragePoolSource as a standalone object which
does need free'ing.
I think we need to rename the current virStoragePoolSourceFree
to virStoragePoolSourceClear(), and then add a new implmenetation
of virStoragePoolSourceFree that calls Clear() and VIR_FREE(def)
making the latter be used where applicable.
Regards,
Daniel
--
|: Red Hat, Engineering, London -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 :|