Re: [PATCH 06/14] secret: Use virSecretDefPtr rather than deref from virSecretObjPtr

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 24, 2017 at 02:00:15PM -0400, John Ferlan wrote:
> Rather than dereferencing obj->def->X, create a local 'def' variable
> variable that will dereference the def and use directly.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  src/conf/virsecretobj.c | 69 +++++++++++++++++++++++++++++++------------------
>  1 file changed, 44 insertions(+), 25 deletions(-)
> 
> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
> index 42f36c8..413955d 100644
> --- a/src/conf/virsecretobj.c
> +++ b/src/conf/virsecretobj.c
> @@ -139,8 +139,9 @@ static void
>  virSecretObjDispose(void *opaque)
>  {
>      virSecretObjPtr obj = opaque;
> +    virSecretDefPtr def = obj->def;
>  
> -    virSecretDefFree(obj->def);
> +    virSecretDefFree(def);

Here it adds only a noise into the code, no actual improvement.

>      if (obj->value) {
>          /* Wipe before free to ensure we don't leave a secret on the heap */
>          memset(obj->value, 0, obj->value_size);
> @@ -203,16 +204,18 @@ virSecretObjSearchName(const void *payload,
>                         const void *opaque)
>  {
>      virSecretObjPtr obj = (virSecretObjPtr) payload;
> +    virSecretDefPtr def;
>      struct virSecretSearchData *data = (struct virSecretSearchData *) opaque;
>      int found = 0;
>  
>      virObjectLock(obj);
> +    def = obj->def;
>  
> -    if (obj->def->usage_type != data->usageType)
> +    if (def->usage_type != data->usageType)
>          goto cleanup;
>  
>      if (data->usageType != VIR_SECRET_USAGE_TYPE_NONE &&
> -        STREQ(obj->def->usage_id, data->usageID))
> +        STREQ(def->usage_id, data->usageID))
>          found = 1;
>  
>   cleanup:
> @@ -278,11 +281,13 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
>                         virSecretObjPtr obj)
>  {
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
> +    virSecretDefPtr def;
>  
>      if (!obj)
>          return;
> +    def = obj->def;
>  
> -    virUUIDFormat(obj->def->uuid, uuidstr);
> +    virUUIDFormat(def->uuid, uuidstr);

Same here,

>      virObjectRef(obj);
>      virObjectUnlock(obj);
>  
> @@ -315,6 +320,7 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
>                            virSecretDefPtr *oldDef)
>  {
>      virSecretObjPtr obj;
> +    virSecretDefPtr def;
>      virSecretObjPtr ret = NULL;
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>      char *configFile = NULL, *base64File = NULL;
> @@ -325,26 +331,27 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
>      /* Is there a secret already matching this UUID */
>      if ((obj = virSecretObjListFindByUUIDLocked(secrets, newdef->uuid))) {
>          virObjectLock(obj);
> +        def = obj->def;
>  
> -        if (STRNEQ_NULLABLE(obj->def->usage_id, newdef->usage_id)) {
> -            virUUIDFormat(obj->def->uuid, uuidstr);
> +        if (STRNEQ_NULLABLE(def->usage_id, newdef->usage_id)) {
> +            virUUIDFormat(def->uuid, uuidstr);
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("a secret with UUID %s is already defined for "
>                               "use with %s"),
> -                           uuidstr, obj->def->usage_id);
> +                           uuidstr, def->usage_id);
>              goto cleanup;
>          }
>  
> -        if (obj->def->isprivate && !newdef->isprivate) {
> +        if (def->isprivate && !newdef->isprivate) {
>              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                             _("cannot change private flag on existing secret"));
>              goto cleanup;
>          }
>  
>          if (oldDef)
> -            *oldDef = obj->def;
> +            *oldDef = def;
>          else
> -            virSecretDefFree(obj->def);
> +            virSecretDefFree(def);
>          obj->def = newdef;
>      } else {
>          /* No existing secret with same UUID,
> @@ -353,7 +360,8 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
>                                                       newdef->usage_type,
>                                                       newdef->usage_id))) {
>              virObjectLock(obj);
> -            virUUIDFormat(obj->def->uuid, uuidstr);
> +            def = obj->def;
> +            virUUIDFormat(def->uuid, uuidstr);
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("a secret with UUID %s already defined for "
>                               "use with %s"),
> @@ -424,6 +432,7 @@ virSecretObjListGetHelper(void *payload,
>  {
>      struct virSecretObjListGetHelperData *data = opaque;
>      virSecretObjPtr obj = payload;
> +    virSecretDefPtr def;
>  
>      if (data->error)
>          return 0;
> @@ -432,8 +441,9 @@ virSecretObjListGetHelper(void *payload,
>          return 0;
>  
>      virObjectLock(obj);
> +    def = obj->def;
>  
> -    if (data->filter && !data->filter(data->conn, obj->def))
> +    if (data->filter && !data->filter(data->conn, def))
>          goto cleanup;
>  
>      if (data->uuids) {
> @@ -444,7 +454,7 @@ virSecretObjListGetHelper(void *payload,
>              goto cleanup;
>          }
>  
> -        virUUIDFormat(obj->def->uuid, uuidstr);
> +        virUUIDFormat(def->uuid, uuidstr);
>          data->uuids[data->got] = uuidstr;
>      }
>  
> @@ -478,20 +488,22 @@ static bool
>  virSecretObjMatchFlags(virSecretObjPtr obj,
>                         unsigned int flags)
>  {
> +    virSecretDefPtr def = obj->def;
> +
>      /* filter by whether it's ephemeral */
>      if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL) &&
>          !((MATCH(VIR_CONNECT_LIST_SECRETS_EPHEMERAL) &&
> -           obj->def->isephemeral) ||
> +           def->isephemeral) ||
>            (MATCH(VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL) &&
> -           !obj->def->isephemeral)))
> +           !def->isephemeral)))
>          return false;
>  
>      /* filter by whether it's private */
>      if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE) &&
>          !((MATCH(VIR_CONNECT_LIST_SECRETS_PRIVATE) &&
> -           obj->def->isprivate) ||
> +           def->isprivate) ||
>            (MATCH(VIR_CONNECT_LIST_SECRETS_NO_PRIVATE) &&
> -           !obj->def->isprivate)))
> +           !def->isprivate)))
>          return false;
>  
>      return true;
> @@ -515,14 +527,16 @@ virSecretObjListPopulate(void *payload,
>  {
>      struct virSecretObjListData *data = opaque;
>      virSecretObjPtr obj = payload;
> +    virSecretDefPtr def;
>      virSecretPtr secret = NULL;
>  
>      if (data->error)
>          return 0;
>  
>      virObjectLock(obj);
> +    def = obj->def;
>  
> -    if (data->filter && !data->filter(data->conn, obj->def))
> +    if (data->filter && !data->filter(data->conn, def))
>          goto cleanup;
>  
>      if (!virSecretObjMatchFlags(obj, data->flags))
> @@ -533,9 +547,9 @@ virSecretObjListPopulate(void *payload,
>          goto cleanup;
>      }
>  
> -    if (!(secret = virGetSecret(data->conn, obj->def->uuid,
> -                                obj->def->usage_type,
> -                                obj->def->usage_id))) {
> +    if (!(secret = virGetSecret(data->conn, def->uuid,
> +                                def->usage_type,
> +                                def->usage_id))) {
>          data->error = true;
>          goto cleanup;
>      }
> @@ -624,7 +638,9 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
>  int
>  virSecretObjDeleteConfig(virSecretObjPtr obj)
>  {
> -    if (!obj->def->isephemeral &&
> +    virSecretDefPtr def = obj->def;
> +
> +    if (!def->isephemeral &&

here,

>          unlink(obj->configFile) < 0 && errno != ENOENT) {
>          virReportSystemError(errno, _("cannot unlink '%s'"),
>                               obj->configFile);
> @@ -653,10 +669,11 @@ virSecretObjDeleteData(virSecretObjPtr obj)
>  int
>  virSecretObjSaveConfig(virSecretObjPtr obj)
>  {
> +    virSecretDefPtr def = obj->def;
>      char *xml = NULL;
>      int ret = -1;
>  
> -    if (!(xml = virSecretDefFormat(obj->def)))
> +    if (!(xml = virSecretDefFormat(def)))
>          goto cleanup;

here,

>      if (virFileRewriteStr(obj->configFile, S_IRUSR | S_IWUSR, xml) < 0)
> @@ -711,11 +728,12 @@ virSecretObjSetDef(virSecretObjPtr obj,
>  unsigned char *
>  virSecretObjGetValue(virSecretObjPtr obj)
>  {
> +    virSecretDefPtr def = obj->def;
>      unsigned char *ret = NULL;
>  
>      if (!obj->value) {
>          char uuidstr[VIR_UUID_STRING_BUFLEN];
> -        virUUIDFormat(obj->def->uuid, uuidstr);
> +        virUUIDFormat(def->uuid, uuidstr);

here,
>          virReportError(VIR_ERR_NO_SECRET,
>                         _("secret '%s' does not have a value"), uuidstr);
>          goto cleanup;
> @@ -735,6 +753,7 @@ virSecretObjSetValue(virSecretObjPtr obj,
>                       const unsigned char *value,
>                       size_t value_size)
>  {
> +    virSecretDefPtr def = obj->def;
>      unsigned char *old_value, *new_value;
>      size_t old_value_size;
>  
> @@ -748,7 +767,7 @@ virSecretObjSetValue(virSecretObjPtr obj,
>      obj->value = new_value;
>      obj->value_size = value_size;
>  
> -    if (!obj->def->isephemeral && virSecretObjSaveData(obj) < 0)
> +    if (!def->isephemeral && virSecretObjSaveData(obj) < 0)
>          goto error;

and here.

Pavel

Attachment: signature.asc
Description: Digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux