From bf688a0067748b197408ccf4b1eaa8e3f524c60e Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Tue, 12 Feb 2019 13:17:56 -0500 Subject: [PATCH] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageSource MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Let's make use of the auto __cleanup capabilities cleaning up any now unnecessary goto paths. Signed-off-by: John Ferlan Reviewed-by: Erik Skultety Reviewed-by: Ján Tomko --- 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 | 25 +++++--------- src/util/virstoragefile.h | 1 + tests/qemublocktest.c | 6 ++-- tests/virstoragetest.c | 50 +++++++++++---------------- 9 files changed, 40 insertions(+), 63 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2d75849e3d..5d49f4388c 100644 --- a/src/conf/domain_conf.c +++ b/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); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 801d25a44b..ac01e861f7 100644 --- a/src/qemu/qemu_domain.c +++ b/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; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1822248749..dc51de0310 100644 --- a/src/qemu/qemu_driver.c +++ b/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); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 3107a279dd..c93ae33476 100644 --- a/src/qemu/qemu_migration.c +++ b/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; } diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 1888314d95..846a647cb6 100644 --- a/src/storage/storage_backend_gluster.c +++ b/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; diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 60a42a2828..f18e38733a 100644 --- a/src/storage/storage_util.c +++ b/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; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index eacc927ea6..8c3a36d473 100644 --- a/src/util/virstoragefile.h +++ b/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 */ diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 5848f6b5b5..d7e5e72a0b 100644 --- a/tests/qemublocktest.c +++ b/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); diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 8db1d294b0..646ae78ff0 100644 --- a/tests/virstoragetest.c +++ b/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; -- 2.47.2