From cff0d342adbbb955664c781b6c51bfd9b9d738b8 Mon Sep 17 00:00:00 2001 From: Osier Yang Date: Fri, 4 May 2012 10:25:58 +0800 Subject: [PATCH] Coverity: Fix resource leaks in phyp driver Coverity logs: Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:523: alloc_fn: Calling allocation function "fopen". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:523: var_assign: Assigning: "fd" = storage returned from "fopen(local_file, "rb")". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:540: noescape: Variable "fd" is not freed or pointed-to in function "fread". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:542: noescape: Variable "fd" is not freed or pointed-to in function "feof". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:575: leaked_storage: Variable "fd" going out of scope leaks the storage it points to. /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:585: leaked_storage: Variable "fd" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2088: alloc_fn: Calling allocation function "phypVolumeLookupByName". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2026: alloc_fn: Storage is returned from allocation function "virGetStorageVol". /builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:724: alloc_arg: "virAlloc" allocates memory that is stored into "ret". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)". /builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:753: return_alloc: Returning allocated memory "ret". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2026: var_assign: Assigning: "vol" = "virGetStorageVol(pool->conn, pool->name, volname, key)". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2030: return_alloc: Returning allocated memory "vol". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2088: leaked_storage: Failing to save storage allocated by "phypVolumeLookupByName(pool, voldef->name)" leaks it. Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2725: alloc_fn: Calling allocation function "phypGetStoragePoolLookUpByUUID". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2689: alloc_fn: Storage is returned from allocation function "virGetStoragePool". /builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:592: alloc_arg: "virAlloc" allocates memory that is stored into "ret". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)". /builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:610: return_alloc: Returning allocated memory "ret". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2689: var_assign: Assigning: "sp" = "virGetStoragePool(conn, pools[i], uuid)". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2694: return_alloc: Returning allocated memory "sp". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2725: leaked_storage: Failing to save storage allocated by "phypGetStoragePoolLookUpByUUID(conn, def->uuid)" leaks it. Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2719: alloc_fn: Calling allocation function "phypStoragePoolLookupByName". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2254: alloc_fn: Storage is returned from allocation function "virGetStoragePool". /builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:592: alloc_arg: "virAlloc" allocates memory that is stored into "ret". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)". /builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:610: return_alloc: Returning allocated memory "ret". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2254: return_alloc_fn: Directly returning storage allocated by "virGetStoragePool". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2719: leaked_storage: Failing to save storage allocated by "phypStoragePoolLookupByName(conn, def->name)" leaks it. Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2270: alloc_fn: Calling allocation function "phypStoragePoolLookupByName". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2254: alloc_fn: Storage is returned from allocation function "virGetStoragePool". /builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:592: alloc_arg: "virAlloc" allocates memory that is stored into "ret". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:101: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)". /builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:610: return_alloc: Returning allocated memory "ret". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2254: return_alloc_fn: Directly returning storage allocated by "virGetStoragePool". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2270: var_assign: Assigning: "sp" = storage returned from "phypStoragePoolLookupByName(vol->conn, vol->pool)". /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2324: leaked_storage: Variable "sp" going out of scope leaks the storage it points to. /builddir/build/BUILD/libvirt-0.9.10/src/phyp/phyp_driver.c:2327: leaked_storage: Variable "sp" going out of scope leaks the storage it points t --- src/phyp/phyp_driver.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index b883b56434..dc2a95f9b3 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -491,7 +491,7 @@ phypUUIDTable_Push(virConnectPtr conn) struct stat local_fileinfo; char buffer[1024]; int rc = 0; - FILE *fd; + FILE *fd = NULL; size_t nread, sent; char *ptr; char local_file[] = "./uuid_table"; @@ -582,6 +582,7 @@ err: libssh2_channel_free(channel); channel = NULL; } + VIR_FORCE_FCLOSE(fd); return -1; } @@ -2039,6 +2040,7 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, virStorageVolDefPtr voldef = NULL; virStoragePoolDefPtr spdef = NULL; virStorageVolPtr vol = NULL; + virStorageVolPtr dup_vol = NULL; char *key = NULL; if (VIR_ALLOC(spdef) < 0) { @@ -2085,8 +2087,9 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, } /* checking if this name already exists on this system */ - if (phypVolumeLookupByName(pool, voldef->name) != NULL) { + if ((dup_vol = phypVolumeLookupByName(pool, voldef->name)) != NULL) { VIR_ERROR(_("StoragePool name already exists.")); + virUnrefStorageVol(dup_vol); goto err; } @@ -2260,7 +2263,7 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) virStorageVolDef voldef; virStoragePoolDef pool; virStoragePoolPtr sp; - char *xml; + char *xml = NULL; virCheckFlags(0, NULL); @@ -2270,23 +2273,23 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) sp = phypStoragePoolLookupByName(vol->conn, vol->pool); if (!sp) - goto err; + goto cleanup; if (sp->name != NULL) { pool.name = sp->name; } else { VIR_ERROR(_("Unable to determine storage sp's name.")); - goto err; + goto cleanup; } if (memcpy(pool.uuid, sp->uuid, VIR_UUID_BUFLEN) == NULL) { VIR_ERROR(_("Unable to determine storage sp's uuid.")); - goto err; + goto cleanup; } if ((pool.capacity = phypGetStoragePoolSize(sp->conn, sp->name)) == -1) { VIR_ERROR(_("Unable to determine storage sps's size.")); - goto err; + goto cleanup; } /* Information not avaliable */ @@ -2298,21 +2301,21 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) if ((pool.source.adapter = phypGetStoragePoolDevice(sp->conn, sp->name)) == NULL) { VIR_ERROR(_("Unable to determine storage sps's source adapter.")); - goto err; + goto cleanup; } if (vol->name != NULL) voldef.name = vol->name; else { VIR_ERROR(_("Unable to determine storage pool's name.")); - goto err; + goto cleanup; } voldef.key = strdup(vol->key); if (voldef.key == NULL) { virReportOOMError(); - goto err; + goto cleanup; } voldef.type = VIR_STORAGE_POOL_LOGICAL; @@ -2321,10 +2324,10 @@ phypVolumeGetXMLDesc(virStorageVolPtr vol, unsigned int flags) VIR_FREE(voldef.key); +cleanup: + if (sp) + virUnrefStoragePool(sp); return xml; - -err: - return NULL; } /* The Volume Group path here will be treated as suggested in the @@ -2710,20 +2713,23 @@ phypStoragePoolCreateXML(virConnectPtr conn, virCheckFlags(0, NULL); virStoragePoolDefPtr def = NULL; + virStoragePoolPtr dup_sp = NULL; virStoragePoolPtr sp = NULL; if (!(def = virStoragePoolDefParseString(xml))) goto err; /* checking if this name already exists on this system */ - if (phypStoragePoolLookupByName(conn, def->name) != NULL) { + if ((dup_sp = phypStoragePoolLookupByName(conn, def->name)) != NULL) { VIR_WARN("StoragePool name already exists."); + virUnrefStoragePool(dup_sp); goto err; } /* checking if ID or UUID already exists on this system */ - if (phypGetStoragePoolLookUpByUUID(conn, def->uuid) != NULL) { + if ((dup_sp = phypGetStoragePoolLookUpByUUID(conn, def->uuid)) != NULL) { VIR_WARN("StoragePool uuid already exists."); + virUnrefStoragePool(dup_sp); goto err; } -- GitLab