Shorten the time needed to keep the list lock and alter the cleanup
path to be more of an error path.

Utilize the the virObjectListFree function to handle the calls for
virObjectUnref on each list element and the VIR_FREE of the list
instead of open coding it.

Change the name of the virHashForEach callback to match the name
of the Export function with the Callback added onto it.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/conf/virsecretobj.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 7a9908d..caed156 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -550,9 +550,9 @@ struct virSecretObjListData {
 };
 
 static int
-virSecretObjListPopulate(void *payload,
-                         const void *name ATTRIBUTE_UNUSED,
-                         void *opaque)
+virSecretObjListExportCallback(void *payload,
+                               const void *name ATTRIBUTE_UNUSED,
+                               void *opaque)
 {
     struct virSecretObjListData *data = opaque;
     virSecretObjPtr obj = payload;
@@ -598,7 +598,6 @@ virSecretObjListExport(virConnectPtr conn,
                        virSecretObjListACLFilter aclfilter,
                        unsigned int flags)
 {
-    int ret = -1;
     struct virSecretObjListData data = {
         .conn = conn, .secrets = NULL,
         .aclfilter = aclfilter, .flags = flags,
@@ -606,30 +605,28 @@ virSecretObjListExport(virConnectPtr conn,
 
     virObjectLock(secretobjs);
     if (secrets &&
-        VIR_ALLOC_N(data.secrets, virHashSize(secretobjs->objs) + 1) < 0)
-        goto cleanup;
+        VIR_ALLOC_N(data.secrets, virHashSize(secretobjs->objs) + 1) < 0) {
+        virObjectUnlock(secretobjs);
+        return -1;
+    }
 
-    virHashForEach(secretobjs->objs, virSecretObjListPopulate, &data);
+    virHashForEach(secretobjs->objs, virSecretObjListExportCallback, &data);
+    virObjectUnlock(secretobjs);
 
     if (data.error)
-        goto cleanup;
+        goto error;
 
     if (data.secrets) {
         /* trim the array to the final size */
         ignore_value(VIR_REALLOC_N(data.secrets, data.nsecrets + 1));
         *secrets = data.secrets;
-        data.secrets = NULL;
     }
 
-    ret = data.nsecrets;
+    return data.nsecrets;
 
- cleanup:
-    virObjectUnlock(secretobjs);
-    while (data.secrets && data.nsecrets)
-        virObjectUnref(data.secrets[--data.nsecrets]);
-
-    VIR_FREE(data.secrets);
-    return ret;
+ error:
+    virObjectListFree(data.secrets);
+    return -1;
 }
 
 
-- 
2.9.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Apr 24, 2017 at 02:00:23PM -0400, John Ferlan wrote:
> Shorten the time needed to keep the list lock and alter the cleanup
> path to be more of an error path.
> 
> Utilize the the virObjectListFree function to handle the calls for
> virObjectUnref on each list element and the VIR_FREE of the list
> instead of open coding it.
> 
> Change the name of the virHashForEach callback to match the name
> of the Export function with the Callback added onto it.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/conf/virsecretobj.c | 31 ++++++++++++++-----------------
>  1 file changed, 14 insertions(+), 17 deletions(-)

ACK

Pavel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list