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 <jferlan@xxxxxxxxxx> --- 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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list