This patch replaces the listInsert and listUnlink with the more commonly
used VIR_APPEND_ELEMENT and VIR_REMOVE_ELEMENT macros used for list mgmt.

Of course that does require any code walking the ->next list to instead
use the ->count for loop

Signed-off-by: John Ferlan <jfer...@redhat.com>
---
 src/secret/secret_driver.c | 244 +++++++++++++++++++++++----------------------
 1 file changed, 124 insertions(+), 120 deletions(-)

diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index fb24237..deb8c59 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -55,7 +55,6 @@ enum { SECRET_MAX_XML_FILE = 10*1024*1024 };
 typedef struct _virSecretObj virSecretObj;
 typedef virSecretObj *virSecretObjPtr;
 struct _virSecretObj {
-    virSecretObjPtr next;
     char *configFile;
     char *base64File;
     virSecretDefPtr def;
@@ -63,11 +62,18 @@ struct _virSecretObj {
     size_t value_size;
 };
 
+typedef struct _virSecretObjList virSecretObjList;
+typedef virSecretObjList *virSecretObjListPtr;
+struct _virSecretObjList {
+    size_t count;
+    virSecretObjPtr *objs;
+};
+
 typedef struct _virSecretDriverState virSecretDriverState;
 typedef virSecretDriverState *virSecretDriverStatePtr;
 struct _virSecretDriverState {
     virMutex lock;
-    virSecretObj *secrets;
+    virSecretObjList secrets;
     char *configDir;
 };
 
@@ -85,23 +91,6 @@ secretDriverUnlock(void)
     virMutexUnlock(&driver->lock);
 }
 
-static virSecretObjPtr
-listUnlink(virSecretObjPtr *pptr)
-{
-    virSecretObjPtr secret;
-
-    secret = *pptr;
-    *pptr = secret->next;
-    return secret;
-}
-
-static void
-listInsert(virSecretObjPtr *pptr,
-           virSecretObjPtr secret)
-{
-    secret->next = *pptr;
-    *pptr = secret;
-}
 
 static void
 secretFree(virSecretObjPtr secret)
@@ -119,13 +108,47 @@ secretFree(virSecretObjPtr secret)
     VIR_FREE(secret);
 }
 
+
+static void
+secretObjListFree(virSecretObjListPtr secrets)
+{
+    size_t i;
+
+    for (i = 0; i < secrets->count; i++)
+        secretFree(secrets->objs[i]);
+    VIR_FREE(secrets->objs);
+    secrets->count = 0;
+}
+
+
+static void
+secretObjRemove(virSecretObjListPtr secrets,
+                virSecretObjPtr secret)
+{
+    size_t i;
+
+    if (!secret)
+        return;
+
+    for (i = 0; i < secrets->count; i++) {
+        if (secrets->objs[i] == secret) {
+            secretFree(secrets->objs[i]);
+            VIR_DELETE_ELEMENT(secrets->objs, i, secrets->count);
+            break;
+        }
+    }
+}
+
+
 static virSecretObjPtr
