util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageSource

Let's make use of the auto __cleanup capabilities cleaning up any
now unnecessary goto paths.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

src/conf/domain_conf.c

@@ -9065,13 +9065,13 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
                                unsigned int flags,
                                virDomainXMLOptionPtr xmlopt)
 {
-    virStorageSourcePtr backingStore = NULL;
     xmlNodePtr save_ctxt = ctxt->node;
     xmlNodePtr source;
     char *type = NULL;
     char *format = NULL;
     char *idx = NULL;
     int ret = -1;
+    VIR_AUTOPTR(virStorageSource) backingStore = NULL;
 
     if (!(ctxt->node = virXPathNode("./backingStore", ctxt))) {
         ret = 0;
@@ -9132,7 +9132,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
     ret = 0;
 
  cleanup:
-    virStorageSourceFree(backingStore);
     VIR_FREE(type);
     VIR_FREE(format);
     VIR_FREE(idx);

src/qemu/qemu_domain.c

@@ -2730,10 +2730,10 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node,
 {
     xmlNodePtr savedNode = ctxt->node;
     qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
-    virStorageSourcePtr migrSource = NULL;
     char *format = NULL;
     char *type = NULL;
     int ret = -1;
+    VIR_AUTOPTR(virStorageSource) migrSource = NULL;
 
     ctxt->node = node;
 
@@ -2781,7 +2781,6 @@ qemuDomainObjPrivateXMLParseJobNBDSource(xmlNodePtr node,
     ret = 0;
 
  cleanup:
-    virStorageSourceFree(migrSource);
     VIR_FREE(format);
     VIR_FREE(type);
     ctxt->node = savedNode;

src/qemu/qemu_driver.c

@@ -274,11 +274,11 @@ qemuSecurityChownCallback(const virStorageSource *src,
                           uid_t uid,
                           gid_t gid)
 {
-    virStorageSourcePtr cpy = NULL;
     struct stat sb;
     int save_errno = 0;
     int ret = -1;
     int rv;
+    VIR_AUTOPTR(virStorageSource) cpy = NULL;
 
     rv = virStorageFileSupportsSecurityDriver(src);
     if (rv <= 0)
@@ -319,7 +319,6 @@ qemuSecurityChownCallback(const virStorageSource *src,
  cleanup:
     save_errno = errno;
     virStorageFileDeinit(cpy);
-    virStorageSourceFree(cpy);
     errno = save_errno;
 
     return ret;
@@ -17958,7 +17957,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 |
@@ -18020,7 +18019,6 @@ qemuDomainBlockRebase(virDomainPtr dom, const char *path, const char *base,
 
  cleanup:
     virDomainObjEndAPI(&vm);
-    virStorageSourceFree(dest);
     return ret;
 }
 
@@ -18150,10 +18148,10 @@ qemuDomainBlockCommit(virDomainPtr dom,
     char *topPath = NULL;
     char *basePath = NULL;
     char *backingPath = NULL;
-    virStorageSourcePtr mirror = NULL;
     unsigned long long speed = bandwidth;
     qemuBlockJobDataPtr job = NULL;
     qemuBlockJobType jobtype = QEMU_BLOCKJOB_TYPE_COMMIT;
+    VIR_AUTOPTR(virStorageSource) mirror = NULL;
 
     /* XXX Add support for COMMIT_DELETE */
     virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
@@ -18352,7 +18350,6 @@ qemuDomainBlockCommit(virDomainPtr dom,
             virFreeError(orig_err);
         }
     }
-    virStorageSourceFree(mirror);
     qemuBlockJobStartupFinalize(job);
     qemuDomainObjEndJob(driver, vm);
 

src/qemu/qemu_migration.c

@@ -788,9 +788,9 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver,
 {
     qemuBlockStorageSourceAttachDataPtr data = NULL;
     qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
-    virStorageSourcePtr copysrc = NULL;
     int mon_ret = 0;
     int ret = -1;
+    VIR_AUTOPTR(virStorageSource) copysrc = NULL;
 
     VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias, host);
 
@@ -849,7 +849,6 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver,
 
  cleanup:
     qemuBlockStorageSourceAttachDataFree(data);
-    virStorageSourceFree(copysrc);
     return ret;
 }
 

src/storage/storage_backend_gluster.c

@@ -236,10 +236,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
 {
     int ret = -1;
     glfs_fd_t *fd = NULL;
-    virStorageSourcePtr meta = NULL;
     ssize_t len;
     int backingFormat;
     VIR_AUTOPTR(virStorageVolDef) vol = NULL;
+    VIR_AUTOPTR(virStorageSource) meta = NULL;
     VIR_AUTOFREE(char *) header = NULL;
 
     *volptr = NULL;
@@ -323,7 +323,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
     VIR_STEAL_PTR(*volptr, vol);
     ret = 0;
  cleanup:
-    virStorageSourceFree(meta);
     if (fd)
         glfs_close(fd);
     return ret;

src/storage/storage_util.c

@@ -3357,10 +3357,9 @@ storageBackendProbeTarget(virStorageSourcePtr target,
                           virStorageEncryptionPtr *encryption)
 {
     int backingStoreFormat;
-    int ret = -1;
     int rc;
-    virStorageSourcePtr meta = NULL;
     struct stat sb;
+    VIR_AUTOPTR(virStorageSource) meta = NULL;
     VIR_AUTOCLOSE fd = -1;
 
     if (encryption)
@@ -3372,17 +3371,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;
         }
     }
 
@@ -3390,11 +3388,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;
 
@@ -3405,7 +3403,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;
@@ -3434,8 +3432,6 @@ storageBackendProbeTarget(virStorageSourcePtr target,
     target->format = meta->format;
 
     /* Default to success below this point */
-    ret = 0;
-
     if (meta->capacity)
         target->capacity = meta->capacity;
 
@@ -3461,9 +3457,7 @@ storageBackendProbeTarget(virStorageSourcePtr target,
         VIR_STEAL_PTR(target->compat, meta->compat);
     }
 
- cleanup:
-    virStorageSourceFree(meta);
-    return ret;
+    return 0;
 }
 
 
@@ -3531,11 +3525,11 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
     struct dirent *ent;
     struct statvfs sb;
     struct stat statbuf;
-    virStorageSourcePtr target = NULL;
     int direrr;
     int ret = -1;
     VIR_AUTOPTR(virStorageVolDef) vol = NULL;
     VIR_AUTOCLOSE fd = -1;
+    VIR_AUTOPTR(virStorageSource) target = NULL;
 
     if (virDirOpen(&dir, def->target.path) < 0)
         goto cleanup;
@@ -3626,7 +3620,6 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool)
     ret = 0;
  cleanup:
     VIR_DIR_CLOSE(dir);
-    virStorageSourceFree(target);
     if (ret < 0)
         virStoragePoolObjClearVols(pool);
     return ret;

src/util/virstoragefile.h

@@ -544,5 +544,6 @@ void virStorageFileReportBrokenChain(int errcode,
                                      virStorageSourcePtr parent);
 
 VIR_DEFINE_AUTOPTR_FUNC(virStorageAuthDef, virStorageAuthDefFree);
+VIR_DEFINE_AUTOPTR_FUNC(virStorageSource, virStorageSourceFree);
 
 #endif /* LIBVIRT_VIRSTORAGEFILE_H */

tests/qemublocktest.c

@@ -46,14 +46,14 @@ testBackingXMLjsonXML(const void *args)
     xmlDocPtr xml = NULL;
     xmlXPathContextPtr ctxt = NULL;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
-    virStorageSourcePtr xmlsrc = NULL;
-    virStorageSourcePtr jsonsrc = NULL;
     virJSONValuePtr backendprops = NULL;
     virJSONValuePtr wrapper = NULL;
     char *propsstr = NULL;
     char *protocolwrapper = NULL;
     char *actualxml = NULL;
     int ret = -1;
+    VIR_AUTOPTR(virStorageSource) xmlsrc = NULL;
+    VIR_AUTOPTR(virStorageSource) jsonsrc = NULL;
 
     if (VIR_ALLOC(xmlsrc) < 0)
         return -1;
@@ -104,8 +104,6 @@ testBackingXMLjsonXML(const void *args)
     ret = 0;
 
  cleanup:
-    virStorageSourceFree(xmlsrc);
-    virStorageSourceFree(jsonsrc);
     VIR_FREE(propsstr);
     VIR_FREE(protocolwrapper);
     VIR_FREE(actualxml);

tests/virstoragetest.c

@@ -95,7 +95,8 @@ testStorageFileGetMetadata(const char *path,
                            uid_t uid, gid_t gid)
 {
     struct stat st;
-    virStorageSourcePtr def = NULL;
+    virStorageSourcePtr ret = NULL;
+    VIR_AUTOPTR(virStorageSource) def = NULL;
 
     if (VIR_ALLOC(def) < 0)
         return NULL;
@@ -112,16 +113,13 @@ testStorageFileGetMetadata(const char *path,
     }
 
     if (VIR_STRDUP(def->path, path) < 0)
-        goto error;
+        return NULL;
 
     if (virStorageFileGetMetadata(def, uid, gid, false) < 0)
-        goto error;
-
-    return def;
+        return NULL;
 
- error:
-    virStorageSourceFree(def);
-    return NULL;
+    VIR_STEAL_PTR(ret, def);
+    return ret;
 }
 
 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,9 +640,9 @@ testBackingParse(const void *args)
 {
     const struct testBackingParseData *data = args;
     virBuffer buf = VIR_BUFFER_INITIALIZER;
-    virStorageSourcePtr src = NULL;
     int ret = -1;
     VIR_AUTOFREE(char *) xml = NULL;
+    VIR_AUTOPTR(virStorageSource) src = NULL;
 
     if (!(src = virStorageSourceNewFromBackingAbsolute(data->backing))) {
         if (!data->expect)
@@ -680,7 +674,6 @@ testBackingParse(const void *args)
     ret = 0;
 
  cleanup:
-    virStorageSourceFree(src);
     virBufferFreeAndReset(&buf);
 
     return ret;
@@ -696,10 +689,10 @@ mymain(void)
     struct testPathCanonicalizeData data3;
     struct testPathRelativeBacking data4;
     struct testBackingParseData data5;
-    virStorageSourcePtr chain = NULL;
     virStorageSourcePtr chain2; /* short for chain->backingStore */
     virStorageSourcePtr chain3; /* short for chain2->backingStore */
     VIR_AUTOPTR(virCommand) cmd = NULL;
+    VIR_AUTOPTR(virStorageSource) chain = NULL;
 
     if (storageRegisterAll() < 0)
        return EXIT_FAILURE;
@@ -1580,7 +1573,6 @@ mymain(void)
 
  cleanup:
     /* Final cleanup */
-    virStorageSourceFree(chain);
     testCleanupImages();
 
     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;