[libvirt] [PATCH 05/14] secret: Use consistent naming for variables

John Ferlan jferlan at redhat.com
Tue Apr 25 11:57:44 UTC 2017



On 04/25/2017 04:29 AM, Pavel Hrdina wrote:
> On Mon, Apr 24, 2017 at 02:00:14PM -0400, John Ferlan wrote:
>> When processing a virSecretPtr use 'secret' as a variable name.
>>
>> When processing a virSecretObjPtr use 'obj' as a variable name.
>>
>> When processing a virSecretDefPtr use 'def' as a variable name,
>> unless a distinction needs to be made with a 'newdef' such as
>> virSecretObjListAddLocked (which also used the VIR_STEAL_PTR macro
>> for the configFile and base64File).
>>
>> NB: Also made a slight algorithm adjustment for virSecretObjListRemove
>> to check if the passed obj was NULL rather than having the caller
>> virSecretLoad need to check prior to calling.
> 
> I can see the motivation for this change, it's easier to follow the code
> if the naming is consistent, however this patch adds one extra step when
> tracing history of the code via git blame.

Yes and I'm certainly not the first one down this path!  Code motion
tends to be the biggest offender though. At least with git blame
variable name changes don't cause one to jump into some other module to
find the code.... My least favorite is when a module "disappears" or is
renamed - those always cause heartache, but I always find that gitk will
find whatever I need without having to know some magic git command.

> 
> All the changes except the variable rename should be in separate patch as
> they are unrelated to the rename.

I can split out the virSecretObjListRemove change into a separate patch
since it's the only non name change patch.

