On 5/7/20, Peter Krempa <pkrempa@redhat.com> wrote:

On Thu, May 07, 2020 at 18:48:55 +0800, Yi Li wrote:

...

The lock should be released.

Signed-off-by: Yi Li <yili@winhong.com> --- src/conf/virstorageobj.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 13b75b648d..9f24ae67ca 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -2077,8 +2077,10 @@ virStoragePoolObjListExport(virConnectPtr conn,

virObjectRWLockRead(poolobjs);

- if (pools && VIR_ALLOC_N(data.pools, virHashSize(poolobjs->objs) + 1) < 0) - goto error; + if (pools && VIR_ALLOC_N(data.pools, virHashSize(poolobjs->objs) + 1) < 0) { + virObjectRWUnlock(poolobjs); + return -1; + }

In this instance, I'd prefer if VIR_ALLOC_N is replaced by g_new0 which will not fail and thus the unlock will not be necessary as there will be no cleanup path.

Thanks, i will redo it.

ping On Fri, May 8, 2020 at 12:23 AM Yi Li <yili@winhong.com> wrote:

Use g_new0 to allocate and remove NULL checks from callers and the lock will release properly

Signed-off-by: Yi Li <yili@winhong.com> --- src/conf/virstorageobj.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 13b75b648d..f3c54d0c52 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1036,10 +1036,7 @@ virStoragePoolObjVolumeListExport(virConnectPtr conn, return ret; }

- if (VIR_ALLOC_N(data.vols, virHashSize(volumes->objsName) + 1) < 0) { - virObjectRWUnlock(volumes); - return -1; - } + data.vols = g_new0(virStorageVolPtr, virHashSize(volumes->objsName) + 1);

virHashForEach(volumes->objsName, virStoragePoolObjVolumeListExportCallback, &data); virObjectRWUnlock(volumes); @@ -2077,8 +2074,13 @@ virStoragePoolObjListExport(virConnectPtr conn,

virObjectRWLockRead(poolobjs);

- if (pools && VIR_ALLOC_N(data.pools, virHashSize(poolobjs->objs) + 1) < 0) - goto error; + if (!pools) { + int ret = virHashSize(poolobjs->objs); + virObjectRWUnlock(poolobjs); + return ret; + } + + data.pools = g_new0(virStoragePoolPtr, virHashSize(poolobjs->objs) + 1);

virHashForEach(poolobjs->objs, virStoragePoolObjListExportCallback, &data); virObjectRWUnlock(poolobjs); -- 2.25.3