Rather than having two exit paths, let's use a @retval value and VIR_STEAL_PTR in order to unite the exit path through the error label. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_file_gluster.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c index f8bbde8cfe..41c66c5a5d 100644 --- a/src/storage/storage_file_gluster.c +++ b/src/storage/storage_file_gluster.c @@ -262,6 +262,7 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, size_t bufsiz = 0; ssize_t ret; struct stat st; + int retval = -1; *linkpath = NULL; @@ -291,13 +292,13 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, buf[ret] = '\0'; - *linkpath = buf; + VIR_STEAL_PTR(*linkpath, buf); - return 0; + retval = 0; error: VIR_FREE(buf); - return -1; + return retval; } -- 2.20.1

Let's make use of the auto __cleanup capabilities. This also allows for the cleanup of some goto paths. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend.c | 9 +-- src/storage/storage_backend_disk.c | 62 +++++++------------ src/storage/storage_backend_fs.c | 17 ++--- src/storage/storage_backend_gluster.c | 30 ++++----- src/storage/storage_backend_iscsi.c | 72 +++++++--------------- src/storage/storage_backend_iscsi_direct.c | 36 ++++------- src/storage/storage_backend_logical.c | 32 +++------- src/storage/storage_backend_mpath.c | 17 +++-- src/storage/storage_backend_rbd.c | 35 ++++------- src/storage/storage_backend_scsi.c | 64 +++++++------------ src/storage/storage_backend_sheepdog.c | 27 +++----- src/storage/storage_backend_vstorage.c | 25 +++----- src/storage/storage_backend_zfs.c | 15 ++--- src/storage/storage_file_gluster.c | 17 ++--- 14 files changed, 150 insertions(+), 308 deletions(-) diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index a54c338cf0..5c8275e978 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -87,8 +87,7 @@ virStorageDriverLoadBackendModule(const char *name, const char *regfunc, bool forceload) { - char *modfile = NULL; - int ret; + VIR_AUTOFREE(char *) modfile = NULL; if (!(modfile = virFileFindResourceFull(name, "libvirt_storage_backend_", @@ -98,11 +97,7 @@ virStorageDriverLoadBackendModule(const char *name, "LIBVIRT_STORAGE_BACKEND_DIR"))) return -1; - ret = virModuleLoad(modfile, regfunc, forceload); - - VIR_FREE(modfile); - - return ret; + return virModuleLoad(modfile, regfunc, forceload); } diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index cd2469320b..4c5b784ca7 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -56,8 +56,9 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *tmp, *devpath, *partname; + char *tmp, *partname; bool addVol = false; + VIR_AUTOFREE(char *) devpath = NULL; /* Prepended path will be same for all partitions, so we can * strip the path to form a reasonable pool-unique name @@ -89,7 +90,6 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, * way of doing this... */ vol->target.path = virStorageBackendStablePath(pool, devpath, true); - VIR_FREE(devpath); if (vol->target.path == NULL) goto error; } @@ -355,12 +355,11 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, */ virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *parthelper_path; struct virStorageBackendDiskPoolVolData cbdata = { .pool = pool, .vol = vol, }; - int ret; + VIR_AUTOFREE(char *) parthelper_path = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", @@ -388,12 +387,7 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, def->allocation = 0; def->capacity = def->available = 0; - ret = virCommandRunNul(cmd, - 6, - virStorageBackendDiskMakeVol, - &cbdata); - VIR_FREE(parthelper_path); - return ret; + return virCommandRunNul(cmd, 6, virStorageBackendDiskMakeVol, &cbdata); } static int @@ -419,8 +413,7 @@ static int virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *parthelper_path; - int ret; + VIR_AUTOFREE(char *) parthelper_path = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", @@ -433,12 +426,8 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) "-g", NULL); - ret = virCommandRunNul(cmd, - 3, - virStorageBackendDiskMakePoolGeometry, - pool); - VIR_FREE(parthelper_path); - return ret; + return virCommandRunNul(cmd, 3, virStorageBackendDiskMakePoolGeometry, + pool); } static int @@ -770,13 +759,12 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, unsigned int flags) { char *part_num = NULL; - char *devpath = NULL; char *dev_name; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src_path = def->source.devices[0].path; char *srcname = last_component(src_path); bool isDevMapperDevice; - int rc = -1; + VIR_AUTOFREE(char *) devpath = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); @@ -800,7 +788,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, virReportSystemError(errno, _("Couldn't read volume target path '%s'"), vol->target.path); - goto cleanup; + return -1; } dev_name = last_component(devpath); } @@ -811,7 +799,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, virReportError(VIR_ERR_INTERNAL_ERROR, _("Volume path '%s' did not start with parent " "pool source device name."), dev_name); - goto cleanup; + return -1; } part_num = dev_name + strlen(srcname); @@ -825,7 +813,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse partition number from target " "'%s'"), dev_name); - goto cleanup; + return -1; } /* eg parted /dev/sda rm 2 or /dev/mapper/mpathc rm 2 */ @@ -836,7 +824,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, part_num, NULL); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; /* Refreshing the pool is the easiest option as LOGICAL and EXTENDED * partition allocation/capacity management is handled within @@ -845,12 +833,9 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, */ virStoragePoolObjClearVols(pool); if (virStorageBackendDiskRefreshPool(pool) < 0) - goto cleanup; + return -1; - rc = 0; - cleanup: - VIR_FREE(devpath); - return rc; + return 0; } @@ -858,11 +843,10 @@ static int virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - int res = -1; - char *partFormat = NULL; unsigned long long startOffset = 0, endOffset = 0; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virErrorPtr save_err; + VIR_AUTOFREE(char *)partFormat = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(PARTED, @@ -875,11 +859,11 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("storage pool only supports LUKS encrypted volumes")); - goto cleanup; + return -1; } if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0) - goto cleanup; + return -1; virCommandAddArg(cmd, partFormat); /* If we're going to encrypt using LUKS, then we could need up to @@ -889,13 +873,13 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, if (virStorageBackendDiskPartBoundaries(pool, &startOffset, &endOffset, vol->target.capacity) < 0) - goto cleanup; + return -1; virCommandAddArgFormat(cmd, "%lluB", startOffset); virCommandAddArgFormat(cmd, "%lluB", endOffset); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; /* wait for device node to show up */ virWaitForDevices(); @@ -919,11 +903,7 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, goto error; } - res = 0; - - cleanup: - VIR_FREE(partFormat); - return res; + return 0; error: /* Best effort to remove the partition. Ignore any errors @@ -933,7 +913,7 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, ignore_value(virStorageBackendDiskDeleteVol(pool, vol, 0)); virSetError(save_err); virFreeError(save_err); - goto cleanup; + return -1; } diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 0436c25af0..97148acebe 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -246,11 +246,11 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *src = NULL; FILE *mtab; struct mntent ent; char buf[1024]; int rc1, rc2; + VIR_AUTOFREE(char *) src = NULL; if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) { virReportSystemError(errno, @@ -282,7 +282,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) cleanup: VIR_FORCE_FCLOSE(mtab); - VIR_FREE(src); return ret; } @@ -299,9 +298,8 @@ static int virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *src = NULL; - int ret = -1; int rc; + VIR_AUTOFREE(char *) src = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; if (virStorageBackendFileSystemIsValid(pool) < 0) @@ -320,13 +318,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) return -1; cmd = virStorageBackendFileSystemMountCmd(MOUNT, def, src); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - VIR_FREE(src); - return ret; + return virCommandRun(cmd, NULL); } @@ -579,10 +571,10 @@ virStoragePoolDefFSNamespaceParse(xmlXPathContextPtr ctxt, void **data) { virStoragePoolFSMountOptionsDefPtr cmdopts = NULL; - xmlNodePtr *nodes = NULL; int nnodes; size_t i; int ret = -1; + VIR_AUTOFREE(xmlNodePtr *)nodes = NULL; if (xmlXPathRegisterNs(ctxt, BAD_CAST "fs", BAD_CAST STORAGE_POOL_FS_NAMESPACE_HREF) < 0) { @@ -617,7 +609,6 @@ virStoragePoolDefFSNamespaceParse(xmlXPathContextPtr ctxt, ret = 0; cleanup: - VIR_FREE(nodes); virStoragePoolDefFSNamespaceFree(cmdopts); return ret; } diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 5428bb92ba..1888314d95 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -127,11 +127,9 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool) if (glfs_set_volfile_server(ret->vol, "tcp", ret->uri->server, ret->uri->port) < 0 || glfs_init(ret->vol) < 0) { - char *uri = virURIFormat(ret->uri); - - virReportSystemError(errno, _("failed to connect to %s"), - NULLSTR(uri)); - VIR_FREE(uri); + VIR_AUTOFREE(char *) uri = NULL; + uri = virURIFormat(ret->uri); + virReportSystemError(errno, _("failed to connect to %s"), NULLSTR(uri)); goto error; } @@ -187,9 +185,8 @@ virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state, virStorageVolDefPtr vol, const char *name) { - int ret = -1; - char *path = NULL; char *tmp; + VIR_AUTOFREE(char *) path = NULL; VIR_FREE(vol->key); VIR_FREE(vol->target.path); @@ -200,35 +197,31 @@ virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state, if (name) { VIR_FREE(vol->name); if (VIR_STRDUP(vol->name, name) < 0) - goto cleanup; + return -1; } if (virAsprintf(&path, "%s%s%s", state->volname, state->dir, vol->name) < 0) - goto cleanup; + return -1; tmp = state->uri->path; if (virAsprintf(&state->uri->path, "/%s", path) < 0) { state->uri->path = tmp; - goto cleanup; + return -1; } if (!(vol->target.path = virURIFormat(state->uri))) { VIR_FREE(state->uri->path); state->uri->path = tmp; - goto cleanup; + return -1; } VIR_FREE(state->uri->path); state->uri->path = tmp; /* the path is unique enough to serve as a volume key */ if (VIR_STRDUP(vol->key, vol->target.path) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - VIR_FREE(path); - return ret; + return 0; } @@ -244,10 +237,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, int ret = -1; glfs_fd_t *fd = NULL; virStorageSourcePtr meta = NULL; - char *header = NULL; ssize_t len; int backingFormat; VIR_AUTOPTR(virStorageVolDef) vol = NULL; + VIR_AUTOFREE(char *) header = NULL; *volptr = NULL; @@ -333,7 +326,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, virStorageSourceFree(meta); if (fd) glfs_close(fd); - VIR_FREE(header); return ret; } diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index dc1a983b58..06f815625f 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -130,27 +130,20 @@ static int virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, const char *session) { - char *sysfs_path; - int retval = -1; uint32_t host; + VIR_AUTOFREE(char *) sysfs_path = NULL; if (virAsprintf(&sysfs_path, "/sys/class/iscsi_session/session%s/device", session) < 0) - goto cleanup; + return -1; if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) - goto cleanup; + return -1; if (virStorageBackendSCSIFindLUs(pool, host) < 0) - goto cleanup; - - retval = 0; - - cleanup: - - VIR_FREE(sysfs_path); + return -1; - return retval; + return 0; } @@ -167,7 +160,7 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec, .nsources = 0, .sources = NULL }; - char *portal = NULL; + VIR_AUTOFREE(char *) portal = NULL; VIR_AUTOPTR(virStoragePoolSource) source = NULL; virCheckFlags(0, NULL); @@ -226,7 +219,6 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec, for (i = 0; i < ntargets; i++) VIR_FREE(targets[i]); VIR_FREE(targets); - VIR_FREE(portal); return ret; } @@ -235,8 +227,8 @@ virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool, bool *isActive) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *session = NULL; int ret = -1; + VIR_AUTOFREE(char *) session = NULL; *isActive = false; @@ -259,10 +251,8 @@ virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool, return -1; } - if ((session = virStorageBackendISCSISession(pool, true)) != NULL) { + if ((session = virStorageBackendISCSISession(pool, true))) *isActive = true; - VIR_FREE(session); - } ret = 0; return ret; @@ -330,9 +320,8 @@ static int virStorageBackendISCSIStartPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *portal = NULL; - char *session = NULL; - int ret = -1; + VIR_AUTOFREE(char *) portal = NULL; + VIR_AUTOFREE(char *) session = NULL; if (def->source.nhost != 1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -355,50 +344,40 @@ virStorageBackendISCSIStartPool(virStoragePoolObjPtr pool) if ((session = virStorageBackendISCSISession(pool, true)) == NULL) { if ((portal = virStorageBackendISCSIPortal(&def->source)) == NULL) - goto cleanup; + return -1; /* Create a static node record for the IQN target. Must be done * in order for login to the target */ if (virISCSINodeNew(portal, def->source.devices[0].path) < 0) - goto cleanup; + return -1; if (virStorageBackendISCSISetAuth(portal, &def->source) < 0) - goto cleanup; + return -1; if (virISCSIConnectionLogin(portal, def->source.initiator.iqn, def->source.devices[0].path) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(portal); - VIR_FREE(session); - return ret; + return 0; } static int virStorageBackendISCSIRefreshPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *session = NULL; + VIR_AUTOFREE(char *) session = NULL; def->allocation = def->capacity = def->available = 0; if ((session = virStorageBackendISCSISession(pool, false)) == NULL) - goto cleanup; + return -1; if (virISCSIRescanLUNs(session) < 0) - goto cleanup; + return -1; if (virStorageBackendISCSIFindLUs(pool, session) < 0) - goto cleanup; - VIR_FREE(session); + return -1; return 0; - - cleanup: - VIR_FREE(session); - return -1; } @@ -406,13 +385,11 @@ static int virStorageBackendISCSIStopPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *portal; - char *session; - int ret = -1; + VIR_AUTOFREE(char *) portal = NULL; + VIR_AUTOFREE(char *) session = NULL; if ((session = virStorageBackendISCSISession(pool, true)) == NULL) return 0; - VIR_FREE(session); if ((portal = virStorageBackendISCSIPortal(&def->source)) == NULL) return -1; @@ -420,12 +397,9 @@ virStorageBackendISCSIStopPool(virStoragePoolObjPtr pool) if (virISCSIConnectionLogout(portal, def->source.initiator.iqn, def->source.devices[0].path) < 0) - goto cleanup; - ret = 0; + return -1; - cleanup: - VIR_FREE(portal); - return ret; + return 0; } virStorageBackend virStorageBackendISCSI = { diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index 42060dd758..4ac4ad471c 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -421,15 +421,13 @@ virISCSIDirectUpdateTargets(struct iscsi_context *iscsi, } for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) { - char *target = NULL; + VIR_AUTOFREE(char *) target = NULL; if (VIR_STRDUP(target, tmp_addr->target_name) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) { - VIR_FREE(target); + if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) goto cleanup; - } } VIR_STEAL_PTR(*targets, tmp_targets); @@ -490,7 +488,7 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, .nsources = 0, .sources = NULL }; - char *portal = NULL; + VIR_AUTOFREE(char *) portal = NULL; VIR_AUTOPTR(virStoragePoolSource) source = NULL; virCheckFlags(0, NULL); @@ -551,7 +549,6 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, for (i = 0; i < ntargets; i++) VIR_FREE(targets[i]); VIR_FREE(targets); - VIR_FREE(portal); return ret; } @@ -561,7 +558,7 @@ virStorageBackendISCSIDirectSetConnection(virStoragePoolObjPtr pool, { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); struct iscsi_context *iscsi = NULL; - char *portal = NULL; + VIR_AUTOFREE(char *) portal = NULL; if (!(iscsi = virISCSIDirectCreateContext(def->source.initiator.iqn))) goto error; @@ -578,7 +575,6 @@ virStorageBackendISCSIDirectSetConnection(virStoragePoolObjPtr pool, VIR_STEAL_PTR(*portalRet, portal); cleanup: - VIR_FREE(portal); return iscsi; error: @@ -591,19 +587,14 @@ static int virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool) { struct iscsi_context *iscsi = NULL; - char *portal = NULL; int ret = -1; - if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal))) - goto cleanup; - if (virISCSIDirectReportLuns(pool, iscsi, portal) < 0) - goto disconect; + VIR_AUTOFREE(char *) portal = NULL; - ret = 0; - disconect: + if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal))) + return -1; + ret = virISCSIDirectReportLuns(pool, iscsi, portal); virISCSIDirectDisconnect(iscsi); iscsi_destroy_context(iscsi); - cleanup: - VIR_FREE(portal); return ret; } @@ -639,7 +630,7 @@ virStorageBackendISCSIDirectVolWipeZero(virStorageVolDefPtr vol, struct scsi_task *task = NULL; int lun = 0; int ret = -1; - unsigned char *data; + VIR_AUTOFREE(unsigned char *) data = NULL; if (virStorageBackendISCSIDirectGetLun(vol, &lun) < 0) return ret; @@ -656,22 +647,19 @@ virStorageBackendISCSIDirectVolWipeZero(virStorageVolDefPtr vol, if (!(task = iscsi_write10_sync(iscsi, lun, lba, data, block_size * BLOCK_PER_PACKET, block_size, 0, 0, 0, 0, 0))) - goto cleanup; + return -1; scsi_free_scsi_task(task); lba += BLOCK_PER_PACKET; } else { if (!(task = iscsi_write10_sync(iscsi, lun, lba, data, block_size, block_size, 0, 0, 0, 0, 0))) - goto cleanup; + return -1; scsi_free_scsi_task(task); lba++; } } - ret = 0; - cleanup: - VIR_FREE(data); - return ret; + return 0; } static int diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index b8da5c7efe..c61d03519f 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -117,14 +117,14 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, { int nextents, ret = -1; const char *regex_unit = "(\\S+)\\((\\S+)\\)"; - char *regex = NULL; - regex_t *reg = NULL; - regmatch_t *vars = NULL; char *p = NULL; size_t i; int err, nvars; unsigned long long offset, size, length; virStorageVolSourceExtent extent; + VIR_AUTOFREE(char *) regex = NULL; + VIR_AUTOFREE(regex_t *) reg = NULL; + VIR_AUTOFREE(regmatch_t *) vars = NULL; memset(&extent, 0, sizeof(extent)); @@ -202,7 +202,7 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, for (i = 0; i < nextents; i++) { size_t j; int len; - char *offset_str = NULL; + VIR_AUTOFREE(char *) offset_str = NULL; j = (i * 2) + 1; len = vars[j].rm_eo - vars[j].rm_so; @@ -219,10 +219,8 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed volume extent offset value")); - VIR_FREE(offset_str); goto cleanup; } - VIR_FREE(offset_str); extent.start = offset * size; extent.end = (offset * size) + length; @@ -234,9 +232,6 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, ret = 0; cleanup: - VIR_FREE(regex); - VIR_FREE(reg); - VIR_FREE(vars); VIR_FREE(extent.path); return ret; } @@ -459,16 +454,15 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups, void *data) { virStoragePoolSourceListPtr sourceList = data; - char *pvname = NULL; - char *vgname = NULL; - int retval = -1; size_t i; virStoragePoolSourceDevicePtr dev; virStoragePoolSource *thisSource; + VIR_AUTOFREE(char *) pvname = NULL; + VIR_AUTOFREE(char *) vgname = NULL; if (VIR_STRDUP(pvname, groups[0]) < 0 || VIR_STRDUP(vgname, groups[1]) < 0) - goto error; + return -1; thisSource = NULL; for (i = 0; i < sourceList->nsources; i++) { @@ -480,13 +474,13 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups, if (thisSource == NULL) { if (!(thisSource = virStoragePoolSourceListNewSource(sourceList))) - goto error; + return -1; VIR_STEAL_PTR(thisSource->name, vgname); } if (VIR_REALLOC_N(thisSource->devices, thisSource->ndevice + 1) != 0) - goto error; + return -1; dev = &thisSource->devices[thisSource->ndevice]; thisSource->ndevice++; @@ -495,13 +489,7 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups, memset(dev, 0, sizeof(*dev)); VIR_STEAL_PTR(dev->path, pvname); - retval = 0; - - error: - VIR_FREE(pvname); - VIR_FREE(vgname); - - return retval; + return 0; } /* diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 423f945fbc..cb1b23288a 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -153,31 +153,31 @@ static int virStorageBackendCreateVols(virStoragePoolObjPtr pool, struct dm_names *names) { - int retval = -1, is_mpath = 0; - char *map_device = NULL; + int is_mpath = 0; uint32_t minor = -1; uint32_t next; + VIR_AUTOFREE(char *) map_device = NULL; do { is_mpath = virStorageBackendIsMultipath(names->name); if (is_mpath < 0) - goto out; + return -1; if (is_mpath == 1) { if (virAsprintf(&map_device, "mapper/%s", names->name) < 0) - goto out; + return -1; if (virStorageBackendGetMinorNumber(names->name, &minor) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to get %s minor number"), names->name); - goto out; + return -1; } if (virStorageBackendMpathNewVol(pool, minor, map_device) < 0) - goto out; + return -1; VIR_FREE(map_device); } @@ -191,10 +191,7 @@ virStorageBackendCreateVols(virStoragePoolObjPtr pool, } while (next); - retval = 0; - out: - VIR_FREE(map_device); - return retval; + return 0; } diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index ece04f0f2d..2b7af1db23 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -86,10 +86,10 @@ virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt, void **data) { virStoragePoolRBDConfigOptionsDefPtr cmdopts = NULL; - xmlNodePtr *nodes = NULL; int nnodes; size_t i; int ret = -1; + VIR_AUTOFREE(xmlNodePtr *)nodes = NULL; if (xmlXPathRegisterNs(ctxt, BAD_CAST "rbd", BAD_CAST STORAGE_POOL_RBD_NAMESPACE_HREF) < 0) { @@ -145,7 +145,6 @@ virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt, ret = 0; cleanup: - VIR_FREE(nodes); virStoragePoolDefRBDNamespaceFree(cmdopts); return ret; } @@ -213,12 +212,12 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, char *rados_key = NULL; virBuffer mon_host = VIR_BUFFER_INITIALIZER; size_t i; - char *mon_buff = NULL; const char *client_mount_timeout = "30"; const char *mon_op_timeout = "30"; const char *osd_op_timeout = "30"; const char *rbd_default_format = "2"; virConnectPtr conn = NULL; + VIR_AUTOFREE(char *) mon_buff = NULL; if (authdef) { VIR_DEBUG("Using cephx authorization, username: %s", authdef->username); @@ -348,7 +347,6 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, virObjectUnref(conn); virBufferFreeAndReset(&mon_host); - VIR_FREE(mon_buff); return ret; } @@ -574,11 +572,12 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) int ret = -1; int len = -1; int r = 0; - char *name, *names = NULL; + char *name; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; struct rados_cluster_stat_t clusterstat; struct rados_pool_stat_t poolstat; + VIR_AUTOFREE(char *) names = NULL; if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; @@ -662,7 +661,6 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) ret = 0; cleanup: - VIR_FREE(names); virStorageBackendRBDFreeState(&ptr); return ret; } @@ -677,8 +675,8 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, int max_snaps = 128; int snap_count, protected; size_t i; - rbd_snap_info_t *snaps = NULL; rbd_image_t image = NULL; + VIR_AUTOFREE(rbd_snap_info_t *) snaps = NULL; if ((r = rbd_open(ioctx, vol->name, &image, NULL)) < 0) { virReportSystemError(-r, _("failed to open the RBD image '%s'"), @@ -737,8 +735,6 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, if (snaps) rbd_snap_list_end(snaps); - VIR_FREE(snaps); - if (image) rbd_close(image); @@ -949,8 +945,8 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, int max_snaps = 128; size_t i; int diff; - rbd_snap_info_t *snaps = NULL; rbd_image_info_t info; + VIR_AUTOFREE(rbd_snap_info_t *) snaps = NULL; if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { virReportSystemError(-r, _("failed to stat the RBD image %s"), @@ -1023,8 +1019,6 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, if (snaps) rbd_snap_list_end(snaps); - VIR_FREE(snaps); - return ret; } @@ -1098,8 +1092,8 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, uint64_t stripe_count; uint64_t stripe_unit; virBuffer snapname = VIR_BUFFER_INITIALIZER; - char *snapname_buff = NULL; rbd_image_t image = NULL; + VIR_AUTOFREE(char *) snapname_buff = NULL; if ((r = rbd_open(io, origvol, &image, NULL)) < 0) { virReportSystemError(-r, _("failed to open the RBD image %s"), @@ -1170,7 +1164,6 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, cleanup: virBufferFreeAndReset(&snapname); - VIR_FREE(snapname_buff); if (image) rbd_close(image); @@ -1271,13 +1264,12 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, uint64_t stripe_count) { int r = -1; - int ret = -1; unsigned long long offset = 0; unsigned long long length; - char *writebuf; + VIR_AUTOFREE(char *) writebuf = NULL; if (VIR_ALLOC_N(writebuf, info->obj_size * stripe_count) < 0) - goto cleanup; + return -1; while (offset < info->size) { length = MIN((info->size - offset), (info->obj_size * stripe_count)); @@ -1286,7 +1278,7 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, virReportSystemError(-r, _("writing %llu bytes failed on " "RBD image %s at offset %llu"), length, imgname, offset); - goto cleanup; + return -1; } VIR_DEBUG("Wrote %llu bytes to RBD image %s at offset %llu", @@ -1295,12 +1287,7 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, offset += length; } - ret = 0; - - cleanup: - VIR_FREE(writebuf); - - return ret; + return 0; } static int diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 591bcb04e2..c1470fa1d5 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -57,13 +57,13 @@ virStorageBackendSCSITriggerRescan(uint32_t host) { int fd = -1; int retval = -1; - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; VIR_DEBUG("Triggering rescan of host %d", host); if (virAsprintf(&path, "%s/host%u/scan", LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) - goto cleanup; + return -1; VIR_DEBUG("Scan trigger path is '%s'", path); @@ -89,7 +89,6 @@ virStorageBackendSCSITriggerRescan(uint32_t host) cleanup: VIR_FORCE_CLOSE(fd); - VIR_FREE(path); VIR_DEBUG("Rescan of host %d complete", host); return retval; } @@ -175,7 +174,6 @@ static char * getAdapterName(virStorageAdapterPtr adapter) { char *name = NULL; - char *parentaddr = NULL; if (adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) { virStorageAdapterSCSIHostPtr scsi_host = &adapter->data.scsi_host; @@ -189,7 +187,7 @@ getAdapterName(virStorageAdapterPtr adapter) addr->slot, addr->function, unique_id))) - goto cleanup; + return NULL; } else { ignore_value(VIR_STRDUP(name, scsi_host->name)); } @@ -203,8 +201,6 @@ getAdapterName(virStorageAdapterPtr adapter) } } - cleanup: - VIR_FREE(parentaddr); return name; } @@ -245,10 +241,10 @@ checkParent(const char *name, const char *parent_name) { unsigned int host_num; - char *scsi_host_name = NULL; - char *vhba_parent = NULL; bool retval = false; virConnectPtr conn = NULL; + VIR_AUTOFREE(char *) scsi_host_name = NULL; + VIR_AUTOFREE(char *) vhba_parent = NULL; VIR_DEBUG("name=%s, parent_name=%s", name, parent_name); @@ -288,8 +284,6 @@ checkParent(const char *name, cleanup: virObjectUnref(conn); - VIR_FREE(vhba_parent); - VIR_FREE(scsi_host_name); return retval; } @@ -299,10 +293,9 @@ createVport(virStoragePoolDefPtr def, const char *configFile, virStorageAdapterFCHostPtr fchost) { - char *name = NULL; virStoragePoolFCRefreshInfoPtr cbdata = NULL; virThread thread; - int ret = -1; + VIR_AUTOFREE(char *) name = NULL; VIR_DEBUG("configFile='%s' parent='%s', wwnn='%s' wwpn='%s'", NULLSTR(configFile), NULLSTR(fchost->parent), @@ -314,14 +307,14 @@ createVport(virStoragePoolDefPtr def, */ if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { if (!(checkName(name))) - goto cleanup; + return -1; /* If a parent was provided, let's make sure the 'name' we've * retrieved has the same parent. If not this will cause failure. */ if (!fchost->parent || checkParent(name, fchost->parent)) - ret = 0; + return 0; - goto cleanup; + return -1; } /* Since we're creating the vHBA, then we need to manage removing it @@ -333,12 +326,12 @@ createVport(virStoragePoolDefPtr def, fchost->managed = VIR_TRISTATE_BOOL_YES; if (configFile) { if (virStoragePoolSaveConfig(configFile, def) < 0) - goto cleanup; + return -1; } } if (!(name = virNodeDeviceCreateVport(fchost))) - goto cleanup; + return -1; /* Creating our own VPORT didn't leave enough time to find any LUN's, * so, let's create a thread whose job it is to call the FindLU's with @@ -357,11 +350,7 @@ createVport(virStoragePoolDefPtr def, } } - ret = 0; - - cleanup: - VIR_FREE(name); - return ret; + return 0; } @@ -370,10 +359,9 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, bool *isActive) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *path = NULL; - char *name = NULL; unsigned int host; - int ret = -1; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) name = NULL; *isActive = false; @@ -391,28 +379,23 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, } if (virSCSIHostGetNumber(name, &host) < 0) - goto cleanup; + return -1; if (virAsprintf(&path, "%s/host%d", LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) - goto cleanup; + return -1; *isActive = virFileExists(path); - ret = 0; - cleanup: - VIR_FREE(path); - VIR_FREE(name); - return ret; + return 0; } static int virStorageBackendSCSIRefreshPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *name = NULL; unsigned int host; - int ret = -1; + VIR_AUTOFREE(char *) name = NULL; def->allocation = def->capacity = def->available = 0; @@ -420,20 +403,17 @@ virStorageBackendSCSIRefreshPool(virStoragePoolObjPtr pool) return -1; if (virSCSIHostGetNumber(name, &host) < 0) - goto out; + return -1; VIR_DEBUG("Scanning host%u", host); if (virStorageBackendSCSITriggerRescan(host) < 0) - goto out; + return -1; if (virStorageBackendSCSIFindLUs(pool, host) < 0) - goto out; + return -1; - ret = 0; - out: - VIR_FREE(name); - return ret; + return 0; } diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index a57faaef58..99f3283a1c 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -136,9 +136,8 @@ virStorageBackendSheepdogAddVolume(virStoragePoolObjPtr pool, const char *diskIn static int virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) { - int ret = -1; - char *output = NULL; size_t i; + VIR_AUTOFREE(char *) output = NULL; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) cells = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; @@ -147,11 +146,11 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; lines = virStringSplit(output, "\n", 0); if (lines == NULL) - goto cleanup; + return -1; for (i = 0; lines[i]; i++) { const char *line = lines[i]; @@ -163,42 +162,34 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) if (cells != NULL && virStringListLength((const char * const *)cells) > 2) { if (virStorageBackendSheepdogAddVolume(pool, cells[1]) < 0) - goto cleanup; + return -1; } virStringListFree(cells); cells = NULL; } - ret = 0; - - cleanup: - VIR_FREE(output); - return ret; + return 0; } static int virStorageBackendSheepdogRefreshPool(virStoragePoolObjPtr pool) { - int ret = -1; - char *output = NULL; + VIR_AUTOFREE(char *) output = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(SHEEPDOGCLI, "node", "info", "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; if (virStorageBackendSheepdogParseNodeInfo(virStoragePoolObjGetDef(pool), output) < 0) - goto cleanup; + return -1; - ret = virStorageBackendSheepdogRefreshAllVol(pool); - cleanup: - VIR_FREE(output); - return ret; + return virStorageBackendSheepdogRefreshAllVol(pool); } diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c index 8c5becd4c4..df157a48b1 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -39,9 +39,9 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *grp_name = NULL; - char *usr_name = NULL; - char *mode = NULL; + VIR_AUTOFREE(char *) grp_name = NULL; + VIR_AUTOFREE(char *) usr_name = NULL; + VIR_AUTOFREE(char *) mode = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; /* Check the permissions */ @@ -55,13 +55,13 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) /* Convert ids to names because vstorage uses names */ if (!(grp_name = virGetGroupName(def->target.perms.gid))) - goto cleanup; + return -1; if (!(usr_name = virGetUserName(def->target.perms.uid))) - goto cleanup; + return -1; if (virAsprintf(&mode, "%o", def->target.perms.mode) < 0) - goto cleanup; + return -1; cmd = virCommandNewArgList(VSTORAGE_MOUNT, "-c", def->source.name, @@ -70,15 +70,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) "-g", grp_name, "-u", usr_name, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - ret = 0; - - cleanup: - VIR_FREE(mode); - VIR_FREE(grp_name); - VIR_FREE(usr_name); - return ret; + return virCommandRun(cmd, NULL); } @@ -90,7 +82,7 @@ virStorageBackendVzIsMounted(virStoragePoolObjPtr pool) FILE *mtab; struct mntent ent; char buf[1024]; - char *cluster = NULL; + VIR_AUTOFREE(char *) cluster = NULL; if (virAsprintf(&cluster, "vstorage://%s", def->source.name) < 0) return -1; @@ -115,7 +107,6 @@ virStorageBackendVzIsMounted(virStoragePoolObjPtr pool) cleanup: VIR_FORCE_FCLOSE(mtab); - VIR_FREE(cluster); return ret; } diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index 7d1a3dd2cd..7ffdff638e 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -52,7 +52,7 @@ static int virStorageBackendZFSVolModeNeeded(void) { int ret = -1, exit_code = -1; - char *error = NULL; + VIR_AUTOFREE(char *) error = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; /* 'zfs get' without arguments prints out @@ -77,7 +77,6 @@ virStorageBackendZFSVolModeNeeded(void) ret = 0; cleanup: - VIR_FREE(error); return ret; } @@ -86,13 +85,12 @@ virStorageBackendZFSCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, bool *isActive) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *devpath; + VIR_AUTOFREE(char *) devpath = NULL; if (virAsprintf(&devpath, "/dev/zvol/%s", def->source.name) < 0) return -1; *isActive = virFileIsDir(devpath); - VIR_FREE(devpath); return 0; } @@ -178,10 +176,10 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *volumes_list = NULL; size_t i; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; + VIR_AUTOFREE(char *) volumes_list = NULL; /** * $ zfs list -Hp -t volume -o name,volsize -r test @@ -203,10 +201,10 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, NULL); virCommandSetOutputBuffer(cmd, &volumes_list); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; if (!(lines = virStringSplit(volumes_list, "\n", 0))) - goto cleanup; + return -1; for (i = 0; lines[i]; i++) { if (STREQ(lines[i], "")) @@ -216,9 +214,6 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, continue; } - cleanup: - VIR_FREE(volumes_list); - return 0; } diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c index 41c66c5a5d..7c2189d297 100644 --- a/src/storage/storage_file_gluster.c +++ b/src/storage/storage_file_gluster.c @@ -258,11 +258,10 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, void *data) { virStorageFileBackendGlusterPrivPtr priv = data; - char *buf = NULL; size_t bufsiz = 0; ssize_t ret; struct stat st; - int retval = -1; + VIR_AUTOFREE(char *) buf = NULL; *linkpath = NULL; @@ -278,13 +277,13 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, realloc: if (VIR_EXPAND_N(buf, bufsiz, 256) < 0) - goto error; + return -1; if ((ret = glfs_readlink(priv->vol, path, buf, bufsiz)) < 0) { virReportSystemError(errno, _("failed to read link of gluster file '%s'"), path); - goto error; + return -1; } if (ret == bufsiz) @@ -294,11 +293,7 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, VIR_STEAL_PTR(*linkpath, buf); - retval = 0; - - error: - VIR_FREE(buf); - return retval; + return 0; } @@ -306,7 +301,7 @@ static const char * virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src) { virStorageFileBackendGlusterPrivPtr priv = src->drv->priv; - char *filePath = NULL; + VIR_AUTOFREE(char *) filePath = NULL; if (priv->canonpath) return priv->canonpath; @@ -322,8 +317,6 @@ virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src) src->volume, filePath)); - VIR_FREE(filePath); - return priv->canonpath; } -- 2.20.1

Rather than overload @ret with trying serve multiple purposes, let's initialize @ret to -1 and introduce an @rc function return value that can be used for functions that may return -1 or -2 and only override @ret when rc < 0. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_util.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 57624425d0..df3dfeb319 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -1754,24 +1754,26 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype, unsigned int openflags, unsigned int readflags) { - int ret, fd = -1; + int ret = -1; + int rc; + int fd = -1; struct stat sb; ssize_t len = VIR_STORAGE_MAX_HEADER; VIR_AUTOFREE(char *) buf = NULL; - if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) - goto cleanup; - fd = ret; + if ((rc = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) + return rc; + fd = rc; - if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0) + if ((virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0) goto cleanup; if ((voltype == VIR_STORAGE_VOL_FILE || voltype == VIR_STORAGE_VOL_BLOCK) && target->format != VIR_STORAGE_FILE_NONE) { if (S_ISDIR(sb.st_mode)) { if (storageBackendIsPloopDir(target->path)) { - if ((ret = storageBackendRedoPloopUpdate(target, &sb, &fd, - openflags)) < 0) + if ((storageBackendRedoPloopUpdate(target, &sb, &fd, + openflags)) < 0) goto cleanup; target->format = VIR_STORAGE_FILE_PLOOP; } else { @@ -1782,7 +1784,6 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype, if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { virReportSystemError(errno, _("cannot seek to start of '%s'"), target->path); - ret = -1; goto cleanup; } @@ -1795,23 +1796,24 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype, virReportSystemError(errno, _("cannot read header '%s'"), target->path); - ret = -1; } goto cleanup; } - if (virStorageSourceUpdateCapacity(target, buf, len, false) < 0) { - ret = -1; + if (virStorageSourceUpdateCapacity(target, buf, len, false) < 0) goto cleanup; - } } if (withBlockVolFormat) { - if ((ret = virStorageBackendDetectBlockVolFormatFD(target, fd, - readflags)) < 0) + if ((rc = virStorageBackendDetectBlockVolFormatFD(target, fd, + readflags)) < 0) { + ret = rc; goto cleanup; + } } + ret = 0; + cleanup: VIR_FORCE_CLOSE(fd); return ret; -- 2.20.1

Modify code to use the VIR_AUTOCLOSE logic cleaning up any now unnecessary goto paths. Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com> --- src/storage/storage_backend_logical.c | 3 +- src/storage/storage_backend_scsi.c | 13 +-- src/storage/storage_file_fs.c | 15 +-- src/storage/storage_util.c | 143 +++++++++----------------- src/util/virstoragefile.c | 39 +++---- 5 files changed, 74 insertions(+), 139 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index c61d03519f..f153d23aec 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -911,10 +911,10 @@ static int virStorageBackendLogicalCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - int fd = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virErrorPtr err; struct stat sb; + VIR_AUTOCLOSE fd = -1; vol->type = VIR_STORAGE_VOL_BLOCK; @@ -971,7 +971,6 @@ virStorageBackendLogicalCreateVol(virStoragePoolObjPtr pool, error: err = virSaveLastError(); - VIR_FORCE_CLOSE(fd); virStorageBackendLogicalDeleteVol(pool, vol, 0); virSetError(err); virFreeError(err); diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index c1470fa1d5..1c27fc74f6 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -55,8 +55,7 @@ struct _virStoragePoolFCRefreshInfo { static int virStorageBackendSCSITriggerRescan(uint32_t host) { - int fd = -1; - int retval = -1; + VIR_AUTOCLOSE fd = -1; VIR_AUTOFREE(char *) path = NULL; VIR_DEBUG("Triggering rescan of host %d", host); @@ -73,7 +72,7 @@ virStorageBackendSCSITriggerRescan(uint32_t host) virReportSystemError(errno, _("Could not open '%s' to trigger host scan"), path); - goto cleanup; + return -1; } if (safewrite(fd, @@ -82,15 +81,11 @@ virStorageBackendSCSITriggerRescan(uint32_t host) virReportSystemError(errno, _("Write to '%s' to trigger host scan failed"), path); - goto cleanup; + return -1; } - retval = 0; - - cleanup: - VIR_FORCE_CLOSE(fd); VIR_DEBUG("Rescan of host %d complete", host); - return retval; + return 0; } /** diff --git a/src/storage/storage_file_fs.c b/src/storage/storage_file_fs.c index 3b6ed6e34d..8817970f44 100644 --- a/src/storage/storage_file_fs.c +++ b/src/storage/storage_file_fs.c @@ -83,8 +83,8 @@ virStorageFileBackendFileInit(virStorageSourcePtr src) static int virStorageFileBackendFileCreate(virStorageSourcePtr src) { - int fd = -1; mode_t mode = S_IRUSR; + VIR_AUTOCLOSE fd = -1; if (!src->readonly) mode |= S_IWUSR; @@ -95,7 +95,6 @@ virStorageFileBackendFileCreate(virStorageSourcePtr src) return -1; } - VIR_FORCE_CLOSE(fd); return 0; } @@ -121,8 +120,8 @@ virStorageFileBackendFileRead(virStorageSourcePtr src, size_t len, char **buf) { - int fd = -1; ssize_t ret = -1; + VIR_AUTOCLOSE fd = -1; if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, src->drv->uid, src->drv->gid, 0)) < 0) { @@ -134,19 +133,15 @@ virStorageFileBackendFileRead(virStorageSourcePtr src, if (offset > 0) { if (lseek(fd, offset, SEEK_SET) == (off_t) -1) { virReportSystemError(errno, _("cannot seek into '%s'"), src->path); - goto cleanup; + return -1; } } if ((ret = virFileReadHeaderFD(fd, len, buf)) < 0) { - virReportSystemError(errno, - _("cannot read header '%s'"), src->path); - goto cleanup; + virReportSystemError(errno, _("cannot read header '%s'"), src->path); + return -1; } - cleanup: - VIR_FORCE_CLOSE(fd); - return ret; } diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index df3dfeb319..60a42a2828 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -130,7 +130,6 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, bool want_sparse, bool reflink_copy) { - int inputfd = -1; int amtread = -1; int ret = 0; size_t rbytes = READ_BLOCK_SIZE_DEFAULT; @@ -139,13 +138,14 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, struct stat st; VIR_AUTOFREE(char *) zerobuf = NULL; VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOCLOSE inputfd = -1; if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { ret = -errno; virReportSystemError(errno, _("could not open input path '%s'"), inputvol->target.path); - goto cleanup; + return ret; } #ifdef __linux__ @@ -157,15 +157,11 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, if (wbytes < WRITE_BLOCK_SIZE_DEFAULT) wbytes = WRITE_BLOCK_SIZE_DEFAULT; - if (VIR_ALLOC_N(zerobuf, wbytes) < 0) { - ret = -errno; - goto cleanup; - } + if (VIR_ALLOC_N(zerobuf, wbytes) < 0) + return -errno; - if (VIR_ALLOC_N(buf, rbytes) < 0) { - ret = -errno; - goto cleanup; - } + if (VIR_ALLOC_N(buf, rbytes) < 0) + return -errno; if (reflink_copy) { if (reflinkCloneFile(fd, inputfd) < 0) { @@ -173,10 +169,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, virReportSystemError(errno, _("failed to clone files from '%s'"), inputvol->target.path); - goto cleanup; + return ret; } else { VIR_DEBUG("btrfs clone finished."); - goto cleanup; + return 0; } } @@ -191,7 +187,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, virReportSystemError(errno, _("failed reading from file '%s'"), inputvol->target.path); - goto cleanup; + return ret; } *total -= amtread; @@ -208,14 +204,14 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, virReportSystemError(errno, _("cannot extend file '%s'"), vol->target.path); - goto cleanup; + return ret; } } else if (safewrite(fd, buf+offset, interval) < 0) { ret = -errno; virReportSystemError(errno, _("failed writing to file '%s'"), vol->target.path); - goto cleanup; + return ret; } } while ((amtleft -= interval) > 0); @@ -225,23 +221,18 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, ret = -errno; virReportSystemError(errno, _("cannot sync data to file '%s'"), vol->target.path); - goto cleanup; + return ret; } - if (VIR_CLOSE(inputfd) < 0) { ret = -errno; virReportSystemError(errno, _("cannot close file '%s'"), inputvol->target.path); - goto cleanup; + return ret; } - inputfd = -1; - - cleanup: - VIR_FORCE_CLOSE(inputfd); - return ret; + return 0; } static int @@ -250,14 +241,13 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr inputvol, unsigned int flags) { - int fd = -1; - int ret = -1; unsigned long long remain; struct stat st; gid_t gid; uid_t uid; mode_t mode; bool reflink_copy = false; + VIR_AUTOCLOSE fd = -1; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA | VIR_STORAGE_VOL_CREATE_REFLINK, @@ -267,7 +257,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("metadata preallocation is not supported for block " "volumes")); - goto cleanup; + return -1; } if (flags & VIR_STORAGE_VOL_CREATE_REFLINK) @@ -277,7 +267,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virReportSystemError(errno, _("cannot create path '%s'"), vol->target.path); - goto cleanup; + return -1; } remain = vol->target.capacity; @@ -285,13 +275,13 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, if (inputvol) { if (virStorageBackendCopyToFD(vol, inputvol, fd, &remain, false, reflink_copy) < 0) - goto cleanup; + return -1; } if (fstat(fd, &st) == -1) { virReportSystemError(errno, _("stat of '%s' failed"), vol->target.path); - goto cleanup; + return -1; } uid = (vol->target.perms->uid != st.st_uid) ? vol->target.perms->uid : (uid_t)-1; @@ -303,7 +293,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, _("cannot chown '%s' to (%u, %u)"), vol->target.path, (unsigned int)uid, (unsigned int)gid); - goto cleanup; + return -1; } mode = (vol->target.perms->mode == (mode_t)-1 ? @@ -312,21 +302,16 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virReportSystemError(errno, _("cannot set mode of '%s' to %04o"), vol->target.path, mode); - goto cleanup; + return -1; } if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("cannot close file '%s'"), vol->target.path); - goto cleanup; + return -1; } - fd = -1; - ret = 0; - cleanup: - VIR_FORCE_CLOSE(fd); - - return ret; + return 0; } static int @@ -419,11 +404,11 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool, { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int ret = -1; - int fd = -1; int operation_flags; bool reflink_copy = false; mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE; bool created = false; + VIR_AUTOCLOSE fd = -1; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA | VIR_STORAGE_VOL_CREATE_REFLINK, @@ -501,7 +486,6 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool, ignore_value(virFileRemove(vol->target.path, vol->target.perms->uid, vol->target.perms->gid)); - VIR_FORCE_CLOSE(fd); return ret; } @@ -542,7 +526,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, * re-open the file and attempt to force the mode change. */ if (mode != (st.st_mode & S_IRWXUGO)) { - int fd = -1; + VIR_AUTOCLOSE fd = -1; int flags = VIR_FILE_OPEN_FORK | VIR_FILE_OPEN_FORCE_MODE; if ((fd = virFileOpenAs(vol->target.path, O_RDWR, mode, @@ -550,7 +534,6 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, vol->target.perms->gid, flags)) >= 0) { /* Success - means we're good */ - VIR_FORCE_CLOSE(fd); ret = 0; goto cleanup; } @@ -1227,10 +1210,10 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, { virStorageEncryptionPtr enc = vol->target.encryption; char *secretPath = NULL; - int fd = -1; uint8_t *secret = NULL; size_t secretlen = 0; virConnectPtr conn = NULL; + VIR_AUTOCLOSE fd = -1; if (!enc) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1268,7 +1251,6 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, _("failed to write secret file")); goto error; } - VIR_FORCE_CLOSE(fd); if ((vol->target.perms->uid != (uid_t)-1) && (vol->target.perms->gid != (gid_t)-1)) { @@ -1283,7 +1265,6 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, cleanup: virObjectUnref(conn); VIR_DISPOSE_N(secret, secretlen); - VIR_FORCE_CLOSE(fd); return secretPath; @@ -1754,19 +1735,18 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype, unsigned int openflags, unsigned int readflags) { - int ret = -1; int rc; - int fd = -1; struct stat sb; ssize_t len = VIR_STORAGE_MAX_HEADER; VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOCLOSE fd = -1; if ((rc = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) return rc; fd = rc; if ((virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0) - goto cleanup; + return -1; if ((voltype == VIR_STORAGE_VOL_FILE || voltype == VIR_STORAGE_VOL_BLOCK) && target->format != VIR_STORAGE_FILE_NONE) { @@ -1774,49 +1754,39 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype, if (storageBackendIsPloopDir(target->path)) { if ((storageBackendRedoPloopUpdate(target, &sb, &fd, openflags)) < 0) - goto cleanup; + return -1; target->format = VIR_STORAGE_FILE_PLOOP; } else { - ret = 0; - goto cleanup; + return 0; } } if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { virReportSystemError(errno, _("cannot seek to start of '%s'"), target->path); - goto cleanup; + return -1; } if ((len = virFileReadHeaderFD(fd, len, &buf)) < 0) { if (readflags & VIR_STORAGE_VOL_READ_NOERROR) { VIR_WARN("ignoring failed header read for '%s'", target->path); - ret = -2; + return -2; } else { virReportSystemError(errno, _("cannot read header '%s'"), target->path); + return -1; } - goto cleanup; } if (virStorageSourceUpdateCapacity(target, buf, len, false) < 0) - goto cleanup; - } - - if (withBlockVolFormat) { - if ((rc = virStorageBackendDetectBlockVolFormatFD(target, fd, - readflags)) < 0) { - ret = rc; - goto cleanup; - } + return -1; } - ret = 0; + if (withBlockVolFormat) + return virStorageBackendDetectBlockVolFormatFD(target, fd, readflags); - cleanup: - VIR_FORCE_CLOSE(fd); - return ret; + return 0; } /* @@ -2622,9 +2592,9 @@ storageBackendVolWipeLocalFile(const char *path, unsigned long long allocation, bool zero_end) { - int ret = -1, fd = -1; const char *alg_char = NULL; struct stat st; + VIR_AUTOCLOSE fd = -1; VIR_AUTOPTR(virCommand) cmd = NULL; fd = open(path, O_RDWR); @@ -2632,14 +2602,14 @@ storageBackendVolWipeLocalFile(const char *path, virReportSystemError(errno, _("Failed to open storage volume with path '%s'"), path); - goto cleanup; + return -1; } if (fstat(fd, &st) == -1) { virReportSystemError(errno, _("Failed to stat storage volume with path '%s'"), path); - goto cleanup; + return -1; } switch ((virStorageVolWipeAlgorithm) algorithm) { @@ -2673,12 +2643,12 @@ storageBackendVolWipeLocalFile(const char *path, case VIR_STORAGE_VOL_WIPE_ALG_TRIM: virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", _("'trim' algorithm not supported")); - goto cleanup; + return -1; case VIR_STORAGE_VOL_WIPE_ALG_LAST: virReportError(VIR_ERR_INVALID_ARG, _("unsupported algorithm %d"), algorithm); - goto cleanup; + return -1; } VIR_DEBUG("Wiping file '%s' with algorithm '%s'", path, alg_char); @@ -2687,24 +2657,14 @@ storageBackendVolWipeLocalFile(const char *path, cmd = virCommandNew(SCRUB); virCommandAddArgList(cmd, "-f", "-p", alg_char, path, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - } else { - if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) { - ret = storageBackendVolZeroSparseFileLocal(path, st.st_size, fd); - } else { - ret = storageBackendWipeLocal(path, fd, allocation, st.st_blksize, - zero_end); - } - if (ret < 0) - goto cleanup; + return virCommandRun(cmd, NULL); } - cleanup: - VIR_FORCE_CLOSE(fd); - return ret; + if (S_ISREG(st.st_mode) && st.st_blocks < (st.st_size / DEV_BSIZE)) + return storageBackendVolZeroSparseFileLocal(path, st.st_size, fd); + + return storageBackendWipeLocal(path, fd, allocation, st.st_blksize, + zero_end); } @@ -3397,11 +3357,11 @@ storageBackendProbeTarget(virStorageSourcePtr target, virStorageEncryptionPtr *encryption) { int backingStoreFormat; - int fd = -1; int ret = -1; int rc; virStorageSourcePtr meta = NULL; struct stat sb; + VIR_AUTOCLOSE fd = -1; if (encryption) *encryption = NULL; @@ -3502,7 +3462,6 @@ storageBackendProbeTarget(virStorageSourcePtr target, } cleanup: - VIR_FORCE_CLOSE(fd); virStorageSourceFree(meta); return ret; } @@ -3574,8 +3533,9 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) struct stat statbuf; virStorageSourcePtr target = NULL; int direrr; - int fd = -1, ret = -1; + int ret = -1; VIR_AUTOPTR(virStorageVolDef) vol = NULL; + VIR_AUTOCLOSE fd = -1; if (virDirOpen(&dir, def->target.path) < 0) goto cleanup; @@ -3666,7 +3626,6 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) ret = 0; cleanup: VIR_DIR_CLOSE(dir); - VIR_FORCE_CLOSE(fd); virStorageSourceFree(target); if (ret < 0) virStoragePoolObjClearVols(pool); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index b08070b782..5a8e5667f5 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1080,10 +1080,9 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta, int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid) { - int fd; - int ret = -1; struct stat sb; ssize_t len = VIR_STORAGE_MAX_HEADER; + VIR_AUTOCLOSE fd = -1; VIR_AUTOFREE(char *) header = NULL; if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) { @@ -1093,31 +1092,24 @@ virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid) if (fstat(fd, &sb) < 0) { virReportSystemError(errno, _("cannot stat file '%s'"), path); - goto cleanup; + return -1; } /* No header to probe for directories */ - if (S_ISDIR(sb.st_mode)) { - ret = VIR_STORAGE_FILE_DIR; - goto cleanup; - } + if (S_ISDIR(sb.st_mode)) + return VIR_STORAGE_FILE_DIR; if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { virReportSystemError(errno, _("cannot set to start of '%s'"), path); - goto cleanup; + return -1; } if ((len = virFileReadHeaderFD(fd, len, &header)) < 0) { virReportSystemError(errno, _("cannot read header '%s'"), path); - goto cleanup; + return -1; } - ret = virStorageFileProbeFormatFromBuf(path, header, len); - - cleanup: - VIR_FORCE_CLOSE(fd); - - return ret; + return virStorageFileProbeFormatFromBuf(path, header, len); } @@ -1312,13 +1304,12 @@ virStorageFileResize(const char *path, unsigned long long capacity, bool pre_allocate) { - int fd = -1; - int ret = -1; int rc; + VIR_AUTOCLOSE fd = -1; if ((fd = open(path, O_RDWR)) < 0) { virReportSystemError(errno, _("Unable to open '%s'"), path); - goto cleanup; + return -1; } if (pre_allocate) { @@ -1331,26 +1322,22 @@ virStorageFileResize(const char *path, _("Failed to pre-allocate space for " "file '%s'"), path); } - goto cleanup; + return -1; } } if (ftruncate(fd, capacity) < 0) { virReportSystemError(errno, _("Failed to truncate file '%s'"), path); - goto cleanup; + return -1; } if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("Unable to save '%s'"), path); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FORCE_CLOSE(fd); - return ret; + return 0; } -- 2.20.1

