[libvirt] [PATCH 15/15] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageSource

John Ferlan jferlan at redhat.com
Wed Feb 6 13:41:47 UTC 2019


Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths. A few methods were modified to use a
more common methodology of defining/using @def and then stealing
the pointer to @ret to return allowing @def to be autofree'd if
the swap didn't occur on various return NULL; error paths.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/domain_conf.c                |   3 +-
 src/qemu/qemu_domain.c                |   3 +-
 src/qemu/qemu_driver.c                |   9 +-
 src/qemu/qemu_migration.c             |   3 +-
 src/storage/storage_backend_gluster.c |   3 +-
 src/storage/storage_util.c            |  31 +--
 src/util/virstoragefile.c             | 292 ++++++++++++--------------
 src/util/virstoragefile.h             |   1 +
 tests/qemublocktest.c                 |   6 +-
 tests/virstoragetest.c                |  60 +++---
 10 files changed, 179 insertions(+), 232 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ee0edff4b2..3af900da7f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9059,7 +9059,7 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
                                unsigned int flags,
                                virDomainXMLOptionPtr xmlopt)
 {
-    virStorageSourcePtr backingStore = NULL;
+    VIR_AUTOPTR(virStorageSource) backingStore = NULL;
     xmlNodePtr save_ctxt = ctxt->node;
     xmlNodePtr source;
     char *type = NULL;
@@ -9126,7 +9126,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
     ret = 0;
 
  cleanup:
-    virStorageSourceFree(backingStore);
     VIR_FREE(type);
     VIR_FREE(format);
     VIR_FREE(idx);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b6c1a0e4e5..b4acf72bf2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2730,7 +2730,7 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node,
 {
     xmlNodePtr savedNode = ctxt->node;
     qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
-    virStorageSourcePtr migrSource = NULL;
+    VIR_AUTOPTR(virStorageSource) migrSource = NULL;
     char *format = NULL;
     char *type = NULL;
     int ret = -1;
@@ -2781,7 +2781,6 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node,
     ret = 0;
 
  cleanup:
-    virStorageSourceFree(migrSource);
     VIR_FREE(format);
     VIR_FREE(type);
     ctxt->node = savedNode;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 427c1d02a8..88dd7846dc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -274,7 +274,7 @@ qemuSecurityChownCallback(const virStorageSource *src,
                           uid_t uid,
                           gid_t gid)
 {
-    virStorageSourcePtr cpy = NULL;
+    VIR_AUTOPTR(virStorageSource) cpy = NULL;
     struct stat sb;
     int save_errno = 0;
     int ret = -1;
@@ -319,7 +319,6 @@ qemuSecurityChownCallback(const virStorageSource *src,
  cleanup:
     save_errno = errno;
     virStorageFileDeinit(cpy);
-    virStorageSourceFree(cpy);
     errno = save_errno;
 
     return ret;
@@ -17888,7 +17887,7 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
     virDomainObjPtr vm;
     int ret = -1;
     unsigned long long speed = bandwidth;
-    virStorageSourcePtr dest = NULL;
+    VIR_AUTOPTR(virStorageSource) dest = NULL;
 
     virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
                   VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
@@ -17950,7 +17949,6 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
 
  cleanup:
     virDomainObjEndAPI(&vm);
-    virStorageSourceFree(dest);
     return ret;
 }
 
@@ -18080,7 +18078,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
     char *topPath = NULL;
     char *basePath = NULL;
     char *backingPath = NULL;
-    virStorageSourcePtr mirror = NULL;
+    VIR_AUTOPTR(virStorageSource) mirror = NULL;
     unsigned long long speed = bandwidth;
     qemuBlockJobDataPtr job = NULL;
     qemuBlockJobType jobtype = QEMU_BLOCKJOB_TYPE_COMMIT;
@@ -18282,7 +18280,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
             virFreeError(orig_err);
         }
     }
-    virStorageSourceFree(mirror);
     qemuBlockJobStartupFinalize(job);
     qemuDomainObjEndJob(driver, vm);
 
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 1433b2c2f3..6a680ba2e8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -788,7 +788,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver,
 {
     qemuBlockStorageSourceAttachDataPtr data = NULL;
     qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
-    virStorageSourcePtr copysrc = NULL;
+    VIR_AUTOPTR(virStorageSource) copysrc = NULL;
     int mon_ret = 0;
     int ret = -1;
 
@@ -849,7 +849,6 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver,
 
  cleanup:
     qemuBlockStorageSourceAttachDataFree(data);
-    virStorageSourceFree(copysrc);
     return ret;
 }
 
diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
index 0fe548f7e0..cb86db22a8 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -237,8 +237,8 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
     int ret = -1;
     VIR_AUTOPTR(virStorageVolDef) vol = NULL;
     VIR_AUTOFREE(char *) header = NULL;
+    VIR_AUTOPTR(virStorageSource) meta = NULL;
     glfs_fd_t *fd = NULL;
-    virStorageSourcePtr meta = NULL;
     ssize_t len;
     int backingFormat;
 
@@ -323,7 +323,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
     VIR_STEAL_PTR(*volptr, vol);
     ret = 0;
  cleanup:
-    virStorageSourceFree(meta);
     if (fd)
         glfs_close(fd);
     return ret;
diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c
index 345566a3af..d138379a81 100644
--- a/src/storage/storage_util.c
+++ b/src/storage/storage_util.c
@@ -3354,9 +3354,8 @@ storageBackendProbeTarget(virStorageSourcePtr target,
 {
     int backingStoreFormat;
     VIR_AUTOCLOSE fd = -1;
-    int ret = -1;
     int rc;
-    virStorageSourcePtr meta = NULL;
+    VIR_AUTOPTR(virStorageSource) meta = NULL;
     struct stat sb;
 
     if (encryption)
@@ -3368,17 +3367,16 @@ storageBackendProbeTarget(virStorageSourcePtr target,
     fd = rc;
 
     if (virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb) < 0)
-        goto cleanup;
+        return -1;
 
     if (S_ISDIR(sb.st_mode)) {
         if (storageBackendIsPloopDir(target->path)) {
             if (storageBackendRedoPloopUpdate(target, &sb, &fd,
                                               VIR_STORAGE_VOL_FS_PROBE_FLAGS) < 0)
-                goto cleanup;
+                return -1;
         } else {
             target->format = VIR_STORAGE_FILE_DIR;
-            ret = 0;
-            goto cleanup;
+            return 0;
         }
     }
 
@@ -3386,11 +3384,11 @@ storageBackendProbeTarget(virStorageSourcePtr target,
                                                  fd,
                                                  VIR_STORAGE_FILE_AUTO,
                                                  &backingStoreFormat)))
-        goto cleanup;
+        return -1;
 
     if (meta->backingStoreRaw) {
         if (!(target->backingStore = virStorageSourceNewFromBacking(meta)))
-            goto cleanup;
+            return -1;
 
         target->backingStore->format = backingStoreFormat;
 
@@ -3401,7 +3399,7 @@ storageBackendProbeTarget(virStorageSourcePtr target,
             virStorageSourceFree(target->backingStore);
 
             if (VIR_ALLOC(target->backingStore) < 0)
-                goto cleanup;
+                return -1;
 
             target->backingStore->type = VIR_STORAGE_TYPE_NETWORK;
             target->backingStore->path = meta->backingStoreRaw;
@@ -3430,8 +3428,6 @@ storageBackendProbeTarget(virStorageSourcePtr target,
     target->format = meta->format;
 
     /* Default to success below this point */
-    ret = 0;
-
     if (meta->capacity)
         target->capacity = meta->capacity;
 
@@ -3450,18 +3446,14 @@ storageBackendProbeTarget(virStorageSourcePtr target,
     }
 
     virBitmapFree(target->features);
-    target->features = meta->features;
-    meta->features = NULL;
+    VIR_STEAL_PTR(target->features, meta->features);
 
     if (meta->compat) {
         VIR_FREE(target->compat);
-        target->compat = meta->compat;
-        meta->compat = NULL;
+        VIR_STEAL_PTR(target->compat, meta->compat);
     }
 
- cleanup:
-    virStorageSourceFree(meta);
-    return ret;
+    return 0;
 }
 
 
@@ -3531,7 +3523,7 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
     struct stat statbuf;
     VIR_AUTOPTR(virStorageVolDef) vol = NULL;
     VIR_AUTOCLOSE fd = -1;
-    virStorageSourcePtr target = NULL;
+    VIR_AUTOPTR(virStorageSource) target = NULL;
     int direrr;
     int ret = -1;
 
@@ -3624,7 +3616,6 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
     ret = 0;
  cleanup:
     VIR_DIR_CLOSE(dir);
-    virStorageSourceFree(target);
     if (ret < 0)
         virStoragePoolObjClearVols(pool);
     return ret;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 1d77f6ac4c..5a4b63acc4 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1117,22 +1117,20 @@ static virStorageSourcePtr
 virStorageFileMetadataNew(const char *path,
                           int format)
 {
+    VIR_AUTOPTR(virStorageSource) def = NULL;
     virStorageSourcePtr ret = NULL;
 
-    if (VIR_ALLOC(ret) < 0)
+    if (VIR_ALLOC(def) < 0)
         return NULL;
 
-    ret->format = format;
-    ret->type = VIR_STORAGE_TYPE_FILE;
+    def->format = format;
+    def->type = VIR_STORAGE_TYPE_FILE;
 
-    if (VIR_STRDUP(ret->path, path) < 0)
-        goto error;
+    if (VIR_STRDUP(def->path, path) < 0)
+        return NULL;
 
+    VIR_STEAL_PTR(ret, def);
     return ret;
-
- error:
-    virStorageSourceFree(ret);
-    return NULL;
 }
 
 
@@ -1205,7 +1203,7 @@ virStorageFileGetMetadataFromFD(const char *path,
 
 {
     virStorageSourcePtr ret = NULL;
-    virStorageSourcePtr meta = NULL;
+    VIR_AUTOPTR(virStorageSource) meta = NULL;
     VIR_AUTOFREE(char *) buf = NULL;
     ssize_t len = VIR_STORAGE_MAX_HEADER;
     struct stat sb;
@@ -1231,21 +1229,21 @@ virStorageFileGetMetadataFromFD(const char *path,
         meta->type = VIR_STORAGE_TYPE_DIR;
         meta->format = VIR_STORAGE_FILE_DIR;
         VIR_STEAL_PTR(ret, meta);
-        goto cleanup;
+        return ret;
     }
 
     if (lseek(fd, 0, SEEK_SET) == (off_t)-1) {
         virReportSystemError(errno, _("cannot seek to start of '%s'"), meta->path);
-        goto cleanup;
+        return NULL;
     }
 
     if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) {
         virReportSystemError(errno, _("cannot read header '%s'"), meta->path);
-        goto cleanup;
+        return NULL;
     }
 
     if (virStorageFileGetMetadataInternal(meta, buf, len, backingFormat) < 0)
-        goto cleanup;
+        return NULL;
 
     if (S_ISREG(sb.st_mode))
         meta->type = VIR_STORAGE_TYPE_FILE;
@@ -1253,9 +1251,6 @@ virStorageFileGetMetadataFromFD(const char *path,
         meta->type = VIR_STORAGE_TYPE_BLOCK;
 
     VIR_STEAL_PTR(ret, meta);
-
- cleanup:
-    virStorageSourceFree(meta);
     return ret;
 }
 
@@ -2243,99 +2238,97 @@ virStorageSourcePtr
 virStorageSourceCopy(const virStorageSource *src,
                      bool backingChain)
 {
+    VIR_AUTOPTR(virStorageSource) def = NULL;
     virStorageSourcePtr ret = NULL;
 
-    if (VIR_ALLOC(ret) < 0)
+    if (VIR_ALLOC(def) < 0)
         return NULL;
 
-    ret->id = src->id;
-    ret->type = src->type;
-    ret->protocol = src->protocol;
-    ret->format = src->format;
-    ret->capacity = src->capacity;
-    ret->allocation = src->allocation;
-    ret->has_allocation = src->has_allocation;
-    ret->physical = src->physical;
-    ret->readonly = src->readonly;
-    ret->shared = src->shared;
-    ret->haveTLS = src->haveTLS;
-    ret->tlsFromConfig = src->tlsFromConfig;
-    ret->detected = src->detected;
-    ret->debugLevel = src->debugLevel;
-    ret->debug = src->debug;
-    ret->iomode = src->iomode;
-    ret->cachemode = src->cachemode;
-    ret->discard = src->discard;
-    ret->detect_zeroes = src->detect_zeroes;
+    def->id = src->id;
+    def->type = src->type;
+    def->protocol = src->protocol;
+    def->format = src->format;
+    def->capacity = src->capacity;
+    def->allocation = src->allocation;
+    def->has_allocation = src->has_allocation;
+    def->physical = src->physical;
+    def->readonly = src->readonly;
+    def->shared = src->shared;
+    def->haveTLS = src->haveTLS;
+    def->tlsFromConfig = src->tlsFromConfig;
+    def->detected = src->detected;
+    def->debugLevel = src->debugLevel;
+    def->debug = src->debug;
+    def->iomode = src->iomode;
+    def->cachemode = src->cachemode;
+    def->discard = src->discard;
+    def->detect_zeroes = src->detect_zeroes;
 
     /* storage driver metadata are not copied */
-    ret->drv = NULL;
-
-    if (VIR_STRDUP(ret->path, src->path) < 0 ||
-        VIR_STRDUP(ret->volume, src->volume) < 0 ||
-        VIR_STRDUP(ret->relPath, src->relPath) < 0 ||
-        VIR_STRDUP(ret->backingStoreRaw, src->backingStoreRaw) < 0 ||
-        VIR_STRDUP(ret->snapshot, src->snapshot) < 0 ||
-        VIR_STRDUP(ret->configFile, src->configFile) < 0 ||
-        VIR_STRDUP(ret->nodeformat, src->nodeformat) < 0 ||
-        VIR_STRDUP(ret->nodestorage, src->nodestorage) < 0 ||
-        VIR_STRDUP(ret->compat, src->compat) < 0 ||
-        VIR_STRDUP(ret->tlsAlias, src->tlsAlias) < 0 ||
-        VIR_STRDUP(ret->tlsCertdir, src->tlsCertdir) < 0)
-        goto error;
+    def->drv = NULL;
+
+    if (VIR_STRDUP(def->path, src->path) < 0 ||
+        VIR_STRDUP(def->volume, src->volume) < 0 ||
+        VIR_STRDUP(def->relPath, src->relPath) < 0 ||
+        VIR_STRDUP(def->backingStoreRaw, src->backingStoreRaw) < 0 ||
+        VIR_STRDUP(def->snapshot, src->snapshot) < 0 ||
+        VIR_STRDUP(def->configFile, src->configFile) < 0 ||
+        VIR_STRDUP(def->nodeformat, src->nodeformat) < 0 ||
+        VIR_STRDUP(def->nodestorage, src->nodestorage) < 0 ||
+        VIR_STRDUP(def->compat, src->compat) < 0 ||
+        VIR_STRDUP(def->tlsAlias, src->tlsAlias) < 0 ||
+        VIR_STRDUP(def->tlsCertdir, src->tlsCertdir) < 0)
+        return NULL;
 
     if (src->nhosts) {
-        if (!(ret->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts)))
-            goto error;
+        if (!(def->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts)))
+            return NULL;
 
-        ret->nhosts = src->nhosts;
+        def->nhosts = src->nhosts;
     }
 
     if (src->srcpool &&
-        !(ret->srcpool = virStorageSourcePoolDefCopy(src->srcpool)))
-        goto error;
+        !(def->srcpool = virStorageSourcePoolDefCopy(src->srcpool)))
+        return NULL;
 
     if (src->features &&
-        !(ret->features = virBitmapNewCopy(src->features)))
-        goto error;
+        !(def->features = virBitmapNewCopy(src->features)))
+        return NULL;
 
     if (src->encryption &&
-        !(ret->encryption = virStorageEncryptionCopy(src->encryption)))
-        goto error;
+        !(def->encryption = virStorageEncryptionCopy(src->encryption)))
+        return NULL;
 
     if (src->perms &&
-        !(ret->perms = virStoragePermsCopy(src->perms)))
-        goto error;
+        !(def->perms = virStoragePermsCopy(src->perms)))
+        return NULL;
 
     if (src->timestamps &&
-        !(ret->timestamps = virStorageTimestampsCopy(src->timestamps)))
-        goto error;
+        !(def->timestamps = virStorageTimestampsCopy(src->timestamps)))
+        return NULL;
 