John
> 
> Pavel
> 
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>  src/conf/virsecretobj.c    | 275 +++++++++++++++++++++++----------------------
>>  src/conf/virsecretobj.h    |  26 ++---
>>  src/secret/secret_driver.c | 139 ++++++++++++-----------
>>  3 files changed, 226 insertions(+), 214 deletions(-)
>>
>> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
>> index 348feb3..42f36c8 100644
>> --- a/src/conf/virsecretobj.c
>> +++ b/src/conf/virsecretobj.c
>> @@ -89,29 +89,29 @@ VIR_ONCE_GLOBAL_INIT(virSecretObj)
>>  static virSecretObjPtr
>>  virSecretObjNew(void)
>>  {
>> -    virSecretObjPtr secret;
>> +    virSecretObjPtr obj;
>>  
>>      if (virSecretObjInitialize() < 0)
>>          return NULL;
>>  
>> -    if (!(secret = virObjectLockableNew(virSecretObjClass)))
>> +    if (!(obj = virObjectLockableNew(virSecretObjClass)))
>>          return NULL;
>>  
>> -    virObjectLock(secret);
>> +    virObjectLock(obj);
>>  
>> -    return secret;
>> +    return obj;
>>  }
>>  
>>  
>>  void
>> -virSecretObjEndAPI(virSecretObjPtr *secret)
>> +virSecretObjEndAPI(virSecretObjPtr *obj)
>>  {
>> -    if (!*secret)
>> +    if (!*obj)
>>          return;
>>  
>> -    virObjectUnlock(*secret);
>> -    virObjectUnref(*secret);
>> -    *secret = NULL;
>> +    virObjectUnlock(*obj);
>> +    virObjectUnref(*obj);
>> +    *obj = NULL;
>>  }
>>  
>>  
>> @@ -136,18 +136,18 @@ virSecretObjListNew(void)
>>  
>>  
>>  static void
>> -virSecretObjDispose(void *obj)
>> +virSecretObjDispose(void *opaque)
>>  {
>> -    virSecretObjPtr secret = obj;
>> +    virSecretObjPtr obj = opaque;
>>  
>> -    virSecretDefFree(secret->def);
>> -    if (secret->value) {
>> +    virSecretDefFree(obj->def);
>> +    if (obj->value) {
>>          /* Wipe before free to ensure we don't leave a secret on the heap */
>> -        memset(secret->value, 0, secret->value_size);
>> -        VIR_FREE(secret->value);
>> +        memset(obj->value, 0, obj->value_size);
>> +        VIR_FREE(obj->value);
>>      }
>> -    VIR_FREE(secret->configFile);
>> -    VIR_FREE(secret->base64File);
>> +    VIR_FREE(obj->configFile);
>> +    VIR_FREE(obj->base64File);
>>  }
>>  
>>  
>> @@ -186,14 +186,14 @@ virSecretObjPtr
>>  virSecretObjListFindByUUID(virSecretObjListPtr secrets,
>>                             const unsigned char *uuid)
>>  {
>> -    virSecretObjPtr ret;
>> +    virSecretObjPtr obj;
>>  
>>      virObjectLock(secrets);
>> -    ret = virSecretObjListFindByUUIDLocked(secrets, uuid);
>> +    obj = virSecretObjListFindByUUIDLocked(secrets, uuid);
>>      virObjectUnlock(secrets);
>> -    if (ret)
>> -        virObjectLock(ret);
>> -    return ret;
>> +    if (obj)
>> +        virObjectLock(obj);
>> +    return obj;
>>  }
>>  
>>  
>> @@ -202,21 +202,21 @@ virSecretObjSearchName(const void *payload,
>>                         const void *name ATTRIBUTE_UNUSED,
>>                         const void *opaque)
>>  {
>> -    virSecretObjPtr secret = (virSecretObjPtr) payload;
>> +    virSecretObjPtr obj = (virSecretObjPtr) payload;
>>      struct virSecretSearchData *data = (struct virSecretSearchData *) opaque;
>>      int found = 0;
>>  
>> -    virObjectLock(secret);
>> +    virObjectLock(obj);
>>  
>> -    if (secret->def->usage_type != data->usageType)
>> +    if (obj->def->usage_type != data->usageType)
>>          goto cleanup;
>>  
>>      if (data->usageType != VIR_SECRET_USAGE_TYPE_NONE &&
>> -        STREQ(secret->def->usage_id, data->usageID))
>> +        STREQ(obj->def->usage_id, data->usageID))
>>          found = 1;
>>  
>>   cleanup:
>> -    virObjectUnlock(secret);
>> +    virObjectUnlock(obj);
>>      return found;
>>  }
>>  
>> @@ -226,14 +226,14 @@ virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
>>                                    int usageType,
>>                                    const char *usageID)
>>  {
>> -    virSecretObjPtr ret = NULL;
>> +    virSecretObjPtr obj = NULL;
>>      struct virSecretSearchData data = { .usageType = usageType,
>>                                          .usageID = usageID };
>>  
>> -    ret = virHashSearch(secrets->objs, virSecretObjSearchName, &data);
>> -    if (ret)
>> -        virObjectRef(ret);
>> -    return ret;
>> +    obj = virHashSearch(secrets->objs, virSecretObjSearchName, &data);
>> +    if (obj)
>> +        virObjectRef(obj);
>> +    return obj;
>>  }
>>  
>>  
>> @@ -253,14 +253,14 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
>>                              int usageType,
>>                              const char *usageID)
>>  {
>> -    virSecretObjPtr ret;
>> +    virSecretObjPtr obj;
>>  
>>      virObjectLock(secrets);
>> -    ret = virSecretObjListFindByUsageLocked(secrets, usageType, usageID);
>> +    obj = virSecretObjListFindByUsageLocked(secrets, usageType, usageID);
>>      virObjectUnlock(secrets);
>> -    if (ret)
>> -        virObjectLock(ret);
>> -    return ret;
>> +    if (obj)
>> +        virObjectLock(obj);
>> +    return obj;
>>  }
>>  
>>  
>> @@ -275,19 +275,22 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
>>   */
>>  void
>>  virSecretObjListRemove(virSecretObjListPtr secrets,
>> -                       virSecretObjPtr secret)
>> +                       virSecretObjPtr obj)
>>  {
>>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>>  
>> -    virUUIDFormat(secret->def->uuid, uuidstr);
>> -    virObjectRef(secret);
>> -    virObjectUnlock(secret);
>> +    if (!obj)
>> +        return;
>> +
>> +    virUUIDFormat(obj->def->uuid, uuidstr);
>> +    virObjectRef(obj);
>> +    virObjectUnlock(obj);
>>  
>>      virObjectLock(secrets);
>> -    virObjectLock(secret);
>> +    virObjectLock(obj);
>>      virHashRemoveEntry(secrets->objs, uuidstr);
>> -    virObjectUnlock(secret);
>> -    virObjectUnref(secret);
>> +    virObjectUnlock(obj);
>> +    virObjectUnref(obj);
>>      virObjectUnlock(secrets);
>>  }
>>  
>> @@ -295,11 +298,11 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
>>  /*
>>   * virSecretObjListAddLocked:
>>   * @secrets: list of secret objects
>> - * @def: new secret definition
>> + * @newdef: new secret definition
>>   * @configDir: directory to place secret config files
>>   * @oldDef: Former secret def (e.g. a reload path perhaps)
>>   *
>> - * Add the new def to the secret obj table hash
>> + * Add the new @newdef to the secret obj table hash
>>   *
>>   * This functions requires @secrets to be locked already!
>>   *
>> @@ -307,11 +310,11 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
>>   */
>>  static virSecretObjPtr
>>  virSecretObjListAddLocked(virSecretObjListPtr secrets,
>> -                          virSecretDefPtr def,
>> +                          virSecretDefPtr newdef,
>>                            const char *configDir,
>>                            virSecretDefPtr *oldDef)
>>  {
>> -    virSecretObjPtr secret;
>> +    virSecretObjPtr obj;
>>      virSecretObjPtr ret = NULL;
>>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>>      char *configFile = NULL, *base64File = NULL;
>> @@ -320,71 +323,69 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
>>          *oldDef = NULL;
>>  
>>      /* Is there a secret already matching this UUID */
>> -    if ((secret = virSecretObjListFindByUUIDLocked(secrets, def->uuid))) {
>> -        virObjectLock(secret);
>> +    if ((obj = virSecretObjListFindByUUIDLocked(secrets, newdef->uuid))) {
>> +        virObjectLock(obj);
>>  
>> -        if (STRNEQ_NULLABLE(secret->def->usage_id, def->usage_id)) {
>> -            virUUIDFormat(secret->def->uuid, uuidstr);
>> +        if (STRNEQ_NULLABLE(obj->def->usage_id, newdef->usage_id)) {
>> +            virUUIDFormat(obj->def->uuid, uuidstr);
>>              virReportError(VIR_ERR_INTERNAL_ERROR,
>>                             _("a secret with UUID %s is already defined for "
>>                               "use with %s"),
>> -                           uuidstr, secret->def->usage_id);
>> +                           uuidstr, obj->def->usage_id);
>>              goto cleanup;
>>          }
>>  
>> -        if (secret->def->isprivate && !def->isprivate) {
>> +        if (obj->def->isprivate && !newdef->isprivate) {
>>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>                             _("cannot change private flag on existing secret"));
>>              goto cleanup;
>>          }
>>  
>>          if (oldDef)
>> -            *oldDef = secret->def;
>> +            *oldDef = obj->def;
>>          else
>> -            virSecretDefFree(secret->def);
>> -        secret->def = def;
>> +            virSecretDefFree(obj->def);
>> +        obj->def = newdef;
>>      } else {
>>          /* No existing secret with same UUID,
>>           * try look for matching usage instead */
>> -        if ((secret = virSecretObjListFindByUsageLocked(secrets,
>> -                                                        def->usage_type,
>> -                                                        def->usage_id))) {
>> -            virObjectLock(secret);
>> -            virUUIDFormat(secret->def->uuid, uuidstr);
>> +        if ((obj = virSecretObjListFindByUsageLocked(secrets,
>> +                                                     newdef->usage_type,
>> +                                                     newdef->usage_id))) {
>> +            virObjectLock(obj);
>> +            virUUIDFormat(obj->def->uuid, uuidstr);
>>              virReportError(VIR_ERR_INTERNAL_ERROR,
>>                             _("a secret with UUID %s already defined for "
>>                               "use with %s"),
>> -                           uuidstr, def->usage_id);
>> +                           uuidstr, newdef->usage_id);
>>              goto cleanup;
>>          }
>>  
>>          /* Generate the possible configFile and base64File strings
>>           * using the configDir, uuidstr, and appropriate suffix
>>           */
>> -        virUUIDFormat(def->uuid, uuidstr);
>> +        virUUIDFormat(newdef->uuid, uuidstr);
>>          if (!(configFile = virFileBuildPath(configDir, uuidstr, ".xml")) ||
>>              !(base64File = virFileBuildPath(configDir, uuidstr, ".base64")))
>>              goto cleanup;
>>  
>> -        if (!(secret = virSecretObjNew()))
>> +        if (!(obj = virSecretObjNew()))
>>              goto cleanup;
>>  
>> -        if (virHashAddEntry(secrets->objs, uuidstr, secret) < 0)
>> +        if (virHashAddEntry(secrets->objs, uuidstr, obj) < 0)
>>              goto cleanup;
>>  
>> -        secret->def = def;
>> -        secret->configFile = configFile;
>> -        secret->base64File = base64File;
>> -        configFile = NULL;
>> -        base64File = NULL;
>> -        virObjectRef(secret);
>> +        obj->def = newdef;
>> +        VIR_STEAL_PTR(obj->configFile, configFile);
>> +        VIR_STEAL_PTR(obj->base64File, base64File);
>> +        virObjectRef(obj);
>>      }
>>  
>> -    ret = secret;
>> -    secret = NULL;
>> +    ret = obj;
>> +    obj = NULL;
>>  
>>   cleanup:
>> -    virSecretObjEndAPI(&secret);
>> +    virSecretObjEndAPI(&obj);
>>      VIR_FREE(configFile);
>>      VIR_FREE(base64File);
>>      return ret;
>> @@ -393,16 +394,16 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
>>  
>>  virSecretObjPtr
>>  virSecretObjListAdd(virSecretObjListPtr secrets,
>> -                    virSecretDefPtr def,
>> +                    virSecretDefPtr newdef,
>>                      const char *configDir,
>>                      virSecretDefPtr *oldDef)
>>  {
>> -    virSecretObjPtr ret;
>> +    virSecretObjPtr obj;
>>  
>>      virObjectLock(secrets);
>> -    ret = virSecretObjListAddLocked(secrets, def, configDir, oldDef);
>> +    obj = virSecretObjListAddLocked(secrets, newdef, configDir, oldDef);
>>      virObjectUnlock(secrets);
>> -    return ret;
>> +    return obj;
>>  }
>>  
>>  
>> @@ -474,23 +475,23 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
>>  
>>  #define MATCH(FLAG) (flags & (FLAG))
>>  static bool
>> -virSecretObjMatchFlags(virSecretObjPtr secret,
>> +virSecretObjMatchFlags(virSecretObjPtr obj,
>>                         unsigned int flags)
>>  {
>>      /* filter by whether it's ephemeral */
>>      if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL) &&
>>          !((MATCH(VIR_CONNECT_LIST_SECRETS_EPHEMERAL) &&
>> -           secret->def->isephemeral) ||
>> +           obj->def->isephemeral) ||
>>            (MATCH(VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL) &&
>> -           !secret->def->isephemeral)))
>> +           !obj->def->isephemeral)))
>>          return false;
>>  
>>      /* filter by whether it's private */
>>      if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE) &&
>>          !((MATCH(VIR_CONNECT_LIST_SECRETS_PRIVATE) &&
>> -           secret->def->isprivate) ||
>> +           obj->def->isprivate) ||
>>            (MATCH(VIR_CONNECT_LIST_SECRETS_NO_PRIVATE) &&
>> -           !secret->def->isprivate)))
>> +           !obj->def->isprivate)))
>>          return false;
>>  
>>      return true;
>> @@ -621,12 +622,12 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
>>  
>>  
>>  int
>> -virSecretObjDeleteConfig(virSecretObjPtr secret)
>> +virSecretObjDeleteConfig(virSecretObjPtr obj)
>>  {
>> -    if (!secret->def->isephemeral &&
>> -        unlink(secret->configFile) < 0 && errno != ENOENT) {
>> +    if (!obj->def->isephemeral &&
>> +        unlink(obj->configFile) < 0 && errno != ENOENT) {
>>          virReportSystemError(errno, _("cannot unlink '%s'"),
>> -                             secret->configFile);
>> +                             obj->configFile);
>>          return -1;
>>      }
>>  
>> @@ -635,11 +636,11 @@ virSecretObjDeleteConfig(virSecretObjPtr secret)
>>  
>>  
>>  void
>> -virSecretObjDeleteData(virSecretObjPtr secret)
>> +virSecretObjDeleteData(virSecretObjPtr obj)
>>  {
>>      /* The configFile will already be removed, so secret won't be
>>       * loaded again if this fails */
>> -    (void)unlink(secret->base64File);
>> +    (void)unlink(obj->base64File);
>>  }
>>  
>>  
>> @@ -650,15 +651,15 @@ virSecretObjDeleteData(virSecretObjPtr secret)
>>     secret is defined, it is stored as base64 (with no formatting) in
>>     "$basename.base64".  "$basename" is in both cases the base64-encoded UUID. */
>>  int
>> -virSecretObjSaveConfig(virSecretObjPtr secret)
>> +virSecretObjSaveConfig(virSecretObjPtr obj)
>>  {
>>      char *xml = NULL;
>>      int ret = -1;
>>  
>> -    if (!(xml = virSecretDefFormat(secret->def)))
>> +    if (!(xml = virSecretDefFormat(obj->def)))
>>          goto cleanup;
>>  
>> -    if (virFileRewriteStr(secret->configFile, S_IRUSR | S_IWUSR, xml) < 0)
>> +    if (virFileRewriteStr(obj->configFile, S_IRUSR | S_IWUSR, xml) < 0)
>>          goto cleanup;
>>  
>>      ret = 0;
>> @@ -670,18 +671,18 @@ virSecretObjSaveConfig(virSecretObjPtr secret)
>>  
>>  
>>  int
>> -virSecretObjSaveData(virSecretObjPtr secret)
>> +virSecretObjSaveData(virSecretObjPtr obj)
>>  {
>>      char *base64 = NULL;
>>      int ret = -1;
>>  
>> -    if (!secret->value)
>> +    if (!obj->value)
>>          return 0;
>>  
>> -    if (!(base64 = virStringEncodeBase64(secret->value, secret->value_size)))
>> +    if (!(base64 = virStringEncodeBase64(obj->value, obj->value_size)))
>>          goto cleanup;
>>  
>> -    if (virFileRewriteStr(secret->base64File, S_IRUSR | S_IWUSR, base64) < 0)
>> +    if (virFileRewriteStr(obj->base64File, S_IRUSR | S_IWUSR, base64) < 0)
>>          goto cleanup;
>>  
>>      ret = 0;
>> @@ -693,36 +694,36 @@ virSecretObjSaveData(virSecretObjPtr secret)
>>  
>>  
>>  virSecretDefPtr
>> -virSecretObjGetDef(virSecretObjPtr secret)
>> +virSecretObjGetDef(virSecretObjPtr obj)
>>  {
>> -    return secret->def;
>> +    return obj->def;
>>  }
>>  
>>  
>>  void
>> -virSecretObjSetDef(virSecretObjPtr secret,
>> +virSecretObjSetDef(virSecretObjPtr obj,
>>                     virSecretDefPtr def)
>>  {
>> -    secret->def = def;
>> +    obj->def = def;
>>  }
>>  
>>  
>>  unsigned char *
>> -virSecretObjGetValue(virSecretObjPtr secret)
>> +virSecretObjGetValue(virSecretObjPtr obj)
>>  {
>>      unsigned char *ret = NULL;
>>  
>> -    if (!secret->value) {
>> +    if (!obj->value) {
>>          char uuidstr[VIR_UUID_STRING_BUFLEN];
>> -        virUUIDFormat(secret->def->uuid, uuidstr);
>> +        virUUIDFormat(obj->def->uuid, uuidstr);
>>          virReportError(VIR_ERR_NO_SECRET,
>>                         _("secret '%s' does not have a value"), uuidstr);
>>          goto cleanup;
>>      }
>>  
>> -    if (VIR_ALLOC_N(ret, secret->value_size) < 0)
>> +    if (VIR_ALLOC_N(ret, obj->value_size) < 0)
>>          goto cleanup;
>> -    memcpy(ret, secret->value, secret->value_size);
>> +    memcpy(ret, obj->value, obj->value_size);
>>  
>>   cleanup:
>>      return ret;
>> @@ -730,7 +731,7 @@ virSecretObjGetValue(virSecretObjPtr secret)
>>  
>>  
>>  int
>> -virSecretObjSetValue(virSecretObjPtr secret,
>> +virSecretObjSetValue(virSecretObjPtr obj,
>>                       const unsigned char *value,
>>                       size_t value_size)
>>  {
>> @@ -740,14 +741,14 @@ virSecretObjSetValue(virSecretObjPtr secret,
>>      if (VIR_ALLOC_N(new_value, value_size) < 0)
>>          return -1;
>>  
>> -    old_value = secret->value;
>> -    old_value_size = secret->value_size;
>> +    old_value = obj->value;
>> +    old_value_size = obj->value_size;
>>  
>>      memcpy(new_value, value, value_size);
>> -    secret->value = new_value;
>> -    secret->value_size = value_size;
>> +    obj->value = new_value;
>> +    obj->value_size = value_size;
>>  
>> -    if (!secret->def->isephemeral && virSecretObjSaveData(secret) < 0)
>> +    if (!obj->def->isephemeral && virSecretObjSaveData(obj) < 0)
>>          goto error;
>>  
>>      /* Saved successfully - drop old value */
>> @@ -760,8 +761,8 @@ virSecretObjSetValue(virSecretObjPtr secret,
>>  
>>   error:
>>      /* Error - restore previous state and free new value */
>> -    secret->value = old_value;
>> -    secret->value_size = old_value_size;
>> +    obj->value = old_value;
>> +    obj->value_size = old_value_size;
>>      memset(new_value, 0, value_size);
>>      VIR_FREE(new_value);
>>      return -1;
>> @@ -769,17 +770,17 @@ virSecretObjSetValue(virSecretObjPtr secret,
>>  
>>  
>>  size_t
>> -virSecretObjGetValueSize(virSecretObjPtr secret)
>> +virSecretObjGetValueSize(virSecretObjPtr obj)
>>  {
>> -    return secret->value_size;
>> +    return obj->value_size;
>>  }
>>  
>>  
>>  void
>> -virSecretObjSetValueSize(virSecretObjPtr secret,
>> +virSecretObjSetValueSize(virSecretObjPtr obj,
>>                           size_t value_size)
>>  {
>> -    secret->value_size = value_size;
>> +    obj->value_size = value_size;
>>  }
>>  
>>  
>> @@ -803,33 +804,33 @@ virSecretLoadValidateUUID(virSecretDefPtr def,
>>  
>>  
>>  static int
>> -virSecretLoadValue(virSecretObjPtr secret)
>> +virSecretLoadValue(virSecretObjPtr obj)
>>  {
>>      int ret = -1, fd = -1;
>>      struct stat st;
>>      char *contents = NULL, *value = NULL;
>>      size_t value_size;
>>  
>> -    if ((fd = open(secret->base64File, O_RDONLY)) == -1) {
>> +    if ((fd = open(obj->base64File, O_RDONLY)) == -1) {
>>          if (errno == ENOENT) {
>>              ret = 0;
>>              goto cleanup;
>>          }
>>          virReportSystemError(errno, _("cannot open '%s'"),
>> -                             secret->base64File);
>> +                             obj->base64File);
>>          goto cleanup;
>>      }
>>  
>>      if (fstat(fd, &st) < 0) {
>>          virReportSystemError(errno, _("cannot stat '%s'"),
>> -                             secret->base64File);
>> +                             obj->base64File);
>>          goto cleanup;
>>      }
>>  
>>      if ((size_t)st.st_size != st.st_size) {
>>          virReportError(VIR_ERR_INTERNAL_ERROR,
>>                         _("'%s' file does not fit in memory"),
>> -                       secret->base64File);
>> +                       obj->base64File);
>>          goto cleanup;
>>      }
>>  
>> @@ -838,7 +839,7 @@ virSecretLoadValue(virSecretObjPtr secret)
>>  
>>      if (saferead(fd, contents, st.st_size) != st.st_size) {
>>          virReportSystemError(errno, _("cannot read '%s'"),
>> -                             secret->base64File);
>> +                             obj->base64File);
>>          goto cleanup;
>>      }
>>  
>> @@ -847,15 +848,15 @@ virSecretLoadValue(virSecretObjPtr secret)
>>      if (!base64_decode_alloc(contents, st.st_size, &value, &value_size)) {
>>          virReportError(VIR_ERR_INTERNAL_ERROR,
>>                         _("invalid base64 in '%s'"),
>> -                       secret->base64File);
>> +                       obj->base64File);
>>          goto cleanup;
>>      }
>>      if (value == NULL)
>>          goto cleanup;
>>  
>> -    secret->value = (unsigned char *)value;
>> +    obj->value = (unsigned char *)value;
>>      value = NULL;
>> -    secret->value_size = value_size;
>> +    obj->value_size = value_size;
>>  
>>      ret = 0;
>>  
>> @@ -880,7 +881,8 @@ virSecretLoad(virSecretObjListPtr secrets,
>>                const char *configDir)
>>  {
>>      virSecretDefPtr def = NULL;
>> -    virSecretObjPtr secret = NULL, ret = NULL;
>> +    virSecretObjPtr obj = NULL;
>> +    virSecretObjPtr ret = NULL;
>>  
>>      if (!(def = virSecretDefParseFile(path)))
>>          goto cleanup;
>> @@ -888,19 +890,18 @@ virSecretLoad(virSecretObjListPtr secrets,
>>      if (virSecretLoadValidateUUID(def, file) < 0)
>>          goto cleanup;
>>  
>> -    if (!(secret = virSecretObjListAdd(secrets, def, configDir, NULL)))
>> +    if (!(obj = virSecretObjListAdd(secrets, def, configDir, NULL)))
>>          goto cleanup;
>>      def = NULL;
>>  
>> -    if (virSecretLoadValue(secret) < 0)
>> +    if (virSecretLoadValue(obj) < 0)
>>          goto cleanup;
>>  
>> -    ret = secret;
>> -    secret = NULL;
>> +    ret = obj;
>> +    obj = NULL;
>>  
>>   cleanup:
>> -    if (secret)
>> -        virSecretObjListRemove(secrets, secret);
>> +    virSecretObjListRemove(secrets, obj);
>>      virSecretDefFree(def);
>>      return ret;
>>  }
>> @@ -921,7 +922,7 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
>>       * loop (if any).  It's better to keep the secrets we managed to find. */
>>      while (virDirRead(dir, &de, NULL) > 0) {
>>          char *path;
>> -        virSecretObjPtr secret;
>> +        virSecretObjPtr obj;
>>  
>>          if (!virFileHasSuffix(de->d_name, ".xml"))
>>              continue;
>> @@ -929,7 +930,7 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
>>          if (!(path = virFileBuildPath(configDir, de->d_name, NULL)))
>>              continue;
>>  
>> -        if (!(secret = virSecretLoad(secrets, de->d_name, path, configDir))) {
>> +        if (!(obj = virSecretLoad(secrets, de->d_name, path, configDir))) {
>>              VIR_ERROR(_("Error reading secret: %s"),
>>                        virGetLastErrorMessage());
>>              VIR_FREE(path);
>> @@ -937,7 +938,7 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
>>          }
>>  
>>          VIR_FREE(path);
>> -        virSecretObjEndAPI(&secret);
>> +        virSecretObjEndAPI(&obj);
>>      }
>>  
>>      VIR_DIR_CLOSE(dir);
>> diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
>> index 9638b69..8038faa 100644
>> --- a/src/conf/virsecretobj.h
>> +++ b/src/conf/virsecretobj.h
>> @@ -30,7 +30,7 @@ typedef struct _virSecretObj virSecretObj;
>>  typedef virSecretObj *virSecretObjPtr;
>>  
>>  void
>> -virSecretObjEndAPI(virSecretObjPtr *secret);
>> +virSecretObjEndAPI(virSecretObjPtr *obj);
>>  
>>  typedef struct _virSecretObjList virSecretObjList;
>>  typedef virSecretObjList *virSecretObjListPtr;
>> @@ -49,11 +49,11 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
>>  
>>  void
>>  virSecretObjListRemove(virSecretObjListPtr secrets,
>> -                       virSecretObjPtr secret);
>> +                       virSecretObjPtr obj);
>>  
>>  virSecretObjPtr
>>  virSecretObjListAdd(virSecretObjListPtr secrets,
>> -                    virSecretDefPtr def,
>> +                    virSecretDefPtr newdef,
>>                      const char *configDir,
>>                      virSecretDefPtr *oldDef);
>>  
>> @@ -81,37 +81,37 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
>>                           virConnectPtr conn);
>>  
>>  int
>> -virSecretObjDeleteConfig(virSecretObjPtr secret);
>> +virSecretObjDeleteConfig(virSecretObjPtr obj);
>>  
>>  void
>> -virSecretObjDeleteData(virSecretObjPtr secret);
>> +virSecretObjDeleteData(virSecretObjPtr obj);
>>  
>>  int
>> -virSecretObjSaveConfig(virSecretObjPtr secret);
>> +virSecretObjSaveConfig(virSecretObjPtr obj);
>>  
>>  int
>> -virSecretObjSaveData(virSecretObjPtr secret);
>> +virSecretObjSaveData(virSecretObjPtr obj);
>>  
>>  virSecretDefPtr
>> -virSecretObjGetDef(virSecretObjPtr secret);
>> +virSecretObjGetDef(virSecretObjPtr obj);
>>  
>>  void
>> -virSecretObjSetDef(virSecretObjPtr secret,
>> +virSecretObjSetDef(virSecretObjPtr obj,
>>                     virSecretDefPtr def);
>>  
>>  unsigned char *
>> -virSecretObjGetValue(virSecretObjPtr secret);
>> +virSecretObjGetValue(virSecretObjPtr obj);
>>  
>>  int
>> -virSecretObjSetValue(virSecretObjPtr secret,
>> +virSecretObjSetValue(virSecretObjPtr obj,
>>                       const unsigned char *value,
>>                       size_t value_size);
>>  
>>  size_t
>> -virSecretObjGetValueSize(virSecretObjPtr secret);
>> +virSecretObjGetValueSize(virSecretObjPtr obj);
>>  
>>  void
>> -virSecretObjSetValueSize(virSecretObjPtr secret,
>> +virSecretObjSetValueSize(virSecretObjPtr obj,
>>                           size_t value_size);
>>  
>>  int
>> diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
>> index 2a371b6..cc050ff 100644
>> --- a/src/secret/secret_driver.c
>> +++ b/src/secret/secret_driver.c
>> @@ -72,6 +72,7 @@ secretDriverLock(void)
>>      virMutexLock(&driver->lock);
>>  }
>>  
>> +
>>  static void
>>  secretDriverUnlock(void)
>>  {
>> @@ -79,7 +80,6 @@ secretDriverUnlock(void)
>>  }
>>  
>>  
>> -
>>  static virSecretObjPtr
>>  secretObjFromSecret(virSecretPtr secret)
>>  {
>> @@ -120,6 +120,7 @@ secretConnectNumOfSecrets(virConnectPtr conn)
>>                                          conn);
>>  }
>>  
>> +
>>  static int
>>  secretConnectListSecrets(virConnectPtr conn,
>>                           char **uuids,
>> @@ -156,10 +157,10 @@ secretLookupByUUID(virConnectPtr conn,
>>                     const unsigned char *uuid)
>>  {
>>      virSecretPtr ret = NULL;
>> -    virSecretObjPtr secret;
>> +    virSecretObjPtr obj;
>>      virSecretDefPtr def;
>>  
>> -    if (!(secret = virSecretObjListFindByUUID(driver->secrets, uuid))) {
>> +    if (!(obj = virSecretObjListFindByUUID(driver->secrets, uuid))) {
>>          char uuidstr[VIR_UUID_STRING_BUFLEN];
>>          virUUIDFormat(uuid, uuidstr);
>>          virReportError(VIR_ERR_NO_SECRET,
>> @@ -167,7 +168,7 @@ secretLookupByUUID(virConnectPtr conn,
>>          goto cleanup;
>>      }
>>  
>> -    def = virSecretObjGetDef(secret);
>> +    def = virSecretObjGetDef(obj);
>>      if (virSecretLookupByUUIDEnsureACL(conn, def) < 0)
>>          goto cleanup;
>>  
>> @@ -177,7 +178,7 @@ secretLookupByUUID(virConnectPtr conn,
>>                         def->usage_id);
>>  
>>   cleanup:
>> -    virSecretObjEndAPI(&secret);
>> +    virSecretObjEndAPI(&obj);
>>      return ret;
>>  }
>>  
>> @@ -188,17 +189,17 @@ secretLookupByUsage(virConnectPtr conn,
>>                      const char *usageID)
>>  {
>>      virSecretPtr ret = NULL;
>> -    virSecretObjPtr secret;
>> +    virSecretObjPtr obj;
>>      virSecretDefPtr def;
>>  
>> -    if (!(secret = virSecretObjListFindByUsage(driver->secrets,
>> -                                               usageType, usageID))) {
>> +    if (!(obj = virSecretObjListFindByUsage(driver->secrets,
>> +                                            usageType, usageID))) {
>>          virReportError(VIR_ERR_NO_SECRET,
>>                         _("no secret with matching usage '%s'"), usageID);
>>          goto cleanup;
>>      }
>>  
>> -    def = virSecretObjGetDef(secret);
>> +    def = virSecretObjGetDef(obj);
>>      if (virSecretLookupByUsageEnsureACL(conn, def) < 0)
>>          goto cleanup;
>>  
>> @@ -208,7 +209,7 @@ secretLookupByUsage(virConnectPtr conn,
>>                         def->usage_id);
>>  
>>   cleanup:
>> -    virSecretObjEndAPI(&secret);
>> +    virSecretObjEndAPI(&obj);
>>      return ret;
>>  }
>>  
>> @@ -219,129 +220,131 @@ secretDefineXML(virConnectPtr conn,
>>                  unsigned int flags)
>>  {
>>      virSecretPtr ret = NULL;
>> -    virSecretObjPtr secret = NULL;
>> +    virSecretObjPtr obj = NULL;
>>      virSecretDefPtr backup = NULL;
>> -    virSecretDefPtr new_attrs;
>> +    virSecretDefPtr def;
>>      virObjectEventPtr event = NULL;
>>  
>>      virCheckFlags(0, NULL);
>>  
>> -    if (!(new_attrs = virSecretDefParseString(xml)))
>> +    if (!(def = virSecretDefParseString(xml)))
>>          return NULL;
>>  
>> -    if (virSecretDefineXMLEnsureACL(conn, new_attrs) < 0)
>> +    if (virSecretDefineXMLEnsureACL(conn, def) < 0)
>>          goto cleanup;
>>  
>> -    if (!(secret = virSecretObjListAdd(driver->secrets, new_attrs,
>> -                                       driver->configDir, &backup)))
>> +    if (!(obj = virSecretObjListAdd(driver->secrets, def,
>> +                                    driver->configDir, &backup)))
>>          goto cleanup;
>>  
>> -    if (!new_attrs->isephemeral) {
>> +    if (!def->isephemeral) {
>>          if (secretEnsureDirectory() < 0)
>>              goto cleanup;
>>  
>>          if (backup && backup->isephemeral) {
>> -            if (virSecretObjSaveData(secret) < 0)
>> +            if (virSecretObjSaveData(obj) < 0)
>>                  goto restore_backup;
>>          }
>>  
>> -        if (virSecretObjSaveConfig(secret) < 0) {
>> +        if (virSecretObjSaveConfig(obj) < 0) {
>>              if (backup && backup->isephemeral) {
>>                  /* Undo the virSecretObjSaveData() above; ignore errors */
>> -                virSecretObjDeleteData(secret);
>> +                virSecretObjDeleteData(obj);
>>              }
>>              goto restore_backup;
>>          }
>>      } else if (backup && !backup->isephemeral) {
>> -        if (virSecretObjDeleteConfig(secret) < 0)
>> +        if (virSecretObjDeleteConfig(obj) < 0)
>>              goto restore_backup;
>>  
>> -        virSecretObjDeleteData(secret);
>> +        virSecretObjDeleteData(obj);
>>      }
>>      /* Saved successfully - drop old values */
>>      virSecretDefFree(backup);
>>  
>> -    event = virSecretEventLifecycleNew(new_attrs->uuid,
>> -                                       new_attrs->usage_type,
>> -                                       new_attrs->usage_id,
>> +    event = virSecretEventLifecycleNew(def->uuid,
>> +                                       def->usage_type,
>> +                                       def->usage_id,
>>                                         VIR_SECRET_EVENT_DEFINED,
>>                                         0);
>>  
>>      ret = virGetSecret(conn,
>> -                       new_attrs->uuid,
>> -                       new_attrs->usage_type,
>> -                       new_attrs->usage_id);
>> -    new_attrs = NULL;
>> +                       def->uuid,
>> +                       def->usage_type,
>> +                       def->usage_id);
>> +    def = NULL;
>>      goto cleanup;
>>  
>>   restore_backup:
>>      /* If we have a backup, then secret was defined before, so just restore
>> -     * the backup. The current (new_attrs) will be handled below.
>> +     * the backup. The current def will be handled below.
>>       * Otherwise, this is a new secret, thus remove it.
>>       */
>>      if (backup)
>> -        virSecretObjSetDef(secret, backup);
>> +        virSecretObjSetDef(obj, backup);
>>      else
>> -        virSecretObjListRemove(driver->secrets, secret);
>> +        virSecretObjListRemove(driver->secrets, obj);
>>  
>>   cleanup:
>> -    virSecretDefFree(new_attrs);
>> -    virSecretObjEndAPI(&secret);
>> +    virSecretDefFree(def);
>> +    virSecretObjEndAPI(&obj);
>>      if (event)
>>          virObjectEventStateQueue(driver->secretEventState, event);
>>  
>>      return ret;
>>  }
>>  
>> +
>>  static char *
>> -secretGetXMLDesc(virSecretPtr obj,
>> +secretGetXMLDesc(virSecretPtr secret,
>>                   unsigned int flags)
>>  {
>>      char *ret = NULL;
>> -    virSecretObjPtr secret;
>> +    virSecretObjPtr obj;
>>      virSecretDefPtr def;
>>  
>>      virCheckFlags(0, NULL);
>>  
>> -    if (!(secret = secretObjFromSecret(obj)))
>> +    if (!(obj = secretObjFromSecret(secret)))
>>          goto cleanup;
>>  
>> -    def = virSecretObjGetDef(secret);
>> -    if (virSecretGetXMLDescEnsureACL(obj->conn, def) < 0)
>> +    def = virSecretObjGetDef(obj);
>> +    if (virSecretGetXMLDescEnsureACL(secret->conn, def) < 0)
>>          goto cleanup;
>>  
>>      ret = virSecretDefFormat(def);
>>  
>>   cleanup:
>> -    virSecretObjEndAPI(&secret);
>> +    virSecretObjEndAPI(&obj);
>>  
>>      return ret;
>>  }
>>  
>> +
>>  static int
>> -secretSetValue(virSecretPtr obj,
>> +secretSetValue(virSecretPtr secret,
>>                 const unsigned char *value,
>>                 size_t value_size,
>>                 unsigned int flags)
>>  {
>>      int ret = -1;
>> -    virSecretObjPtr secret;
>> +    virSecretObjPtr obj;
>>      virSecretDefPtr def;
>>      virObjectEventPtr event = NULL;
>>  
>>      virCheckFlags(0, -1);
>>  
>> -    if (!(secret = secretObjFromSecret(obj)))
>> +    if (!(obj = secretObjFromSecret(secret)))
>>          goto cleanup;
>>  
>> -    def = virSecretObjGetDef(secret);
>> -    if (virSecretSetValueEnsureACL(obj->conn, def) < 0)
>> +    def = virSecretObjGetDef(obj);
>> +    if (virSecretSetValueEnsureACL(secret->conn, def) < 0)
>>          goto cleanup;
>>  
>>      if (secretEnsureDirectory() < 0)
>>          goto cleanup;
>>  
>> -    if (virSecretObjSetValue(secret, value, value_size) < 0)
>> +    if (virSecretObjSetValue(obj, value, value_size) < 0)
>>          goto cleanup;
>>  
>>      event = virSecretEventValueChangedNew(def->uuid,
>> @@ -350,30 +353,31 @@ secretSetValue(virSecretPtr obj,
>>      ret = 0;
>>  
>>   cleanup:
>> -    virSecretObjEndAPI(&secret);
>> +    virSecretObjEndAPI(&obj);
>>      if (event)
>>          virObjectEventStateQueue(driver->secretEventState, event);
>>  
>>      return ret;
>>  }
>>  
>> +
>>  static unsigned char *
>> -secretGetValue(virSecretPtr obj,
>> +secretGetValue(virSecretPtr secret,
>>                 size_t *value_size,
>>                 unsigned int flags,
>>                 unsigned int internalFlags)
>>  {
>>      unsigned char *ret = NULL;
>> -    virSecretObjPtr secret;
>> +    virSecretObjPtr obj;
>>      virSecretDefPtr def;
>>  
>>      virCheckFlags(0, NULL);
>>  
>> -    if (!(secret = secretObjFromSecret(obj)))
>> +    if (!(obj = secretObjFromSecret(secret)))
>>          goto cleanup;
>>  
>> -    def = virSecretObjGetDef(secret);
>> -    if (virSecretGetValueEnsureACL(obj->conn, def) < 0)
>> +    def = virSecretObjGetDef(obj);
>> +    if (virSecretGetValueEnsureACL(secret->conn, def) < 0)
>>          goto cleanup;
>>  
>>      if ((internalFlags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 &&
>> @@ -383,33 +387,34 @@ secretGetValue(virSecretPtr obj,
>>          goto cleanup;
>>      }
>>  
>> -    if (!(ret = virSecretObjGetValue(secret)))
>> +    if (!(ret = virSecretObjGetValue(obj)))
>>          goto cleanup;
>>  
>> -    *value_size = virSecretObjGetValueSize(secret);
>> +    *value_size = virSecretObjGetValueSize(obj);
>>  
>>   cleanup:
>> -    virSecretObjEndAPI(&secret);
>> +    virSecretObjEndAPI(&obj);
>>  
>>      return ret;
>>  }
>>  
>> +
>>  static int
>> -secretUndefine(virSecretPtr obj)
>> +secretUndefine(virSecretPtr secret)
>>  {
>>      int ret = -1;
>> -    virSecretObjPtr secret;
>> +    virSecretObjPtr obj;
>>      virSecretDefPtr def;
>>      virObjectEventPtr event = NULL;
>>  
>> -    if (!(secret = secretObjFromSecret(obj)))
>> +    if (!(obj = secretObjFromSecret(secret)))
>>          goto cleanup;
>>  
>> -    def = virSecretObjGetDef(secret);
>> -    if (virSecretUndefineEnsureACL(obj->conn, def) < 0)
>> +    def = virSecretObjGetDef(obj);
>> +    if (virSecretUndefineEnsureACL(secret->conn, def) < 0)
>>          goto cleanup;
>>  
>> -    if (virSecretObjDeleteConfig(secret) < 0)
>> +    if (virSecretObjDeleteConfig(obj) < 0)
>>          goto cleanup;
>>  
>>      event = virSecretEventLifecycleNew(def->uuid,
>> @@ -418,20 +423,21 @@ secretUndefine(virSecretPtr obj)
>>                                         VIR_SECRET_EVENT_UNDEFINED,
>>                                         0);
>>  
>> -    virSecretObjDeleteData(secret);
>> +    virSecretObjDeleteData(obj);
>>  
>> -    virSecretObjListRemove(driver->secrets, secret);
>> +    virSecretObjListRemove(driver->secrets, obj);
>>  
>>      ret = 0;
>>  
>>   cleanup:
>> -    virSecretObjEndAPI(&secret);
>> +    virSecretObjEndAPI(&obj);
>>      if (event)
>>          virObjectEventStateQueue(driver->secretEventState, event);
>>  
>>      return ret;
>>  }
>>  
>> +
>>  static int
>>  secretStateCleanup(void)
>>  {
>> @@ -452,6 +458,7 @@ secretStateCleanup(void)
>>      return 0;
>>  }
>>  
>> +
>>  static int
>>  secretStateInitialize(bool privileged,
>>                        virStateInhibitCallback callback ATTRIBUTE_UNUSED,
>> @@ -497,6 +504,7 @@ secretStateInitialize(bool privileged,
>>      return -1;
>>  }
>>  
>> +
>>  static int
>>  secretStateReload(void)
>>  {
>> @@ -511,6 +519,7 @@ secretStateReload(void)
>>      return 0;
>>  }
>>  
>> +
>>  static int
>>  secretConnectSecretEventRegisterAny(virConnectPtr conn,
>>                                      virSecretPtr secret,
>> @@ -532,6 +541,7 @@ secretConnectSecretEventRegisterAny(virConnectPtr conn,
>>      return callbackID;
>>  }
>>  
>> +
>>  static int
>>  secretConnectSecretEventDeregisterAny(virConnectPtr conn,
>>                                        int callbackID)
>> @@ -576,6 +586,7 @@ static virStateDriver stateDriver = {
>>      .stateReload = secretStateReload,
>>  };
>>  
>> +
>>  int
>>  secretRegister(void)
>>  {
>> -- 
>> 2.9.3
>>
>> --
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list




More information about the libvir-list mailing list