[libvirt] [PATCH 0/7] Fix resource leaks detected by coverity
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=771021 For more details, please refer to the coverity logs attached in the BZ. There is still one error which I don't understand: Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/uml/uml_driver.c:325: alloc_fn: Calling allocation function "virDomainEventNewFromObj". /builddir/build/BUILD/libvirt-0.9.10/src/conf/domain_event.c:723: alloc_fn: Storage is returned from allocation function "virDomainEventNewFromDef". /builddir/build/BUILD/libvirt-0.9.10/src/conf/domain_event.c:728: alloc_fn: Storage is returned from allocation function "virDomainEventNew". /builddir/build/BUILD/libvirt-0.9.10/src/conf/domain_event.c:705: alloc_fn: Storage is returned from allocation function "virDomainEventNewInternal". /builddir/build/BUILD/libvirt-0.9.10/src/conf/domain_event.c:684: alloc_arg: "virAlloc" allocates memory that is stored into "event". /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/conf/domain_event.c:698: return_alloc: Returning allocated memory "event". /builddir/build/BUILD/libvirt-0.9.10/src/conf/domain_event.c:705: var_assign: Assigning: "event" = "virDomainEventNewInternal(0, id, name, uuid)". /builddir/build/BUILD/libvirt-0.9.10/src/conf/domain_event.c:713: return_alloc: Returning allocated memory "event". /builddir/build/BUILD/libvirt-0.9.10/src/conf/domain_event.c:728: return_alloc_fn: Directly returning storage allocated by "virDomainEventNew". /builddir/build/BUILD/libvirt-0.9.10/src/conf/domain_event.c:723: return_alloc_fn: Directly returning storage allocated by "virDomainEventNewFromDef". /builddir/build/BUILD/libvirt-0.9.10/src/uml/uml_driver.c:325: var_assign: Assigning: "event" = storage returned from "virDomainEventNewFromObj(dom, 5, 0)". /builddir/build/BUILD/libvirt-0.9.10/src/uml/uml_driver.c:325: overwrite_var: Overwriting "event" in call "event = virDomainEventNewFromObj(dom, 5, 0)" leaks the storage that "event" points to. I can't see how it's overwrote. And I'd persuade myself it's mistake of coverage. But appreciated if anybody could kill it if I blindly bypass the porblem. Osier Yang (7): Coverity: Fix the forward_null error in Python binding codes Coverity: Fix resource leaks in phyp driver Coverity: Fix resource leak in esx driver Coverity: Fix resource leak in xen driver Coverity: Fix resource leak in test driver Coverity: Fix resource leak in nodeinfo.c Coverity: Fix resource leak in virnetlink.c python/libvirt-override.c | 10 +++++----- src/esx/esx_vi.c | 4 ++++ src/nodeinfo.c | 1 + src/phyp/phyp_driver.c | 34 ++++++++++++++++++++-------------- src/test/test_driver.c | 1 + src/util/virnetlink.c | 9 +++++---- src/xen/xen_hypervisor.c | 5 +++-- src/xen/xen_inotify.c | 1 + 8 files changed, 40 insertions(+), 25 deletions(-) Regards, Osier -- 1.7.7.3
Related coverity log: Error: FORWARD_NULL: /builddir/build/BUILD/libvirt-0.9.10/python/libvirt-override.c:355: assign_zero: Assigning: "params" = 0. /builddir/build/BUILD/libvirt-0.9.10/python/libvirt-override.c:458: var_deref_model: Passing null variable "params" to function "getPyVirTypedParameter", which dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.) --- python/libvirt-override.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 56f96ba..7fd7cb7 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -435,14 +435,14 @@ libvirt_virDomainGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) goto error; } + if (VIR_ALLOC_N(params, nparams) < 0) { + error = PyErr_NoMemory(); + goto error; + } + if (nparams) { sumparams = nparams; - if (VIR_ALLOC_N(params, nparams) < 0) { - error = PyErr_NoMemory(); - goto error; - } - LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetCPUStats(domain, params, nparams, -1, 1, flags); LIBVIRT_END_ALLOW_THREADS; -- 1.7.7.3
On Wed, May 02, 2012 at 10:51:32PM +0800, Osier Yang wrote:
Related coverity log:
Error: FORWARD_NULL: /builddir/build/BUILD/libvirt-0.9.10/python/libvirt-override.c:355: assign_zero: Assigning: "params" = 0. /builddir/build/BUILD/libvirt-0.9.10/python/libvirt-override.c:458: var_deref_model: Passing null variable "params" to function "getPyVirTypedParameter", which dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.) --- python/libvirt-override.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 56f96ba..7fd7cb7 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -435,14 +435,14 @@ libvirt_virDomainGetCPUStats(PyObject *self ATTRIBUTE_UNUSED, PyObject *args) goto error; }
+ if (VIR_ALLOC_N(params, nparams) < 0) { + error = PyErr_NoMemory(); + goto error; + } + if (nparams) { sumparams = nparams;
- if (VIR_ALLOC_N(params, nparams) < 0) { - error = PyErr_NoMemory(); - goto error; - } - LIBVIRT_BEGIN_ALLOW_THREADS; i_retval = virDomainGetCPUStats(domain, params, nparams, -1, 1, flags); LIBVIRT_END_ALLOW_THREADS;
NACK, the correct fix is to remove teh NONNULL annotation from getPyVirTypedParameter() since it is perfectly acceptable to pass in NULL there & the code handles it fine. 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 :|
Related coverity log: Error: FORWARD_NULL: /builddir/build/BUILD/libvirt-0.9.10/python/libvirt-override.c:355: assign_zero: Assigning: "params" = 0. /builddir/build/BUILD/libvirt-0.9.10/python/libvirt-override.c:458: var_deref_model: Passing null variable "params" to function "getPyVirTypedParameter", which dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.) --- python/libvirt-override.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 56f96ba..130e702 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -71,7 +71,7 @@ static char *py_str(PyObject *obj) /* Helper function to convert a virTypedParameter output array into a * Python dictionary for return to the user. Return NULL on failure, * after raising a python exception. */ -static PyObject * ATTRIBUTE_NONNULL(1) +static PyObject * getPyVirTypedParameter(const virTypedParameterPtr params, int nparams) { PyObject *key, *val, *info; -- 1.7.7.3
On 05/02/2012 09:05 AM, Osier Yang wrote:
Related coverity log:
Error: FORWARD_NULL: /builddir/build/BUILD/libvirt-0.9.10/python/libvirt-override.c:355: assign_zero: Assigning: "params" = 0. /builddir/build/BUILD/libvirt-0.9.10/python/libvirt-override.c:458: var_deref_model: Passing null variable "params" to function "getPyVirTypedParameter", which dereferences it. (The dereference is assumed on the basis of the 'nonnull' parameter attribute.) --- python/libvirt-override.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/python/libvirt-override.c b/python/libvirt-override.c index 56f96ba..130e702 100644 --- a/python/libvirt-override.c +++ b/python/libvirt-override.c @@ -71,7 +71,7 @@ static char *py_str(PyObject *obj) /* Helper function to convert a virTypedParameter output array into a * Python dictionary for return to the user. Return NULL on failure, * after raising a python exception. */ -static PyObject * ATTRIBUTE_NONNULL(1) +static PyObject *
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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 | 34 ++++++++++++++++++++-------------- 1 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index b883b56..2f81a89 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -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; } -- 1.7.7.3
On 05/02/2012 08:51 AM, Osier Yang wrote:
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.
--- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -582,6 +582,7 @@ err: libssh2_channel_free(channel); channel = NULL; } + VIR_FORCE_FCLOSE(fd); return -1; }
ACK if you squash this in (otherwise, you can end up calling fclose(garbage), never a good idea). [And why did we name it fd? That's a lousy name, because it makes me think of low-level fds for open/close, not high-level files for fopen/fclose.] diff --git i/src/phyp/phyp_driver.c w/src/phyp/phyp_driver.c index b883b56..25336ef 100644 --- i/src/phyp/phyp_driver.c +++ w/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"; -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1893: alloc_arg: Calling allocation function "esxVI_ObjectSpec_Alloc" on "objectSpec". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi_types.generated.c:2065: alloc_arg: "esxVI_Alloc" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1626: alloc_arg: "virAllocN" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:2006: leaked_storage: Variable "objectSpec" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1945: alloc_arg: Calling allocation function "esxVI_PropertySpec_Alloc" on "propertySpec". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi_types.generated.c:2693: alloc_arg: "esxVI_Alloc" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1626: alloc_arg: "virAllocN" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:2006: leaked_storage: Variable "propertySpec" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:3913: alloc_arg: Calling allocation function "esxVI_ObjectSpec_Alloc" on "objectSpec". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi_types.generated.c:2065: alloc_arg: "esxVI_Alloc" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1626: alloc_arg: "virAllocN" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:4075: leaked_storage: Variable "objectSpec" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:3920: alloc_arg: Calling allocation function "esxVI_PropertySpec_Alloc" on "propertySpec". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi_types.generated.c:2693: alloc_arg: "esxVI_Alloc" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1626: alloc_arg: "virAllocN" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:4075: leaked_storage: Variable "propertySpec" going out of scope leaks the storage it points to. --- src/esx/esx_vi.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 42e0976..1c30e5a 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2001,6 +2001,8 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx, propertySpec->pathSet = NULL; } + esxVI_ObjectSpec_Free(&objectSpec); + esxVI_PropertySpec_Free(&propertySpec); esxVI_PropertyFilterSpec_Free(&propertyFilterSpec); return result; @@ -4066,6 +4068,8 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, propertySpec->type = NULL; } + esxVI_ObjectSpec_Free(&objectSpec); + esxVI_PropertySpec_Free(&propertySpec); esxVI_PropertyFilterSpec_Free(&propertyFilterSpec); esxVI_ManagedObjectReference_Free(&propertyFilter); VIR_FREE(version); -- 1.7.7.3
2012/5/2 Osier Yang <jyang@redhat.com>:
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1893: alloc_arg: Calling allocation function "esxVI_ObjectSpec_Alloc" on "objectSpec". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi_types.generated.c:2065: alloc_arg: "esxVI_Alloc" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1626: alloc_arg: "virAllocN" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:2006: leaked_storage: Variable "objectSpec" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1945: alloc_arg: Calling allocation function "esxVI_PropertySpec_Alloc" on "propertySpec". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi_types.generated.c:2693: alloc_arg: "esxVI_Alloc" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1626: alloc_arg: "virAllocN" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:2006: leaked_storage: Variable "propertySpec" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:3913: alloc_arg: Calling allocation function "esxVI_ObjectSpec_Alloc" on "objectSpec". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi_types.generated.c:2065: alloc_arg: "esxVI_Alloc" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1626: alloc_arg: "virAllocN" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:4075: leaked_storage: Variable "objectSpec" going out of scope leaks the storage it points to.
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:3920: alloc_arg: Calling allocation function "esxVI_PropertySpec_Alloc" on "propertySpec". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi_types.generated.c:2693: alloc_arg: "esxVI_Alloc" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:1626: alloc_arg: "virAllocN" allocates memory that is stored into "*ptrptr". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/esx/esx_vi.c:4075: leaked_storage: Variable "propertySpec" going out of scope leaks the storage it points to. --- src/esx/esx_vi.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index 42e0976..1c30e5a 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -2001,6 +2001,8 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx, propertySpec->pathSet = NULL; }
+ esxVI_ObjectSpec_Free(&objectSpec); + esxVI_PropertySpec_Free(&propertySpec); esxVI_PropertyFilterSpec_Free(&propertyFilterSpec);
return result; @@ -4066,6 +4068,8 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx, propertySpec->type = NULL; }
+ esxVI_ObjectSpec_Free(&objectSpec); + esxVI_PropertySpec_Free(&propertySpec); esxVI_PropertyFilterSpec_Free(&propertyFilterSpec); esxVI_ManagedObjectReference_Free(&propertyFilter); VIR_FREE(version);
NACK, Coverity is wrong here. The following lines in esxVI_LookupObjectContentByType and esxVI_WaitForTaskCompletion thransfer ownership of the objects to another one that will free them. esxVI_PropertySpec_AppendToList(&propertyFilterSpec->propSet, propertySpec) esxVI_ObjectSpec_AppendToList(&propertyFilterSpec->objectSet, objectSpec) So there is no leak in the common case here. Yes there is a possibility in a certain error path to leak them, but your patch creates a double-free in the common case. I'll post a patch later on to fix the possible leak in the special error path. -- Matthias Bolte http://photron.blogspot.com
Coverity logs: Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_inotify.c:103: alloc_fn: Calling allocation function "xenDaemonLookupByUUID". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xend_internal.c:2534: alloc_fn: Storage is returned from allocation function "virGetDomain". /builddir/build/BUILD/libvirt-0.9.10/src/datatypes.c:191: 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:210: return_alloc: Returning allocated memory "ret". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xend_internal.c:2534: var_assign: Assigning: "ret" = "virGetDomain(conn, name, uuid)". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xend_internal.c:2541: return_alloc: Returning allocated memory "ret". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_inotify.c:103: var_assign: Assigning: "dom" = storage returned from "xenDaemonLookupByUUID(conn, rawuuid)". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_inotify.c:126: leaked_storage: Variable "dom" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2742: alloc_fn: Calling allocation function "fopen". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2742: var_assign: Assigning: "cpuinfo" = storage returned from "fopen("/proc/cpuinfo", "r")". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2763: noescape: Variable "cpuinfo" is not freed or pointed-to in function "xenHypervisorMakeCapabilitiesInternal". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2574:45: noescape: "xenHypervisorMakeCapabilitiesInternal" does not free or save its pointer parameter "cpuinfo". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2768: leaked_storage: Variable "cpuinfo" going out of scope leaks the storage it points to. Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2752: alloc_fn: Calling allocation function "fopen". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2752: var_assign: Assigning: "capabilities" = storage returned from "fopen("/sys/hypervisor/properties/capabilities", "r")". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2763: noescape: Variable "capabilities" is not freed or pointed-to in function "xenHypervisorMakeCapabilitiesInternal". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2574:60: noescape: "xenHypervisorMakeCapabilitiesInternal" does not free or save its pointer parameter "capabilities". /builddir/build/BUILD/libvirt-0.9.10/src/xen/xen_hypervisor.c:2768: leaked_storage: Variable "capabilities" going out of scope leaks the storage it points to. --- src/xen/xen_hypervisor.c | 5 +++-- src/xen/xen_inotify.c | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 0dae8d9..b4ec579 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -2732,7 +2732,7 @@ xenHypervisorMakeCapabilities(virConnectPtr conn) #ifdef __sun return xenHypervisorMakeCapabilitiesSunOS(conn); #else - virCapsPtr caps; + virCapsPtr caps = NULL; FILE *cpuinfo, *capabilities; struct utsname utsname; @@ -2765,11 +2765,12 @@ xenHypervisorMakeCapabilities(virConnectPtr conn) cpuinfo, capabilities); if (caps == NULL) - return NULL; + goto cleanup; if (virNodeSuspendGetTargetMask(&caps->host.powerMgmt) < 0) VIR_WARN("Failed to get host power management capabilities"); +cleanup: VIR_FORCE_FCLOSE(cpuinfo); VIR_FORCE_FCLOSE(capabilities); diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index d0cd907..f7495b5 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -123,6 +123,7 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, const char *filename, if (!(*name = strdup(dom->name))) { virReportOOMError(); + virDomainFree(dom); return -1; } memcpy(uuid, dom->uuid, VIR_UUID_BUFLEN); -- 1.7.7.3
On 05/02/2012 08:51 AM, Osier Yang wrote:
Coverity logs:
--- src/xen/xen_hypervisor.c | 5 +++-- src/xen/xen_inotify.c | 1 + 2 files changed, 4 insertions(+), 2 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/test/test_driver.c:1041: alloc_arg: Calling allocation function "virXPathNodeSet" on "devs". /builddir/build/BUILD/libvirt-0.9.10/src/util/xml.c:621: alloc_arg: "virAllocN" allocates memory that is stored into "*list". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/util/xml.c:625: noescape: Variable "*list" is not freed or pointed-to in function "memcpy". /builddir/build/BUILD/libvirt-0.9.10/src/test/test_driver.c:1098: leaked_storage: Variable "devs" going out of scope leaks the storage it points to. --- src/test/test_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 73a7cb1..7038741 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1090,6 +1090,7 @@ static int testOpenFromFile(virConnectPtr conn, VIR_FREE(networks); VIR_FREE(ifaces); VIR_FREE(pools); + VIR_FREE(devs); virDomainObjListDeinit(&privconn->domains); virNetworkObjListFree(&privconn->networks); virInterfaceObjListFree(&privconn->ifaces); -- 1.7.7.3
On 05/02/2012 08:51 AM, Osier Yang wrote:
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/test/test_driver.c:1041: alloc_arg: Calling allocation function "virXPathNodeSet" on "devs". /builddir/build/BUILD/libvirt-0.9.10/src/util/xml.c:621: alloc_arg: "virAllocN" allocates memory that is stored into "*list". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: alloc_fn: Storage is returned from allocation function "calloc". /builddir/build/BUILD/libvirt-0.9.10/src/util/memory.c:129: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(count, size)". /builddir/build/BUILD/libvirt-0.9.10/src/util/xml.c:625: noescape: Variable "*list" is not freed or pointed-to in function "memcpy". /builddir/build/BUILD/libvirt-0.9.10/src/test/test_driver.c:1098: leaked_storage: Variable "devs" going out of scope leaks the storage it points to. --- src/test/test_driver.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 73a7cb1..7038741 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1090,6 +1090,7 @@ static int testOpenFromFile(virConnectPtr conn, VIR_FREE(networks); VIR_FREE(ifaces); VIR_FREE(pools); + VIR_FREE(devs); virDomainObjListDeinit(&privconn->domains);
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: alloc_fn: Calling allocation function "fopen". /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: var_assign: Assigning: "cpuinfo" = storage returned from "fopen("/proc/cpuinfo", "r")". /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:638: leaked_storage: Variable "cpuinfo" going out of scope leaks the storage it points to. --- src/nodeinfo.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index e842474..4072da2 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -636,6 +636,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { } if (virAsprintf(&sysfs_cpuinfo, CPU_SYS_PATH) < 0) { + VIR_FORCE_FCLOSE(cpuinfo); virReportOOMError(); return -1; } -- 1.7.7.3
On Wed, May 02, 2012 at 10:51:37PM +0800, Osier Yang wrote:
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: alloc_fn: Calling allocation function "fopen". /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: var_assign: Assigning: "cpuinfo" = storage returned from "fopen("/proc/cpuinfo", "r")". /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:638: leaked_storage: Variable "cpuinfo" going out of scope leaks the storage it points to. --- src/nodeinfo.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c index e842474..4072da2 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -636,6 +636,7 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { }
if (virAsprintf(&sysfs_cpuinfo, CPU_SYS_PATH) < 0) { + VIR_FORCE_FCLOSE(cpuinfo); virReportOOMError(); return -1; }
This method would really benefit from being changed to use the 'goto cleanup' style rather than duplicating cleanup in the many 'return' statements. 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 :|
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: alloc_fn: Calling allocation function "fopen". /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: var_assign: Assigning: "cpuinfo" = storage returned from "fopen("/proc/cpuinfo", "r")". /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:638: leaked_storage: Variable "cpuinfo" going out of scope leaks the storage it points to. --- src/nodeinfo.c | 17 ++++++++--------- 1 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index e842474..56b9f54 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -626,8 +626,8 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { #ifdef __linux__ { - int ret; - char *sysfs_cpuinfo; + int ret = -1; + char *sysfs_cpuinfo = NULL; FILE *cpuinfo = fopen(CPUINFO_PATH, "r"); if (!cpuinfo) { virReportSystemError(errno, @@ -637,20 +637,19 @@ int nodeGetInfo(virConnectPtr conn ATTRIBUTE_UNUSED, virNodeInfoPtr nodeinfo) { if (virAsprintf(&sysfs_cpuinfo, CPU_SYS_PATH) < 0) { virReportOOMError(); - return -1; + goto cleanup; } ret = linuxNodeInfoCPUPopulate(cpuinfo, sysfs_cpuinfo, nodeinfo); - VIR_FORCE_FCLOSE(cpuinfo); - if (ret < 0) { - VIR_FREE(sysfs_cpuinfo); - return -1; - } + if (ret < 0) + goto cleanup; - VIR_FREE(sysfs_cpuinfo); /* Convert to KB. */ nodeinfo->memory = physmem_total () / 1024; +cleanup: + VIR_FORCE_FCLOSE(cpuinfo); + VIR_FREE(sysfs_cpuinfo); return ret; } #else -- 1.7.7.3
On 05/02/2012 09:18 AM, Osier Yang wrote:
Error: RESOURCE_LEAK: /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: alloc_fn: Calling allocation function "fopen". /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:629: var_assign: Assigning: "cpuinfo" = storage returned from "fopen("/proc/cpuinfo", "r")". /builddir/build/BUILD/libvirt-0.9.10/src/nodeinfo.c:638: leaked_storage: Variable "cpuinfo" going out of scope leaks the storage it points to. --- src/nodeinfo.c | 17 ++++++++--------- 1 files changed, 8 insertions(+), 9 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/util/virnetlink.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index b2e9d51..33db8db 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -337,11 +337,13 @@ virNetlinkEventServiceStart(void) if (VIR_ALLOC(srv) < 0) { virReportOOMError(); - goto error; + return -1; } - if (virMutexInit(&srv->lock) < 0) - goto error; + if (virMutexInit(&srv->lock) < 0) { + VIR_FREE(srv); + return -1; + } virNetlinkEventServerLock(srv); @@ -400,7 +402,6 @@ error_locked: virMutexDestroy(&srv->lock); VIR_FREE(srv); } -error: return ret; } -- 1.7.7.3
On 05/02/2012 08:51 AM, Osier Yang wrote:
--- src/util/virnetlink.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-)
ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2012年05月03日 02:31, Eric Blake wrote:
On 05/02/2012 08:51 AM, Osier Yang wrote:
--- src/util/virnetlink.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-)
ACK.
Thanks for the reviewing, pushed the set without 3/7, and "FILE *fd = NULL" squashed in 2/7 (yeah, strange to name it as 'fd'). Regards, Osier
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Matthias Bolte -
Osier Yang