-    if (virStorageSourceSeclabelsCopy(ret, src) < 0)
-        goto error;
+    if (virStorageSourceSeclabelsCopy(def, src) < 0)
+        return NULL;
 
     if (src->auth &&
-        !(ret->auth = virStorageAuthDefCopy(src->auth)))
-        goto error;
+        !(def->auth = virStorageAuthDefCopy(src->auth)))
+        return NULL;
 
     if (src->pr &&
-        !(ret->pr = virStoragePRDefCopy(src->pr)))
-        goto error;
+        !(def->pr = virStoragePRDefCopy(src->pr)))
+        return NULL;
 
-    if (virStorageSourceInitiatorCopy(&ret->initiator, &src->initiator))
-        goto error;
+    if (virStorageSourceInitiatorCopy(&def->initiator, &src->initiator))
+        return NULL;
 
     if (backingChain && src->backingStore) {
-        if (!(ret->backingStore = virStorageSourceCopy(src->backingStore,
+        if (!(def->backingStore = virStorageSourceCopy(src->backingStore,
                                                        true)))
-            goto error;
+            return NULL;
     }
 
+    VIR_STEAL_PTR(ret, def);
     return ret;
-
- error:
-    virStorageSourceFree(ret);
-    return NULL;
 }
 
 
@@ -2578,55 +2571,51 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
                                        const char *rel)
 {
     VIR_AUTOFREE(char *) dirname = NULL;
-    virStorageSourcePtr ret;
+    VIR_AUTOPTR(virStorageSource) def = NULL;
+    virStorageSourcePtr ret = NULL;
 
-    if (VIR_ALLOC(ret) < 0)
+    if (VIR_ALLOC(def) < 0)
         return NULL;
 
     /* store relative name */
-    if (VIR_STRDUP(ret->relPath, parent->backingStoreRaw) < 0)
-        goto error;
+    if (VIR_STRDUP(def->relPath, parent->backingStoreRaw) < 0)
+        return NULL;
 
     if (!(dirname = mdir_name(parent->path))) {
         virReportOOMError();
-        goto error;
+        return NULL;
     }
 
     if (STRNEQ(dirname, "/")) {
-        if (virAsprintf(&ret->path, "%s/%s", dirname, rel) < 0)
-            goto error;
+        if (virAsprintf(&def->path, "%s/%s", dirname, rel) < 0)
+            return NULL;
     } else {
-        if (virAsprintf(&ret->path, "/%s", rel) < 0)
-            goto error;
+        if (virAsprintf(&def->path, "/%s", rel) < 0)
+            return NULL;
     }
 
     if (virStorageSourceGetActualType(parent) == VIR_STORAGE_TYPE_NETWORK) {
-        ret->type = VIR_STORAGE_TYPE_NETWORK;
+        def->type = VIR_STORAGE_TYPE_NETWORK;
 
         /* copy the host network part */
-        ret->protocol = parent->protocol;
+        def->protocol = parent->protocol;
         if (parent->nhosts) {
-            if (!(ret->hosts = virStorageNetHostDefCopy(parent->nhosts,
+            if (!(def->hosts = virStorageNetHostDefCopy(parent->nhosts,
                                                         parent->hosts)))
-                goto error;
+                return NULL;
 
-            ret->nhosts = parent->nhosts;
+            def->nhosts = parent->nhosts;
         }
 
-        if (VIR_STRDUP(ret->volume, parent->volume) < 0)
-            goto error;
+        if (VIR_STRDUP(def->volume, parent->volume) < 0)
+            return NULL;
     } else {
         /* set the type to _FILE, the caller shall update it to the actual type */
-        ret->type = VIR_STORAGE_TYPE_FILE;
+        def->type = VIR_STORAGE_TYPE_FILE;
     }
 
- cleanup:
+    VIR_STEAL_PTR(ret, def);
     return ret;
-
- error:
-    virStorageSourceFree(ret);
-    ret = NULL;
-    goto cleanup;
 }
 
 
@@ -3619,49 +3608,47 @@ virStorageSourcePtr
 virStorageSourceNewFromBackingAbsolute(const char *path)
 {
     const char *json;
-    virStorageSourcePtr ret;
+    VIR_AUTOPTR(virStorageSource) def = NULL;
+    virStorageSourcePtr ret = NULL;
     int rc;
 
-    if (VIR_ALLOC(ret) < 0)
+    if (VIR_ALLOC(def) < 0)
         return NULL;
 
     if (virStorageIsFile(path)) {
-        ret->type = VIR_STORAGE_TYPE_FILE;
+        def->type = VIR_STORAGE_TYPE_FILE;
 
-        if (VIR_STRDUP(ret->path, path) < 0)
-            goto error;
+        if (VIR_STRDUP(def->path, path) < 0)
+            return NULL;
     } else {
-        ret->type = VIR_STORAGE_TYPE_NETWORK;
+        def->type = VIR_STORAGE_TYPE_NETWORK;
 
         VIR_DEBUG("parsing backing store string: '%s'", path);
 
         /* handle URI formatted backing stores */
         if ((json = STRSKIP(path, "json:")))
-            rc = virStorageSourceParseBackingJSON(ret, json);
+            rc = virStorageSourceParseBackingJSON(def, json);
         else if (strstr(path, "://"))
-            rc = virStorageSourceParseBackingURI(ret, path);
+            rc = virStorageSourceParseBackingURI(def, path);
         else
-            rc = virStorageSourceParseBackingColon(ret, path);
+            rc = virStorageSourceParseBackingColon(def, path);
 
         if (rc < 0)
-            goto error;
+            return NULL;
 
-        virStorageSourceNetworkAssignDefaultPorts(ret);
+        virStorageSourceNetworkAssignDefaultPorts(def);
 
         /* Some of the legacy parsers parse authentication data since they are
          * also used in other places. For backing store detection the
          * authentication data would be invalid anyways, so we clear it */
-        if (ret->auth) {
-            virStorageAuthDefFree(ret->auth);
-            ret->auth = NULL;
+        if (def->auth) {
+            virStorageAuthDefFree(def->auth);
+            def->auth = NULL;
         }
     }
 
+    VIR_STEAL_PTR(ret, def);
     return ret;
-
- error:
-    virStorageSourceFree(ret);
-    return NULL;
 }
 
 
@@ -3669,40 +3656,38 @@ virStorageSourcePtr
 virStorageSourceNewFromBacking(virStorageSourcePtr parent)
 {
     struct stat st;
-    virStorageSourcePtr ret;
+    VIR_AUTOPTR(virStorageSource) def = NULL;
+    virStorageSourcePtr ret = NULL;
 
     if (virStorageIsRelative(parent->backingStoreRaw))
-        ret = virStorageSourceNewFromBackingRelative(parent,
+        def = virStorageSourceNewFromBackingRelative(parent,
                                                      parent->backingStoreRaw);
     else
-        ret = virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw);
+        def = virStorageSourceNewFromBackingAbsolute(parent->backingStoreRaw);
 
-    if (ret) {
+    if (def) {
         /* possibly update local type */
-        if (ret->type == VIR_STORAGE_TYPE_FILE) {
-            if (stat(ret->path, &st) == 0) {
+        if (def->type == VIR_STORAGE_TYPE_FILE) {
+            if (stat(def->path, &st) == 0) {
                 if (S_ISDIR(st.st_mode)) {
-                    ret->type = VIR_STORAGE_TYPE_DIR;
-                    ret->format = VIR_STORAGE_FILE_DIR;
+                    def->type = VIR_STORAGE_TYPE_DIR;
+                    def->format = VIR_STORAGE_FILE_DIR;
                 } else if (S_ISBLK(st.st_mode)) {
-                    ret->type = VIR_STORAGE_TYPE_BLOCK;
+                    def->type = VIR_STORAGE_TYPE_BLOCK;
                 }
             }
         }
 
         /* copy parent's labelling and other top level stuff */
-        if (virStorageSourceInitChainElement(ret, parent, true) < 0)
-            goto error;
+        if (virStorageSourceInitChainElement(def, parent, true) < 0)
+            return NULL;
 
-        ret->readonly = true;
-        ret->detected = true;
+        def->readonly = true;
+        def->detected = true;
     }
 
+    VIR_STEAL_PTR(ret, def);
     return ret;
-
- error:
-    virStorageSourceFree(ret);
-    return NULL;
 }
 
 
@@ -3843,8 +3828,7 @@ virStorageSourceUpdateCapacity(virStorageSourcePtr src,
                                ssize_t len,
                                bool probe)
 {
-    int ret = -1;
-    virStorageSourcePtr meta = NULL;
+    VIR_AUTOPTR(virStorageSource) meta = NULL;
     int format = src->format;
 
     /* Raw files: capacity is physical size.  For all other files: if
@@ -3855,12 +3839,12 @@ virStorageSourceUpdateCapacity(virStorageSourcePtr src,
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("no disk format for %s and probing is disabled"),
                            src->path);
-            goto cleanup;
+            return -1;
         }
 
         if ((format = virStorageFileProbeFormatFromBuf(src->path,
                                                        buf, len)) < 0)
-            goto cleanup;
+            return -1;
 
         src->format = format;
     }
@@ -3873,17 +3857,13 @@ virStorageSourceUpdateCapacity(virStorageSourcePtr src,
         if (src->encryption && meta->encryption)
             src->encryption->payload_offset = meta->encryption->payload_offset;
     } else {
-        goto cleanup;
+        return -1;
     }
 
     if (src->encryption && src->encryption->payload_offset != -1)
         src->capacity -= src->encryption->payload_offset * 512;
 
-    ret = 0;
-
- cleanup:
-    virStorageSourceFree(meta);
-    return ret;
+    return 0;
 }
 
 
@@ -4821,8 +4801,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
     int ret = -1;
     const char *uniqueName;
     VIR_AUTOFREE(char *) buf = NULL;
+    VIR_AUTOPTR(virStorageSource) backingStore = NULL;
     ssize_t headerLen;
-    virStorageSourcePtr backingStore = NULL;
     int backingFormat;
     int rv;
 
@@ -4900,15 +4880,13 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
             goto cleanup;
     }
 
-    src->backingStore = backingStore;
-    backingStore = NULL;
+    VIR_STEAL_PTR(src->backingStore, backingStore);
     ret = 0;
 
  cleanup:
     if (virStorageSourceHasBacking(src))
         src->backingStore->id = depth;
     virStorageFileDeinit(src);
-    virStorageSourceFree(backingStore);
     return ret;
 }
 
@@ -4977,10 +4955,9 @@ int
 virStorageFileGetBackingStoreStr(virStorageSourcePtr src,
                                  char **backing)
 {
-    virStorageSourcePtr tmp = NULL;
+    VIR_AUTOPTR(virStorageSource) tmp = NULL;
     VIR_AUTOFREE(char *) buf = NULL;
     ssize_t headerLen;
-    int ret = -1;
     int rv;
 
     *backing = NULL;
@@ -5005,17 +4982,12 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src,
     }
 
     if (!(tmp = virStorageSourceCopy(src, false)))
-        goto cleanup;
+        return -1;
 
     if (virStorageFileGetMetadataInternal(tmp, buf, headerLen, NULL) < 0)
-        goto cleanup;
+        return -1;
 
     VIR_STEAL_PTR(*backing, tmp->backingStoreRaw);
 
-    ret = 0;
-
- cleanup:
-    virStorageSourceFree(tmp);
-
-    return ret;
+    return 0;
 }
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index f1164dde9b..6da8c2fdf9 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -544,4 +544,5 @@ void virStorageFileReportBrokenChain(int errcode,
                                      virStorageSourcePtr parent);
 
 VIR_DEFINE_AUTOPTR_FUNC(virStorageAuthDef, virStorageAuthDefFree);
+VIR_DEFINE_AUTOPTR_FUNC(virStorageSource, virStorageSourceFree);
 #endif /* LIBVIRT_VIRSTORAGEFILE_H */
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 5848f6b5b5..0171289dd9 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -46,8 +46,8 @@ testBackingXMLjsonXML(const void *args)
     xmlDocPtr xml = NULL;
     xmlXPathContextPtr ctxt = NULL;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
-    virStorageSourcePtr xmlsrc = NULL;
-    virStorageSourcePtr jsonsrc = NULL;
+    VIR_AUTOPTR(virStorageSource) xmlsrc = NULL;
+    VIR_AUTOPTR(virStorageSource) jsonsrc = NULL;
     virJSONValuePtr backendprops = NULL;
     virJSONValuePtr wrapper = NULL;
     char *propsstr = NULL;
@@ -104,8 +104,6 @@ testBackingXMLjsonXML(const void *args)
     ret = 0;
 
  cleanup:
-    virStorageSourceFree(xmlsrc);
-    virStorageSourceFree(jsonsrc);
     VIR_FREE(propsstr);
     VIR_FREE(protocolwrapper);
     VIR_FREE(actualxml);
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 71c371891f..90a6dce50a 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -95,33 +95,31 @@ testStorageFileGetMetadata(const char *path,
                            uid_t uid, gid_t gid)
 {
     struct stat st;
+    VIR_AUTOPTR(virStorageSource) def = NULL;
     virStorageSourcePtr ret = NULL;
 
-    if (VIR_ALLOC(ret) < 0)
+    if (VIR_ALLOC(def) < 0)
         return NULL;
 
-    ret->type = VIR_STORAGE_TYPE_FILE;
-    ret->format = format;
+    def->type = VIR_STORAGE_TYPE_FILE;
+    def->format = format;
 
     if (stat(path, &st) == 0) {
         if (S_ISDIR(st.st_mode)) {
-            ret->type = VIR_STORAGE_TYPE_DIR;
+            def->type = VIR_STORAGE_TYPE_DIR;
         } else if (S_ISBLK(st.st_mode)) {
-            ret->type = VIR_STORAGE_TYPE_BLOCK;
+            def->type = VIR_STORAGE_TYPE_BLOCK;
         }
     }
 
-    if (VIR_STRDUP(ret->path, path) < 0)
-        goto error;
+    if (VIR_STRDUP(def->path, path) < 0)
+        return NULL;
 
-    if (virStorageFileGetMetadata(ret, uid, gid, false) < 0)
-        goto error;
+    if (virStorageFileGetMetadata(def, uid, gid, false) < 0)
+        return NULL;
 
+    VIR_STEAL_PTR(ret, def);
     return ret;
-
- error:
-    virStorageSourceFree(ret);
-    return NULL;
 }
 
 static int
@@ -308,41 +306,40 @@ static int
 testStorageChain(const void *args)
 {
     const struct testChainData *data = args;
-    int ret = -1;
-    virStorageSourcePtr meta;
     virStorageSourcePtr elt;
     size_t i = 0;
+    VIR_AUTOPTR(virStorageSource) meta = NULL;
     VIR_AUTOFREE(char *) broken = NULL;
 
     meta = testStorageFileGetMetadata(data->start, data->format, -1, -1);
     if (!meta) {
         if (data->flags & EXP_FAIL) {
             virResetLastError();
-            ret = 0;
+            return 0;
         }
-        goto cleanup;
+        return -1;
     } else if (data->flags & EXP_FAIL) {
         fprintf(stderr, "call should have failed\n");
-        goto cleanup;
+        return -1;
     }
     if (data->flags & EXP_WARN) {
         if (virGetLastErrorCode() == VIR_ERR_OK) {
             fprintf(stderr, "call should have warned\n");
-            goto cleanup;
+            return -1;
         }
         virResetLastError();
         if (virStorageFileChainGetBroken(meta, &broken) || !broken) {
             fprintf(stderr, "call should identify broken part of chain\n");
-            goto cleanup;
+            return -1;
         }
     } else {
         if (virGetLastErrorCode()) {
             fprintf(stderr, "call should not have warned\n");
-            goto cleanup;
+            return -1;
         }
         if (virStorageFileChainGetBroken(meta, &broken) || broken) {
             fprintf(stderr, "chain should not be identified as broken\n");
-            goto cleanup;
+            return -1;
         }
     }
 
@@ -353,7 +350,7 @@ testStorageChain(const void *args)
 
         if (i == data->nfiles) {
             fprintf(stderr, "probed chain was too long\n");
-            goto cleanup;
+            return -1;
         }
 
         if (virAsprintf(&expect,
@@ -378,24 +375,21 @@ testStorageChain(const void *args)
                         elt->format,
                         virStorageNetProtocolTypeToString(elt->protocol),
                         NULLSTR(elt->nhosts ? elt->hosts[0].name : NULL)) < 0) {
-            goto cleanup;
+            return -1;
         }
         if (STRNEQ(expect, actual)) {
             virTestDifference(stderr, expect, actual);
-            goto cleanup;
+            return -1;
         }
         elt = elt->backingStore;
         i++;
     }
     if (i != data->nfiles) {
         fprintf(stderr, "probed chain was too short\n");
-        goto cleanup;
+        return -1;
     }
 
-    ret = 0;
- cleanup:
-    virStorageSourceFree(meta);
-    return ret;
+    return 0;
 }
 
 struct testLookupData
@@ -646,7 +640,7 @@ testBackingParse(const void *args)
 {
     const struct testBackingParseData *data = args;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
-    virStorageSourcePtr src = NULL;
+    VIR_AUTOPTR(virStorageSource) src = NULL;
     VIR_AUTOFREE(char *) xml = NULL;
     int ret = -1;
 
@@ -680,7 +674,6 @@ testBackingParse(const void *args)
     ret = 0;
 
  cleanup:
-    virStorageSourceFree(src);
     virBufferFreeAndReset(&buf);
 
     return ret;
@@ -697,7 +690,7 @@ mymain(void)
     struct testPathCanonicalizeData data3;
     struct testPathRelativeBacking data4;
     struct testBackingParseData data5;
-    virStorageSourcePtr chain = NULL;
+    VIR_AUTOPTR(virStorageSource) chain = NULL;
     virStorageSourcePtr chain2; /* short for chain->backingStore */
     virStorageSourcePtr chain3; /* short for chain2->backingStore */
 
@@ -1580,7 +1573,6 @@ mymain(void)
 
  cleanup:
     /* Final cleanup */
-    virStorageSourceFree(chain);
     testCleanupImages();
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
-- 
2.20.1




More information about the libvir-list mailing list