-secretFindByUUID(const unsigned char *uuid)
+secretFindByUUID(virSecretObjListPtr secrets,
+                 const unsigned char *uuid)
 {
-    virSecretObjPtr *pptr, secret;
+    size_t i;
+    virSecretObjPtr secret;
 
-    for (pptr = &driver->secrets; *pptr != NULL; pptr = &secret->next) {
-        secret = *pptr;
+    for (i = 0; i < secrets->count; i++) {
+        secret = secrets->objs[i];
         if (memcmp(secret->def->uuid, uuid, VIR_UUID_BUFLEN) == 0)
             return secret;
     }
@@ -133,13 +156,15 @@ secretFindByUUID(const unsigned char *uuid)
 }
 
 static virSecretObjPtr
-secretFindByUsage(int usageType,
+secretFindByUsage(virSecretObjListPtr secrets,
+                  int usageType,
                   const char *usageID)
 {
-    virSecretObjPtr *pptr, secret;
+    size_t i;
+    virSecretObjPtr secret;
 
-    for (pptr = &driver->secrets; *pptr != NULL; pptr = &secret->next) {
-        secret = *pptr;
+    for (i = 0; i < secrets->count; i++) {
+        secret = secrets->objs[i];
 
         if (secret->def->usage_type != usageType)
             continue;
@@ -170,7 +195,7 @@ secretFindByUsage(int usageType,
 
 
 static virSecretObjPtr
-secretAssignDef(virSecretObjPtr *list,
+secretAssignDef(virSecretObjListPtr list,
                 virSecretDefPtr def)
 {
     virSecretObjPtr secret;
@@ -178,7 +203,11 @@ secretAssignDef(virSecretObjPtr *list,
     if (VIR_ALLOC(secret) < 0)
         return NULL;
 
-    listInsert(list, secret);
+    if (VIR_APPEND_ELEMENT_COPY(list->objs, list->count, secret) < 0) {
+        secretFree(secret);
+        return NULL;
+    }
+
     secret->def = def;
 
     return secret;
@@ -370,27 +399,8 @@ secretLoadValue(virSecretObjPtr secret)
 }
 
 
-static void
-listUnlinkSecret(virSecretObjPtr *pptr,
-                 virSecretObjPtr secret)
-{
-    if (!secret)
-        return;
-
-    if (*pptr == secret) {
-        *pptr = secret->next;
-    } else {
-        virSecretObjPtr tmp = *pptr;
-        while (tmp && tmp->next != secret)
-            tmp = tmp->next;
-        if (tmp)
-            tmp->next = secret->next;
-    }
-}
-
-
 static virSecretObjPtr
-secretLoad(virSecretObjPtr *list,
+secretLoad(virSecretObjListPtr list,
            const char *file,
            const char *path,
            const char *base64path)
@@ -421,19 +431,18 @@ secretLoad(virSecretObjPtr *list,
     secret = NULL;
 
  cleanup:
-    listUnlinkSecret(list, secret);
-    secretFree(secret);
+    secretObjRemove(list, secret);
     virSecretDefFree(def);
     return ret;
 }
 
+
 static int
-secretLoadAllConfigs(virSecretObjPtr *dest,
+secretLoadAllConfigs(virSecretObjListPtr secrets,
                      const char *configDir)
 {
     DIR *dir = NULL;
     struct dirent *de;
-    virSecretObjPtr list = NULL;
 
     if (!(dir = opendir(configDir))) {
         if (errno == ENOENT)
@@ -442,6 +451,8 @@ secretLoadAllConfigs(virSecretObjPtr *dest,
         return -1;
     }
 
+    /* Ignore errors reported by readdir or other calls within the
+     * loop (if any).  It's better to keep the secrets we managed to find. */
     while (virDirRead(dir, &de, NULL) > 0) {
         char *path, *base64name, *base64path;
         virSecretObjPtr secret;
@@ -466,7 +477,7 @@ secretLoadAllConfigs(virSecretObjPtr *dest,
         }
         VIR_FREE(base64name);
 
-        if (!(secret = secretLoad(&list, de->d_name, path, base64path))) {
+        if (!(secret = secretLoad(secrets, de->d_name, path, base64path))) {
             virErrorPtr err = virGetLastError();
 
             VIR_ERROR(_("Error reading secret: %s"),
@@ -480,15 +491,6 @@ secretLoadAllConfigs(virSecretObjPtr *dest,
         VIR_FREE(path);
         VIR_FREE(base64path);
     }
-    /* Ignore error reported by readdir, if any.  It's better to keep the
-       secrets we managed to find. */
-
-    while (list != NULL) {
-        virSecretObjPtr secret;
-
-        secret = listUnlink(&list);
-        listInsert(dest, secret);
-    }
 
     closedir(dir);
     return 0;
@@ -500,6 +502,7 @@ static int
 secretConnectNumOfSecrets(virConnectPtr conn)
 {
     size_t i;
+    int count = 0;
     virSecretObjPtr secret;
 
     if (virConnectNumOfSecretsEnsureACL(conn) < 0)
@@ -507,15 +510,14 @@ secretConnectNumOfSecrets(virConnectPtr conn)
 
     secretDriverLock();
 
-    i = 0;
-    for (secret = driver->secrets; secret != NULL; secret = secret->next) {
-        if (virConnectNumOfSecretsCheckACL(conn,
-                                           secret->def))
-            i++;
+    for (i = 0; i < driver->secrets.count; i++) {
+        secret = driver->secrets.objs[i];
+        if (virConnectNumOfSecretsCheckACL(conn, secret->def))
+            count++;
     }
 
     secretDriverUnlock();
-    return i;
+    return count;
 }
 
 static int
@@ -524,6 +526,7 @@ secretConnectListSecrets(virConnectPtr conn,
                          int maxuuids)
 {
     size_t i;
+    int count = 0;
     virSecretObjPtr secret;
 
     memset(uuids, 0, maxuuids * sizeof(*uuids));
@@ -533,23 +536,26 @@ secretConnectListSecrets(virConnectPtr conn,
 
     secretDriverLock();
 
-    i = 0;
-    for (secret = driver->secrets; secret != NULL; secret = secret->next) {
+    for (i = 0; i < driver->secrets.count; i++) {
         char *uuidstr;
-        if (!virConnectListSecretsCheckACL(conn,
-                                           secret->def))
+
+        secret = driver->secrets.objs[i];
+        if (!virConnectListSecretsCheckACL(conn, secret->def))
             continue;
-        if (i == maxuuids)
+
+        if (count == maxuuids)
             break;
+
         if (VIR_ALLOC_N(uuidstr, VIR_UUID_STRING_BUFLEN) < 0)
             goto cleanup;
+
         virUUIDFormat(secret->def->uuid, uuidstr);
-        uuids[i] = uuidstr;
-        i++;
+        uuids[count] = uuidstr;
+        count++;
     }
 
     secretDriverUnlock();
-    return i;
+    return count;
 
  cleanup:
     secretDriverUnlock();
@@ -588,10 +594,9 @@ secretConnectListAllSecrets(virConnectPtr conn,
                             unsigned int flags)
 {
     virSecretPtr *tmp_secrets = NULL;
-    int nsecrets = 0;
     int ret_nsecrets = 0;
     virSecretObjPtr secret = NULL;
-    size_t i = 0;
+    size_t i;
     int ret = -1;
 
     virCheckFlags(VIR_CONNECT_LIST_SECRETS_FILTERS_ALL, -1);
@@ -601,15 +606,13 @@ secretConnectListAllSecrets(virConnectPtr conn,
 
     secretDriverLock();
 
-    for (secret = driver->secrets; secret != NULL; secret = secret->next)
-        nsecrets++;
-
-    if (secrets && VIR_ALLOC_N(tmp_secrets, nsecrets + 1) < 0)
+    if (secrets && VIR_ALLOC_N(tmp_secrets, driver->secrets.count + 1) < 0)
         goto cleanup;
 
-    for (secret = driver->secrets; secret != NULL; secret = secret->next) {
-        if (!virConnectListAllSecretsCheckACL(conn,
-                                              secret->def))
+    for (i = 0; i < driver->secrets.count; i++) {
+        secret = driver->secrets.objs[i];
+
+        if (!virConnectListAllSecretsCheckACL(conn, secret->def))
             continue;
 
         /* filter by whether it's ephemeral */
@@ -670,7 +673,7 @@ secretLookupByUUID(virConnectPtr conn,
 
     secretDriverLock();
 
-    if (!(secret = secretFindByUUID(uuid))) {
+    if (!(secret = secretFindByUUID(&driver->secrets, uuid))) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
         virUUIDFormat(uuid, uuidstr);
         virReportError(VIR_ERR_NO_SECRET,
@@ -702,7 +705,7 @@ secretLookupByUsage(virConnectPtr conn,
 
     secretDriverLock();
 
-    if (!(secret = secretFindByUsage(usageType, usageID))) {
+    if (!(secret = secretFindByUsage(&driver->secrets, usageType, usageID))) {
         virReportError(VIR_ERR_NO_SECRET,
                        _("no secret with matching usage '%s'"), usageID);
         goto cleanup;
@@ -742,14 +745,15 @@ secretDefineXML(virConnectPtr conn,
     if (virSecretDefineXMLEnsureACL(conn, new_attrs) < 0)
         goto cleanup;
 
-    if (!(secret = secretFindByUUID(new_attrs->uuid))) {
+    if (!(secret = secretFindByUUID(&driver->secrets, new_attrs->uuid))) {
         /* No existing secret with same UUID,
          * try look for matching usage instead */
         const char *usageID = secretUsageIDForDef(new_attrs);
         char uuidstr[VIR_UUID_STRING_BUFLEN];
 
         virUUIDFormat(secret->def->uuid, uuidstr);
-        if ((secret = secretFindByUsage(new_attrs->usage_type, usageID))) {
+        if ((secret = secretFindByUsage(&driver->secrets,
+                                        new_attrs->usage_type, usageID))) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("a secret with UUID %s already defined for "
                              "use with %s"),
@@ -757,7 +761,7 @@ secretDefineXML(virConnectPtr conn,
             goto cleanup;
         }
 
-        /* No existing secret at all, create one */
+        /* No existing secret at all, create one, add to driver secrets list */
         if (!(secret = secretAssignDef(&driver->secrets, new_attrs)))
             goto cleanup;
 
@@ -765,14 +769,14 @@ secretDefineXML(virConnectPtr conn,
          * the uuidstr, and .xml suffix */
         if (!(secret->configFile = virFileBuildPath(driver->configDir,
                                                     uuidstr, ".xml"))) {
-            secretFree(secret);
+            secretObjRemove(&driver->secrets, secret);
             goto cleanup;
         }
         /* Generate base64File using driver->configDir,
          * the uuidstr, and .base64 suffix */
         if (!(secret->base64File = virFileBuildPath(driver->configDir,
                                                     uuidstr, ".base64"))) {
-            secretFree(secret);
+            secretObjRemove(&driver->secrets, secret);
             goto cleanup;
         }
     } else {
@@ -830,12 +834,8 @@ secretDefineXML(virConnectPtr conn,
         /* Error - restore previous state and free new attributes */
         secret->def = backup;
     } else {
-        /* "secret" was added to the head of the list above */
-        if (listUnlink(&driver->secrets) != secret)
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("list of secrets is inconsistent"));
-        else
-            secretFree(secret);
+        /* "secret" was added to driver listthe head of the list above */
+        secretObjRemove(&driver->secrets, secret);
     }
 
  cleanup:
@@ -856,7 +856,7 @@ secretGetXMLDesc(virSecretPtr obj,
 
     secretDriverLock();
 
-    if (!(secret = secretFindByUUID(obj->uuid))) {
+    if (!(secret = secretFindByUUID(&driver->secrets, obj->uuid))) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
         virUUIDFormat(obj->uuid, uuidstr);
         virReportError(VIR_ERR_NO_SECRET,
@@ -893,7 +893,7 @@ secretSetValue(virSecretPtr obj,
 
     secretDriverLock();
 
-    if (!(secret = secretFindByUUID(obj->uuid))) {
+    if (!(secret = secretFindByUUID(&driver->secrets, obj->uuid))) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
         virUUIDFormat(obj->uuid, uuidstr);
         virReportError(VIR_ERR_NO_SECRET,
@@ -951,7 +951,7 @@ secretGetValue(virSecretPtr obj,
 
     secretDriverLock();
 
-    if (!(secret = secretFindByUUID(obj->uuid))) {
+    if (!(secret = secretFindByUUID(&driver->secrets, obj->uuid))) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
         virUUIDFormat(obj->uuid, uuidstr);
         virReportError(VIR_ERR_NO_SECRET,
@@ -996,7 +996,7 @@ secretUndefine(virSecretPtr obj)
 
     secretDriverLock();
 
-    if (!(secret = secretFindByUUID(obj->uuid))) {
+    if (!(secret = secretFindByUUID(&driver->secrets, obj->uuid))) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
         virUUIDFormat(obj->uuid, uuidstr);
         virReportError(VIR_ERR_NO_SECRET,
@@ -1011,8 +1011,7 @@ secretUndefine(virSecretPtr obj)
         secretDeleteSaved(secret) < 0)
         goto cleanup;
 
-    listUnlinkSecret(&driver->secrets, secret);
-    secretFree(secret);
+    secretObjRemove(&driver->secrets, secret);
 
     ret = 0;
 
@@ -1030,12 +1029,7 @@ secretStateCleanup(void)
 
     secretDriverLock();
 
-    while (driver->secrets != NULL) {
-        virSecretObjPtr secret;
-
-        secret = listUnlink(&driver->secrets);
-        secretFree(secret);
-    }
+    secretObjListFree(&driver->secrets);
     VIR_FREE(driver->configDir);
 
     secretDriverUnlock();
@@ -1088,7 +1082,8 @@ secretStateInitialize(bool privileged,
 static int
 secretStateReload(void)
 {
-    virSecretObjPtr new_secrets = NULL;
+    size_t i;
+    virSecretObjList new_secrets = {0, NULL};
 
     if (!driver)
         return -1;
@@ -1101,15 +1096,24 @@ secretStateReload(void)
     /* Keep ephemeral secrets from current state.
      * Discard non-ephemeral secrets that were removed
      * by the secrets configDir.  */
-    while (driver->secrets != NULL) {
-        virSecretObjPtr secret;
-
-        secret = listUnlink(&driver->secrets);
-        if (secret->def->ephemeral)
-            listInsert(&new_secrets, secret);
-        else
-            secretFree(secret);
+    for (i = 0; i < driver->secrets.count; i++) {
+        virSecretObjPtr secret = driver->secrets.objs[i];
+
+        if (secret->def->ephemeral) {
+            /* Remove from current, place on to be current soon */
+            VIR_DELETE_ELEMENT(driver->secrets.objs, i, driver->secrets.count);
+            if (VIR_APPEND_ELEMENT(new_secrets.objs, new_secrets.count,
+                                   secret) < 0) {
+                virErrorPtr err = virGetLastError();
+
+                VIR_ERROR(_("Error reloading ephemeral secret: %s"),
+                          err != NULL ? err->message : _("unknown error"));
+                virResetError(err);
+            }
+        }
+        secretFree(secret);
     }
+    secretObjListFree(&driver->secrets);
     driver->secrets = new_secrets;
 
  end:
-- 
2.5.0

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

Reply via email to