Only one path will consume the @def; otherwise, we need to free it. Signed-off-by: John Ferlan <jferlan@redhat.com> --- tests/storagepoolxml2argvtest.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c index 288b81af1d..b7e32064af 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -25,29 +25,33 @@ testCompareXMLToArgvFiles(bool shouldFail, int ret = -1; virStoragePoolDefPtr def = NULL; virStoragePoolObjPtr pool = NULL; + const char *defTypeStr; VIR_AUTOFREE(char *) actualCmdline = NULL; VIR_AUTOFREE(char *) src = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; + defTypeStr = virStoragePoolTypeToString(def->type); switch ((virStoragePoolType)def->type) { case VIR_STORAGE_POOL_FS: case VIR_STORAGE_POOL_NETFS: if (!(pool = virStoragePoolObjNew())) { - VIR_TEST_DEBUG("pool type %d alloc pool obj fails\n", def->type); + VIR_TEST_DEBUG("pool type '%s' alloc pool obj fails\n", defTypeStr); virStoragePoolDefFree(def); goto cleanup; } virStoragePoolObjSetDef(pool, def); if (!(src = virStorageBackendFileSystemGetPoolSource(pool))) { - VIR_TEST_DEBUG("pool type %d has no pool source\n", def->type); + VIR_TEST_DEBUG("pool type '%s' has no pool source\n", defTypeStr); + def = NULL; goto cleanup; } cmd = virStorageBackendFileSystemMountCmd(MOUNT, def, src); + def = NULL; break; case VIR_STORAGE_POOL_LOGICAL: @@ -67,12 +71,12 @@ testCompareXMLToArgvFiles(bool shouldFail, case VIR_STORAGE_POOL_VSTORAGE: case VIR_STORAGE_POOL_LAST: default: - VIR_TEST_DEBUG("pool type %d has no xml2argv test\n", def->type); + VIR_TEST_DEBUG("pool type '%s' has no xml2argv test\n", defTypeStr); goto cleanup; }; if (!(actualCmdline = virCommandToString(cmd, false))) { - VIR_TEST_DEBUG("pool type %d failed to get commandline\n", def->type); + VIR_TEST_DEBUG("pool type '%s' failed to get commandline\n", defTypeStr); goto cleanup; } @@ -83,6 +87,7 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: + virStoragePoolDefFree(def); virStoragePoolObjEndAPI(&pool); if (shouldFail) { virResetLastError(); -- 2.20.1

If virDomainHostdevSubsysSCSIiSCSIDefParseXML processing finds a duplicated <auth> structure, we should error out rather than continue. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9e46cf721b..2d75849e3d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7616,6 +7616,12 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, while (cur != NULL) { if (cur->type == XML_ELEMENT_NODE && virXMLNodeNameEqual(cur, "auth")) { + if (iscsisrc->src->auth) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("an <auth> definition already found for " + "the <hostdev> iSCSI definition")); + return -1; + } if (!(authdef = virStorageAuthDefParse(cur, ctxt))) return -1; if ((auth_secret_usage = -- 2.20.1

If we find multiple "id=" strings during processing, then we need to force an error since we cannot have multiple <auth>'s defined for a single source volume. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/virstoragefile.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 5a8e5667f5..8d85d9dac4 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2840,6 +2840,11 @@ virStorageSourceParseRBDColonString(const char *rbdstr, if (STRPREFIX(p, "id=")) { /* formulate authdef for src->auth */ + if (src->auth) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("duplicate 'id' found in '%s'"), src->path); + return -1; + } if (VIR_ALLOC(authdef) < 0) return -1; -- 2.20.1

Let's make use of the auto __cleanup capabilities cleaning up any now unnecessary goto paths. Signed-off-by: John Ferlan <jferlan@redhat.com> Reviewed-by: Ján Tomko <jtomko@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 | 25 ++--- src/util/virstoragefile.c | 141 +++++++++++--------------- src/util/virstoragefile.h | 1 + tests/qemublocktest.c | 6 +- tests/virstoragetest.c | 50 ++++----- 10 files changed, 97 insertions(+), 147 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.c b/src/util/virstoragefile.c index 8d85d9dac4..8eccc428da 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1117,7 +1117,8 @@ static virStorageSourcePtr virStorageFileMetadataNew(const char *path, int format) { - virStorageSourcePtr def = NULL; + virStorageSourcePtr ret = NULL; + VIR_AUTOPTR(virStorageSource) def = NULL; if (VIR_ALLOC(def) < 0) return NULL; @@ -1126,13 +1127,10 @@ virStorageFileMetadataNew(const char *path, def->type = VIR_STORAGE_TYPE_FILE; if (VIR_STRDUP(def->path, path) < 0) - goto error; - - return def; + return NULL; - error: - virStorageSourceFree(def); - return NULL; + VIR_STEAL_PTR(ret, def); + return ret; } @@ -1205,11 +1203,11 @@ virStorageFileGetMetadataFromFD(const char *path, { virStorageSourcePtr ret = NULL; - virStorageSourcePtr meta = NULL; ssize_t len = VIR_STORAGE_MAX_HEADER; struct stat sb; int dummy; VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOPTR(virStorageSource) meta = NULL; if (!backingFormat) backingFormat = &dummy; @@ -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,7 +2238,8 @@ virStorageSourcePtr virStorageSourceCopy(const virStorageSource *src, bool backingChain) { - virStorageSourcePtr def = NULL; + virStorageSourcePtr ret = NULL; + VIR_AUTOPTR(virStorageSource) def = NULL; if (VIR_ALLOC(def) < 0) return NULL; @@ -2282,60 +2278,57 @@ virStorageSourceCopy(const virStorageSource *src, VIR_STRDUP(def->compat, src->compat) < 0 || VIR_STRDUP(def->tlsAlias, src->tlsAlias) < 0 || VIR_STRDUP(def->tlsCertdir, src->tlsCertdir) < 0) - goto error; + return NULL; if (src->nhosts) { if (!(def->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts))) - goto error; + return NULL; def->nhosts = src->nhosts; } if (src->srcpool && !(def->srcpool = virStorageSourcePoolDefCopy(src->srcpool))) - goto error; + return NULL; if (src->features && !(def->features = virBitmapNewCopy(src->features))) - goto error; + return NULL; if (src->encryption && !(def->encryption = virStorageEncryptionCopy(src->encryption))) - goto error; + return NULL; if (src->perms && !(def->perms = virStoragePermsCopy(src->perms))) - goto error; + return NULL; if (src->timestamps && !(def->timestamps = virStorageTimestampsCopy(src->timestamps))) - goto error; + return NULL; if (virStorageSourceSeclabelsCopy(def, src) < 0) - goto error; + return NULL; if (src->auth && !(def->auth = virStorageAuthDefCopy(src->auth))) - goto error; + return NULL; if (src->pr && !(def->pr = virStoragePRDefCopy(src->pr))) - goto error; + return NULL; if (virStorageSourceInitiatorCopy(&def->initiator, &src->initiator)) - goto error; + return NULL; if (backingChain && src->backingStore) { if (!(def->backingStore = virStorageSourceCopy(src->backingStore, true))) - goto error; + return NULL; } - return def; - - error: - virStorageSourceFree(def); - return NULL; + VIR_STEAL_PTR(ret, def); + return ret; } @@ -2577,27 +2570,28 @@ static virStorageSourcePtr virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, const char *rel) { - virStorageSourcePtr def; + virStorageSourcePtr ret = NULL; VIR_AUTOFREE(char *) dirname = NULL; + VIR_AUTOPTR(virStorageSource) def = NULL; if (VIR_ALLOC(def) < 0) return NULL; /* store relative name */ if (VIR_STRDUP(def->relPath, parent->backingStoreRaw) < 0) - goto error; + return NULL; if (!(dirname = mdir_name(parent->path))) { virReportOOMError(); - goto error; + return NULL; } if (STRNEQ(dirname, "/")) { if (virAsprintf(&def->path, "%s/%s", dirname, rel) < 0) - goto error; + return NULL; } else { if (virAsprintf(&def->path, "/%s", rel) < 0) - goto error; + return NULL; } if (virStorageSourceGetActualType(parent) == VIR_STORAGE_TYPE_NETWORK) { @@ -2608,25 +2602,20 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, if (parent->nhosts) { if (!(def->hosts = virStorageNetHostDefCopy(parent->nhosts, parent->hosts))) - goto error; + return NULL; def->nhosts = parent->nhosts; } if (VIR_STRDUP(def->volume, parent->volume) < 0) - goto error; + return NULL; } else { /* set the type to _FILE, the caller shall update it to the actual type */ def->type = VIR_STORAGE_TYPE_FILE; } - cleanup: - return def; - - error: - virStorageSourceFree(def); - def = NULL; - goto cleanup; + VIR_STEAL_PTR(ret, def); + return ret; } @@ -3624,8 +3613,9 @@ virStorageSourcePtr virStorageSourceNewFromBackingAbsolute(const char *path) { const char *json; - virStorageSourcePtr def; + virStorageSourcePtr ret = NULL; int rc; + VIR_AUTOPTR(virStorageSource) def = NULL; if (VIR_ALLOC(def) < 0) return NULL; @@ -3634,7 +3624,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path) def->type = VIR_STORAGE_TYPE_FILE; if (VIR_STRDUP(def->path, path) < 0) - goto error; + return NULL; } else { def->type = VIR_STORAGE_TYPE_NETWORK; @@ -3649,7 +3639,7 @@ virStorageSourceNewFromBackingAbsolute(const char *path) rc = virStorageSourceParseBackingColon(def, path); if (rc < 0) - goto error; + return NULL; virStorageSourceNetworkAssignDefaultPorts(def); @@ -3662,11 +3652,8 @@ virStorageSourceNewFromBackingAbsolute(const char *path) } } - return def; - - error: - virStorageSourceFree(def); - return NULL; + VIR_STEAL_PTR(ret, def); + return ret; } @@ -3674,7 +3661,8 @@ virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent) { struct stat st; - virStorageSourcePtr def; + virStorageSourcePtr ret = NULL; + VIR_AUTOPTR(virStorageSource) def = NULL; if (virStorageIsRelative(parent->backingStoreRaw)) def = virStorageSourceNewFromBackingRelative(parent, @@ -3697,17 +3685,14 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent) /* copy parent's labelling and other top level stuff */ if (virStorageSourceInitChainElement(def, parent, true) < 0) - goto error; + return NULL; def->readonly = true; def->detected = true; } - return def; - - error: - virStorageSourceFree(def); - return NULL; + VIR_STEAL_PTR(ret, def); + return ret; } @@ -3848,9 +3833,8 @@ virStorageSourceUpdateCapacity(virStorageSourcePtr src, ssize_t len, bool probe) { - int ret = -1; - virStorageSourcePtr meta = NULL; int format = src->format; + VIR_AUTOPTR(virStorageSource) meta = NULL; /* Raw files: capacity is physical size. For all other files: if * the metadata has a capacity, use that, otherwise fall back to @@ -3860,12 +3844,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; } @@ -3878,17 +3862,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; } @@ -4825,10 +4805,10 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, int ret = -1; const char *uniqueName; ssize_t headerLen; - virStorageSourcePtr backingStore = NULL; int backingFormat; int rv; VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOPTR(virStorageSource) backingStore = NULL; VIR_DEBUG("path=%s format=%d uid=%u gid=%u", src->path, src->format, @@ -4911,7 +4891,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, if (virStorageSourceHasBacking(src)) src->backingStore->id = depth; virStorageFileDeinit(src); - virStorageSourceFree(backingStore); return ret; } @@ -4980,11 +4959,10 @@ int virStorageFileGetBackingStoreStr(virStorageSourcePtr src, char **backing) { - virStorageSourcePtr tmp = NULL; ssize_t headerLen; - int ret = -1; int rv; VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOPTR(virStorageSource) tmp = NULL; *backing = NULL; @@ -5008,17 +4986,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 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.20.1