On Sun, Apr 03, 2011 at 11:21:24AM +0200, Matthias Bolte wrote:
--- src/phyp/phyp_driver.c | 13 +++++++++++-- 1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index fe2e99d..76207c2 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -2362,13 +2362,22 @@ phypBuildVolume(virConnectPtr conn, const char *lvname, const char *spname, static virStorageVolPtr phypVolumeLookupByName(virStoragePoolPtr pool, const char *volname) { + char *key; + virStorageVolPtr vol;
- char key[MAX_KEY_SIZE]; + if (VIR_ALLOC_N(key, MAX_KEY_SIZE) < 0) { + virReportOOMError(); + return NULL; + }
if (phypVolumeGetKey(pool->conn, key, volname) == -1) return NULL;
- return virGetStorageVol(pool->conn, pool->name, volname, key); + vol = virGetStorageVol(pool->conn, pool->name, volname, key); + + VIR_FREE(key); + + return vol; }
I think the signature of phypVolumeGetKey() is rather dangerous - it is blindly assuming the caller allocates MAX_KEY_SIZE for 'key'. The phypVolumeGetKey knows exactly how long the key it has is, so it'd be better for it to allocate the buffer itself & return it to the callers I realize this isn't a new problem from your patch, but I reckon we should fix it here. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|