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

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@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;

[...]