[libvirt] [PATCH 5/5] util: Replace virStorageSourceFree with virObjectUnref

[John Ferlan]

On 2/15/19 7:42 AM, Peter Krempa wrote:
> Now that virStorageSource is a subclass of virObject we can use
> virObjectUnref and remove virStorageSourceFree which was a thin wrapper.
> 
> Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> ---
>  src/conf/domain_conf.c     |  8 ++++----
>  src/conf/snapshot_conf.c   |  2 +-
>  src/libvirt_private.syms   |  1 -
>  src/qemu/qemu_blockjob.c   | 10 +++++-----
>  src/qemu/qemu_domain.c     |  2 +-
>  src/qemu/qemu_driver.c     |  6 +++---
>  src/qemu/qemu_hotplug.c    |  2 +-
>  src/qemu/qemu_migration.c  |  2 +-
>  src/storage/storage_util.c |  2 +-
>  src/util/virstoragefile.c  | 35 ++++++++++++++---------------------
>  src/util/virstoragefile.h  |  1 -
>  tests/virstoragetest.c     |  4 ++--
>  12 files changed, 33 insertions(+), 42 deletions(-)
> 

[...]

> diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
> index 56c6510c5e..9dd27b5ca6 100644
> --- a/src/util/virstoragefile.c
> +++ b/src/util/virstoragefile.c
> @@ -1133,7 +1133,7 @@ virStorageFileMetadataNew(const char *path,
>      return def;
> 
>   error:
> -    virStorageSourceFree(def);
> +    virObjectUnref(def);

Is this right?  @def is VIR_ALLOC'd and not virStorageSourceNew alloc'd.
Also anything that calls this would have the same problem I would think.

FWIW: This module is the one I ran afoul of the MinGW builds when trying
to use VIR_AUTOPTR. I see no change in here to use VIR_AUTOUNREF, so I
assume this was the way around it, albeit without the object alloc.

John

>      return NULL;
>  }
> 
> @@ -1158,7 +1158,7 @@ virStorageFileMetadataNew(const char *path,
>   * image didn't specify an explicit format for its backing store. Callers are
>   * advised against probing for the backing store format in this case.
>   *
> - * Caller MUST free the result after use via virStorageSourceFree.
> + * Caller MUST free the result after use via virObjectUnref.
>   */
>  virStorageSourcePtr
>  virStorageFileGetMetadataFromBuf(const char *path,
> @@ -1178,7 +1178,7 @@ virStorageFileGetMetadataFromBuf(const char *path,
> 
>      if (virStorageFileGetMetadataInternal(ret, buf, len,
>                                            backingFormat) < 0) {
> -        virStorageSourceFree(ret);
> +        virObjectUnref(ret);
>          return NULL;
>      }
> 
> @@ -1197,7 +1197,7 @@ virStorageFileGetMetadataFromBuf(const char *path,
>   * format, since a malicious guest can turn a raw file into any
>   * other non-raw format at will.
>   *
> - * Caller MUST free the result after use via virStorageSourceFree.
> + * Caller MUST free the result after use via virObjectUnref.
>   */
>  virStorageSourcePtr
>  virStorageFileGetMetadataFromFD(const char *path,
> @@ -1257,7 +1257,7 @@ virStorageFileGetMetadataFromFD(const char *path,
>      VIR_STEAL_PTR(ret, meta);
> 
>   cleanup:
> -    virStorageSourceFree(meta);
> +    virObjectUnref(meta);
>      return ret;
>  }
> 
> @@ -2336,7 +2336,7 @@ virStorageSourceCopy(const virStorageSource *src,
>      return def;
> 
>   error:
> -    virStorageSourceFree(def);
> +    virObjectUnref(def);
>      return NULL;
>  }
> 
> @@ -2522,7 +2522,7 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr def)
>      VIR_FREE(def->backingStoreRaw);
> 
>      /* recursively free backing chain */
> -    virStorageSourceFree(def->backingStore);
> +    virObjectUnref(def->backingStore);
>      def->backingStore = NULL;
>  }
> 
> @@ -2598,13 +2598,6 @@ virStorageSourceNew(void)
>  }
> 
> 
> -void
> -virStorageSourceFree(virStorageSourcePtr def)
> -{
> -    virObjectUnref(def);
> -}
> -
> -
>  static virStorageSourcePtr
>  virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
>                                         const char *rel)
> @@ -2656,7 +2649,7 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
>      return def;
> 
>   error:
> -    virStorageSourceFree(def);
> +    virObjectUnref(def);
>      def = NULL;
>      goto cleanup;
>  }
> @@ -3697,7 +3690,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
>      return def;
> 
>   error:
> -    virStorageSourceFree(def);
> +    virObjectUnref(def);
>      return NULL;
>  }
> 
> @@ -3738,7 +3731,7 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent)
>      return def;
> 
>   error:
> -    virStorageSourceFree(def);
> +    virObjectUnref(def);
>      return NULL;
>  }
> 
> @@ -3919,7 +3912,7 @@ virStorageSourceUpdateCapacity(virStorageSourcePtr src,
>      ret = 0;
> 
>   cleanup:
> -    virStorageSourceFree(meta);
> +    virObjectUnref(meta);
>      return ret;
>  }
> 
> @@ -4943,7 +4936,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
>      if (virStorageSourceHasBacking(src))
>          src->backingStore->id = depth;
>      virStorageFileDeinit(src);
> -    virStorageSourceFree(backingStore);
> +    virObjectUnref(backingStore);
>      return ret;
>  }
> 
> @@ -4966,7 +4959,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
>   * If @report_broken is true, the whole function fails with a possibly sane
>   * error instead of just returning a broken chain.
>   *
> - * Caller MUST free result after use via virStorageSourceFree.
> + * Caller MUST free result after use via virObjectUnref.
>   */
>  int
>  virStorageFileGetMetadata(virStorageSourcePtr src,
> @@ -5050,7 +5043,7 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src,
>      ret = 0;
> 
>   cleanup:
> -    virStorageSourceFree(tmp);
> +    virObjectUnref(tmp);
> 
>      return ret;

[...]