Let's make use of the auto __cleanup capabilities cleaning up any now unnecessary goto paths. For virStorageAuthDefCopy use authdef and ret as consistently as similar other code. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 29 +++++++++++------------------ src/conf/storage_conf.c | 6 ++---- src/qemu/qemu_parse_command.c | 6 ++---- src/util/virstoragefile.c | 33 ++++++++++++++------------------- src/util/virstoragefile.h | 1 + 5 files changed, 30 insertions(+), 45 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1fc4c8a35a..5699a60549 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7578,10 +7578,9 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, virDomainHostdevSubsysSCSIPtr def, xmlXPathContextPtr ctxt) { - int ret = -1; int auth_secret_usage = -1; xmlNodePtr cur; - virStorageAuthDefPtr authdef = NULL; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &def->u.iscsi; /* For the purposes of command line creation, this needs to look @@ -7594,23 +7593,23 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, if (!(iscsisrc->src->path = virXMLPropString(sourcenode, "name"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing iSCSI hostdev source path name")); - goto cleanup; + return -1; } if (virDomainStorageNetworkParseHosts(sourcenode, &iscsisrc->src->hosts, &iscsisrc->src->nhosts) < 0) - goto cleanup; + return -1; if (iscsisrc->src->nhosts < 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing the host address for the iSCSI hostdev")); - goto cleanup; + return -1; } if (iscsisrc->src->nhosts > 1) { virReportError(VIR_ERR_XML_ERROR, "%s", _("only one source host address may be specified " "for the iSCSI hostdev")); - goto cleanup; + return -1; } cur = sourcenode->children; @@ -7618,30 +7617,25 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, if (cur->type == XML_ELEMENT_NODE && virXMLNodeNameEqual(cur, "auth")) { if (!(authdef = virStorageAuthDefParse(cur, ctxt))) - goto cleanup; + return -1; if ((auth_secret_usage = virSecretUsageTypeFromString(authdef->secrettype)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("invalid secret type %s"), authdef->secrettype); - goto cleanup; + return -1; } if (auth_secret_usage != VIR_SECRET_USAGE_TYPE_ISCSI) { virReportError(VIR_ERR_INTERNAL_ERROR, _("hostdev invalid secret type '%s'"), authdef->secrettype); - goto cleanup; + return -1; } - iscsisrc->src->auth = authdef; - authdef = NULL; + VIR_STEAL_PTR(iscsisrc->src->auth, authdef); } cur = cur->next; } - ret = 0; - - cleanup: - virStorageAuthDefFree(authdef); - return ret; + return 0; } static int @@ -9684,7 +9678,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, virStorageEncryptionPtr encryption = NULL; char *serial = NULL; char *startupPolicy = NULL; - virStorageAuthDefPtr authdef = NULL; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; char *tray = NULL; char *removable = NULL; char *logical_block_size = NULL; @@ -10094,7 +10088,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(target); VIR_FREE(tray); VIR_FREE(removable); - virStorageAuthDefFree(authdef); VIR_FREE(devaddr); VIR_FREE(serial); virStorageEncryptionFree(encryption); diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 1ee31ca676..9cd3d836a3 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -458,7 +458,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, int nsource; size_t i; virStoragePoolOptionsPtr options; - virStorageAuthDefPtr authdef = NULL; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; char *name = NULL; char *port = NULL; char *ver = NULL; @@ -584,8 +584,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, goto cleanup; } - source->auth = authdef; - authdef = NULL; + VIR_STEAL_PTR(source->auth, authdef); } /* Option protocol version string (NFSvN) */ @@ -615,7 +614,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, VIR_FREE(port); VIR_FREE(nodeset); - virStorageAuthDefFree(authdef); return ret; } diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index c4650f01e0..81691cb85e 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -59,7 +59,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, char *sock = NULL; char *volimg = NULL; char *secret = NULL; - virStorageAuthDefPtr authdef = NULL; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; if (VIR_ALLOC(def->src->hosts) < 0) goto error; @@ -133,8 +133,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, if (VIR_STRDUP(authdef->secrettype, secrettype) < 0) goto error; } - def->src->auth = authdef; - authdef = NULL; + VIR_STEAL_PTR(def->src->auth, authdef); /* Cannot formulate a secretType (eg, usage or uuid) given * what is provided. @@ -152,7 +151,6 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, error: virStorageNetHostDefClear(def->src->hosts); VIR_FREE(def->src->hosts); - virStorageAuthDefFree(authdef); goto cleanup; } diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 8319ba9c8c..b3f5e0204d 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1879,26 +1879,24 @@ virStorageAuthDefFree(virStorageAuthDefPtr authdef) virStorageAuthDefPtr virStorageAuthDefCopy(const virStorageAuthDef *src) { - virStorageAuthDefPtr ret; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; + virStorageAuthDefPtr ret = NULL; - if (VIR_ALLOC(ret) < 0) + if (VIR_ALLOC(authdef) < 0) return NULL; - if (VIR_STRDUP(ret->username, src->username) < 0) - goto error; + if (VIR_STRDUP(authdef->username, src->username) < 0) + return NULL; /* Not present for storage pool, but used for disk source */ - if (VIR_STRDUP(ret->secrettype, src->secrettype) < 0) - goto error; - ret->authType = src->authType; + if (VIR_STRDUP(authdef->secrettype, src->secrettype) < 0) + return NULL; + authdef->authType = src->authType; - if (virSecretLookupDefCopy(&ret->seclookupdef, &src->seclookupdef) < 0) - goto error; + if (virSecretLookupDefCopy(&authdef->seclookupdef, &src->seclookupdef) < 0) + return NULL; + VIR_STEAL_PTR(ret, authdef); return ret; - - error: - virStorageAuthDefFree(ret); - return NULL; } @@ -1907,7 +1905,7 @@ virStorageAuthDefParse(xmlNodePtr node, xmlXPathContextPtr ctxt) { xmlNodePtr saveNode = ctxt->node; - virStorageAuthDefPtr authdef = NULL; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; virStorageAuthDefPtr ret = NULL; xmlNodePtr secretnode = NULL; char *authtype = NULL; @@ -1958,7 +1956,6 @@ virStorageAuthDefParse(xmlNodePtr node, cleanup: VIR_FREE(authtype); - virStorageAuthDefFree(authdef); ctxt->node = saveNode; return ret; @@ -2832,7 +2829,7 @@ virStorageSourceParseRBDColonString(const char *rbdstr, { char *options = NULL; char *p, *e, *next; - virStorageAuthDefPtr authdef = NULL; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; /* optionally skip the "rbd:" prefix if provided */ if (STRPREFIX(rbdstr, "rbd:")) @@ -2895,9 +2892,8 @@ virStorageSourceParseRBDColonString(const char *rbdstr, if (VIR_STRDUP(authdef->secrettype, virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_CEPH)) < 0) goto error; - src->auth = authdef; + VIR_STEAL_PTR(src->auth, authdef); src->authInherited = true; - authdef = NULL; /* Cannot formulate a secretType (eg, usage or uuid) given * what is provided. @@ -2936,7 +2932,6 @@ virStorageSourceParseRBDColonString(const char *rbdstr, error: VIR_FREE(options); - virStorageAuthDefFree(authdef); return -1; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index a98d5103fa..f1164dde9b 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -543,4 +543,5 @@ void virStorageFileReportBrokenChain(int errcode, virStorageSourcePtr src, virStorageSourcePtr parent); +VIR_DEFINE_AUTOPTR_FUNC(virStorageAuthDef, virStorageAuthDefFree); #endif /* LIBVIRT_VIRSTORAGEFILE_H */ -- 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> --- src/conf/storage_conf.c | 4 ++-- src/conf/storage_conf.h | 2 ++ src/storage/storage_backend_fs.c | 3 +-- src/storage/storage_backend_gluster.c | 3 +-- src/storage/storage_backend_iscsi.c | 3 +-- src/storage/storage_backend_iscsi_direct.c | 3 +-- src/test/test_driver.c | 14 +++++++------- 7 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9cd3d836a3..6099c64b26 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -625,7 +625,8 @@ virStoragePoolDefParseSourceString(const char *srcSpec, xmlDocPtr doc = NULL; xmlNodePtr node = NULL; xmlXPathContextPtr xpath_ctxt = NULL; - virStoragePoolSourcePtr def = NULL, ret = NULL; + VIR_AUTOPTR(virStoragePoolSource) def = NULL; + virStoragePoolSourcePtr ret = NULL; if (!(doc = virXMLParseStringCtxt(srcSpec, _("(storage_source_specification)"), @@ -647,7 +648,6 @@ virStoragePoolDefParseSourceString(const char *srcSpec, VIR_STEAL_PTR(ret, def); cleanup: - virStoragePoolSourceFree(def); xmlFreeDoc(doc); xmlXPathFreeContext(xpath_ctxt); diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 1ba4b80616..3fa97bba76 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -460,4 +460,6 @@ VIR_ENUM_DECL(virStoragePartedFs); VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_AUTOSTART | \ VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) +VIR_DEFINE_AUTOPTR_FUNC(virStoragePoolSource, virStoragePoolSourceFree); + #endif /* LIBVIRT_STORAGE_CONF_H */ diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index ddb422d874..06c76fde4f 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -145,7 +145,7 @@ virStorageBackendFileSystemNetFindPoolSources(const char *srcSpec, .sources = NULL } }; - virStoragePoolSourcePtr source = NULL; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; char *ret = NULL; size_t i; int retNFS = -1; @@ -196,7 +196,6 @@ virStorageBackendFileSystemNetFindPoolSources(const char *srcSpec, virStoragePoolSourceClear(&state.list.sources[i]); VIR_FREE(state.list.sources); - virStoragePoolSourceFree(source); return ret; } diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index e09bc54196..8c6205b081 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -489,7 +489,7 @@ virStorageBackendGlusterFindPoolSources(const char *srcSpec, .nsources = 0, .sources = NULL }; - virStoragePoolSourcePtr source = NULL; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; char *ret = NULL; int rc; size_t i; @@ -532,7 +532,6 @@ virStorageBackendGlusterFindPoolSources(const char *srcSpec, virStoragePoolSourceClear(&list.sources[i]); VIR_FREE(list.sources); - virStoragePoolSourceFree(source); return ret; } diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 4792bd70b8..483ba15102 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -158,7 +158,7 @@ static char * virStorageBackendISCSIFindPoolSources(const char *srcSpec, unsigned int flags) { - virStoragePoolSourcePtr source = NULL; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; size_t ntargets = 0; char **targets = NULL; char *ret = NULL; @@ -227,7 +227,6 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec, VIR_FREE(targets[i]); VIR_FREE(targets); VIR_FREE(portal); - virStoragePoolSourceFree(source); return ret; } diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index 0babe26550..ddc150f03d 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -486,7 +486,7 @@ static char * virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, unsigned int flags) { - virStoragePoolSourcePtr source = NULL; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; size_t ntargets = 0; char **targets = NULL; char *ret = NULL; @@ -557,7 +557,6 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, VIR_FREE(targets[i]); VIR_FREE(targets); VIR_FREE(portal); - virStoragePoolSourceFree(source); return ret; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b06567855e..743d03ae8b 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4394,7 +4394,7 @@ testConnectFindStoragePoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, const char *srcSpec, unsigned int flags) { - virStoragePoolSourcePtr source = NULL; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; int pool_type; char *ret = NULL; @@ -4404,30 +4404,32 @@ testConnectFindStoragePoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, if (!pool_type) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown storage pool type %s"), type); - goto cleanup; + return NULL; } if (srcSpec) { source = virStoragePoolDefParseSourceString(srcSpec, pool_type); if (!source) - goto cleanup; + return NULL; } switch (pool_type) { case VIR_STORAGE_POOL_LOGICAL: ignore_value(VIR_STRDUP(ret, defaultPoolSourcesLogicalXML)); + return ret; break; case VIR_STORAGE_POOL_NETFS: if (!source || !source->hosts[0].name) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("hostname must be specified for netfs sources")); - goto cleanup; + return NULL; } ignore_value(virAsprintf(&ret, defaultPoolSourcesNetFSXML, source->hosts[0].name)); + return ret; break; default: @@ -4435,9 +4437,7 @@ testConnectFindStoragePoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, _("pool type '%s' does not support source discovery"), type); } - cleanup: - virStoragePoolSourceFree(source); - return ret; + return NULL; } -- 2.20.1

Let's make use of the auto __cleanup capabilities cleaning up any now unnecessary goto paths. For virStorageVolDefParseXML use the @def/@ret similarly as other methods. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 112 ++++++++++----------- src/conf/storage_conf.h | 1 + src/esx/esx_storage_backend_vmfs.c | 6 +- src/phyp/phyp_driver.c | 3 +- src/storage/storage_backend_gluster.c | 9 +- src/storage/storage_backend_iscsi_direct.c | 19 ++-- src/storage/storage_backend_mpath.c | 24 ++--- src/storage/storage_backend_rbd.c | 9 +- src/storage/storage_backend_sheepdog.c | 15 ++- src/storage/storage_driver.c | 6 +- src/storage/storage_util.c | 6 +- src/test/test_driver.c | 10 +- src/vbox/vbox_storage.c | 3 +- tests/storagebackendsheepdogtest.c | 3 +- tests/storagevolxml2argvtest.c | 5 +- tests/storagevolxml2xmltest.c | 3 +- 16 files changed, 100 insertions(+), 134 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 6099c64b26..83ca379217 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1168,7 +1168,8 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, xmlXPathContextPtr ctxt, unsigned int flags) { - virStorageVolDefPtr ret; + VIR_AUTOPTR(virStorageVolDef) def = NULL; + virStorageVolDefPtr ret = NULL; virStorageVolOptionsPtr options; char *type = NULL; char *allocation = NULL; @@ -1187,132 +1188,132 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (options == NULL) return NULL; - if (VIR_ALLOC(ret) < 0) + if (VIR_ALLOC(def) < 0) return NULL; - ret->target.type = VIR_STORAGE_TYPE_FILE; + def->target.type = VIR_STORAGE_TYPE_FILE; - ret->name = virXPathString("string(./name)", ctxt); - if (ret->name == NULL) { + def->name = virXPathString("string(./name)", ctxt); + if (def->name == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing volume name element")); - goto error; + goto cleanup; } /* Normally generated by pool refresh, but useful for unit tests */ - ret->key = virXPathString("string(./key)", ctxt); + def->key = virXPathString("string(./key)", ctxt); /* Technically overridden by pool refresh, but useful for unit tests */ type = virXPathString("string(./@type)", ctxt); if (type) { - if ((ret->type = virStorageVolTypeFromString(type)) < 0) { + if ((def->type = virStorageVolTypeFromString(type)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume type '%s'"), type); - goto error; + goto cleanup; } } if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { - if (VIR_ALLOC(ret->target.backingStore) < 0) - goto error; + if (VIR_ALLOC(def->target.backingStore) < 0) + goto cleanup; - ret->target.backingStore->type = VIR_STORAGE_TYPE_FILE; + def->target.backingStore->type = VIR_STORAGE_TYPE_FILE; - ret->target.backingStore->path = backingStore; + def->target.backingStore->path = backingStore; backingStore = NULL; if (options->formatFromString) { char *format = virXPathString("string(./backingStore/format/@type)", ctxt); if (format == NULL) - ret->target.backingStore->format = options->defaultFormat; + def->target.backingStore->format = options->defaultFormat; else - ret->target.backingStore->format = (options->formatFromString)(format); + def->target.backingStore->format = (options->formatFromString)(format); - if (ret->target.backingStore->format < 0) { + if (def->target.backingStore->format < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume format type %s"), format); VIR_FREE(format); - goto error; + goto cleanup; } VIR_FREE(format); } - if (VIR_ALLOC(ret->target.backingStore->perms) < 0) - goto error; - if (virStorageDefParsePerms(ctxt, ret->target.backingStore->perms, + if (VIR_ALLOC(def->target.backingStore->perms) < 0) + goto cleanup; + if (virStorageDefParsePerms(ctxt, def->target.backingStore->perms, "./backingStore/permissions") < 0) - goto error; + goto cleanup; } capacity = virXPathString("string(./capacity)", ctxt); unit = virXPathString("string(./capacity/@unit)", ctxt); if (capacity) { - if (virStorageSize(unit, capacity, &ret->target.capacity) < 0) - goto error; + if (virStorageSize(unit, capacity, &def->target.capacity) < 0) + goto cleanup; } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) && !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && - virStorageSourceHasBacking(&ret->target))) { + virStorageSourceHasBacking(&def->target))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element")); - goto error; + goto cleanup; } VIR_FREE(unit); allocation = virXPathString("string(./allocation)", ctxt); if (allocation) { unit = virXPathString("string(./allocation/@unit)", ctxt); - if (virStorageSize(unit, allocation, &ret->target.allocation) < 0) - goto error; - ret->target.has_allocation = true; + if (virStorageSize(unit, allocation, &def->target.allocation) < 0) + goto cleanup; + def->target.has_allocation = true; } else { - ret->target.allocation = ret->target.capacity; + def->target.allocation = def->target.capacity; } - ret->target.path = virXPathString("string(./target/path)", ctxt); + def->target.path = virXPathString("string(./target/path)", ctxt); if (options->formatFromString) { char *format = virXPathString("string(./target/format/@type)", ctxt); if (format == NULL) - ret->target.format = options->defaultFormat; + def->target.format = options->defaultFormat; else - ret->target.format = (options->formatFromString)(format); + def->target.format = (options->formatFromString)(format); - if (ret->target.format < 0) { + if (def->target.format < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume format type %s"), format); VIR_FREE(format); - goto error; + goto cleanup; } VIR_FREE(format); } - if (VIR_ALLOC(ret->target.perms) < 0) - goto error; - if (virStorageDefParsePerms(ctxt, ret->target.perms, + if (VIR_ALLOC(def->target.perms) < 0) + goto cleanup; + if (virStorageDefParsePerms(ctxt, def->target.perms, "./target/permissions") < 0) - goto error; + goto cleanup; node = virXPathNode("./target/encryption", ctxt); if (node != NULL) { - ret->target.encryption = virStorageEncryptionParseNode(node, ctxt); - if (ret->target.encryption == NULL) - goto error; + def->target.encryption = virStorageEncryptionParseNode(node, ctxt); + if (def->target.encryption == NULL) + goto cleanup; } - ret->target.compat = virXPathString("string(./target/compat)", ctxt); - if (virStorageFileCheckCompat(ret->target.compat) < 0) - goto error; + def->target.compat = virXPathString("string(./target/compat)", ctxt); + if (virStorageFileCheckCompat(def->target.compat) < 0) + goto cleanup; if (virXPathNode("./target/nocow", ctxt)) - ret->target.nocow = true; + def->target.nocow = true; if (virXPathNode("./target/features", ctxt)) { if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0) - goto error; + goto cleanup; - if (!ret->target.compat && VIR_STRDUP(ret->target.compat, "1.1") < 0) - goto error; + if (!def->target.compat && VIR_STRDUP(def->target.compat, "1.1") < 0) + goto cleanup; - if (!(ret->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) - goto error; + if (!(def->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) + goto cleanup; for (i = 0; i < n; i++) { int f = virStorageFileFeatureTypeFromString((const char*)nodes[i]->name); @@ -1320,13 +1321,15 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (f < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported feature %s"), (const char*)nodes[i]->name); - goto error; + goto cleanup; } - ignore_value(virBitmapSetBit(ret->target.features, f)); + ignore_value(virBitmapSetBit(def->target.features, f)); } VIR_FREE(nodes); } + VIR_STEAL_PTR(ret, def); + cleanup: VIR_FREE(nodes); VIR_FREE(allocation); @@ -1335,11 +1338,6 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, VIR_FREE(type); VIR_FREE(backingStore); return ret; - - error: - virStorageVolDefFree(ret); - ret = NULL; - goto cleanup; } diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 3fa97bba76..b8e73864c4 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -461,5 +461,6 @@ VIR_ENUM_DECL(virStoragePartedFs); VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) VIR_DEFINE_AUTOPTR_FUNC(virStoragePoolSource, virStoragePoolSourceFree); +VIR_DEFINE_AUTOPTR_FUNC(virStorageVolDef, virStorageVolDefFree); #endif /* LIBVIRT_STORAGE_CONF_H */ diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 8458d4e95c..6e1c3c0285 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -836,7 +836,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, virStorageVolPtr volume = NULL; esxPrivate *priv = pool->conn->privateData; virStoragePoolDef poolDef; - virStorageVolDefPtr def = NULL; + VIR_AUTOPTR(virStorageVolDef) def = NULL; char *tmp; char *unescapedDatastorePath = NULL; char *unescapedDirectoryName = NULL; @@ -1024,7 +1024,6 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, virtualDiskSpec->adapterType = NULL; } - virStorageVolDefFree(def); VIR_FREE(unescapedDatastorePath); VIR_FREE(unescapedDirectoryName); VIR_FREE(unescapedDirectoryAndFileName); @@ -1054,7 +1053,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, esxPrivate *priv = pool->conn->privateData; virStoragePoolDef poolDef; char *sourceDatastorePath = NULL; - virStorageVolDefPtr def = NULL; + VIR_AUTOPTR(virStorageVolDef) def = NULL; char *tmp; char *unescapedDatastorePath = NULL; char *unescapedDirectoryName = NULL; @@ -1207,7 +1206,6 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, cleanup: VIR_FREE(sourceDatastorePath); - virStorageVolDefFree(def); VIR_FREE(unescapedDatastorePath); VIR_FREE(unescapedDirectoryName); VIR_FREE(unescapedDirectoryAndFileName); diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 4ffa08ff43..966e0b3c0f 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1952,7 +1952,7 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, { virCheckFlags(0, NULL); - virStorageVolDefPtr voldef = NULL; + VIR_AUTOPTR(virStorageVolDef) voldef = NULL; virStoragePoolDefPtr spdef = NULL; virStorageVolPtr vol = NULL; virStorageVolPtr dup_vol = NULL; @@ -2036,7 +2036,6 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, err: VIR_FREE(key); - virStorageVolDefFree(voldef); virStoragePoolDefFree(spdef); virObjectUnref(vol); return NULL; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 8c6205b081..cb9f7e4735 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -242,7 +242,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, virStorageVolDefPtr *volptr) { int ret = -1; - virStorageVolDefPtr vol = NULL; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; glfs_fd_t *fd = NULL; virStorageSourcePtr meta = NULL; char *header = NULL; @@ -278,8 +278,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, if (S_ISDIR(st->st_mode)) { vol->type = VIR_STORAGE_VOL_NETDIR; vol->target.format = VIR_STORAGE_FILE_DIR; - *volptr = vol; - vol = NULL; + VIR_STEAL_PTR(*volptr, vol); ret = 0; goto cleanup; } @@ -328,12 +327,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, vol->target.compat = meta->compat; meta->compat = NULL; - *volptr = vol; - vol = NULL; + VIR_STEAL_PTR(*volptr, vol); ret = 0; cleanup: virStorageSourceFree(meta); - virStorageVolDefFree(vol); if (fd) glfs_close(fd); VIR_FREE(header); diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index ddc150f03d..f287fbf010 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -305,22 +305,21 @@ virISCSIDirectRefreshVol(virStoragePoolObjPtr pool, char *portal) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virStorageVolDefPtr vol = NULL; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; uint32_t block_size; uint32_t nb_block; - int ret = -1; virStoragePoolObjClearVols(pool); if (virISCSIDirectTestUnitReady(iscsi, lun) < 0) - goto cleanup; + return -1; if (VIR_ALLOC(vol) < 0) - goto cleanup; + return -1; vol->type = VIR_STORAGE_VOL_NETWORK; if (virISCSIDirectGetVolumeCapacity(iscsi, lun, &block_size, &nb_block) < 0) - goto cleanup; + return -1; vol->target.capacity = block_size * nb_block; vol->target.allocation = block_size * nb_block; @@ -328,17 +327,13 @@ virISCSIDirectRefreshVol(virStoragePoolObjPtr pool, def->allocation += vol->target.allocation; if (virISCSIDirectSetVolumeAttributes(pool, vol, lun, portal) < 0) - goto cleanup; + return -1; if (virStoragePoolObjAddVol(pool, vol) < 0) - goto cleanup; - + return -1; vol = NULL; - ret = 0; - cleanup: - virStorageVolDefFree(vol); - return ret; + return 0; } static int diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index b3a49ee1b2..423f945fbc 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -46,42 +46,36 @@ virStorageBackendMpathNewVol(virStoragePoolObjPtr pool, const char *dev) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virStorageVolDefPtr vol; - int ret = -1; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; if (VIR_ALLOC(vol) < 0) - goto cleanup; + return -1; vol->type = VIR_STORAGE_VOL_BLOCK; if (virAsprintf(&(vol->name), "dm-%u", devnum) < 0) - goto cleanup; + return -1; if (virAsprintf(&vol->target.path, "/dev/%s", dev) < 0) - goto cleanup; + return -1; if (virStorageBackendUpdateVolInfo(vol, true, VIR_STORAGE_VOL_OPEN_DEFAULT, 0) < 0) { - goto cleanup; + return -1; } /* XXX should use logical unit's UUID instead */ if (VIR_STRDUP(vol->key, vol->target.path) < 0) - goto cleanup; + return -1; if (virStoragePoolObjAddVol(pool, vol) < 0) - goto cleanup; + return -1; def->capacity += vol->target.capacity; def->allocation += vol->target.allocation; - ret = 0; - - cleanup: + vol = NULL; - if (ret != 0) - virStorageVolDefFree(vol); - - return ret; + return 0; } diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index cfbce1ad19..ece04f0f2d 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -618,7 +618,7 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) } for (name = names; name < names + max_size;) { - virStorageVolDefPtr vol; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; if (STREQ(name, "")) break; @@ -626,10 +626,8 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) if (VIR_ALLOC(vol) < 0) goto cleanup; - if (VIR_STRDUP(vol->name, name) < 0) { - VIR_FREE(vol); + if (VIR_STRDUP(vol->name, name) < 0) goto cleanup; - } name += strlen(name) + 1; @@ -648,15 +646,14 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) if (r == -ENOENT || r == -ETIMEDOUT) continue; - virStorageVolDefFree(vol); goto cleanup; } if (virStoragePoolObjAddVol(pool, vol) < 0) { - virStorageVolDefFree(vol); virStoragePoolObjClearVols(pool); goto cleanup; } + vol = NULL; } VIR_DEBUG("Found %zu images in RBD pool %s", diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index b6f424bea1..7cba6e02aa 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -110,30 +110,27 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, static int virStorageBackendSheepdogAddVolume(virStoragePoolObjPtr pool, const char *diskInfo) { - virStorageVolDefPtr vol = NULL; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; if (diskInfo == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing disk info when adding volume")); - goto error; + return -1; } if (VIR_ALLOC(vol) < 0 || VIR_STRDUP(vol->name, diskInfo) < 0) - goto error; + return -1; vol->type = VIR_STORAGE_VOL_NETWORK; if (virStorageBackendSheepdogRefreshVol(pool, vol) < 0) - goto error; + return -1; if (virStoragePoolObjAddVol(pool, vol) < 0) - goto error; + return -1; + vol = NULL; return 0; - - error: - virStorageVolDefFree(vol); - return -1; } static int diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 878a40cac5..48b4c5127f 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1820,7 +1820,7 @@ storageVolCreateXML(virStoragePoolPtr pool, virStoragePoolObjPtr obj; virStoragePoolDefPtr def; virStorageBackendPtr backend; - virStorageVolDefPtr voldef = NULL; + VIR_AUTOPTR(virStorageVolDef) voldef = NULL; virStorageVolPtr vol = NULL, newvol = NULL; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA, NULL); @@ -1941,7 +1941,6 @@ storageVolCreateXML(virStoragePoolPtr pool, cleanup: virObjectUnref(newvol); - virStorageVolDefFree(voldef); virStoragePoolObjEndAPI(&obj); return vol; } @@ -1957,7 +1956,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, virStoragePoolObjPtr objsrc = NULL; virStorageBackendPtr backend; virStorageVolDefPtr voldefsrc = NULL; - virStorageVolDefPtr voldef = NULL; + VIR_AUTOPTR(virStorageVolDef) voldef = NULL; virStorageVolDefPtr shadowvol = NULL; virStorageVolPtr newvol = NULL; virStorageVolPtr vol = NULL; @@ -2134,7 +2133,6 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, cleanup: virObjectUnref(newvol); - virStorageVolDefFree(voldef); VIR_FREE(shadowvol); virStoragePoolObjEndAPI(&obj); virStoragePoolObjEndAPI(&objsrc); diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 95e05d950b..158cf3adb6 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3672,7 +3672,7 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) struct dirent *ent; struct statvfs sb; struct stat statbuf; - virStorageVolDefPtr vol = NULL; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; virStorageSourcePtr target = NULL; int direrr; int fd = -1, ret = -1; @@ -3767,7 +3767,6 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) cleanup: VIR_DIR_CLOSE(dir); VIR_FORCE_CLOSE(fd); - virStorageVolDefFree(vol); virStorageSourceFree(target); if (ret < 0) virStoragePoolObjClearVols(pool); @@ -3816,7 +3815,7 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, const char *dev) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virStorageVolDefPtr vol = NULL; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; char *devpath = NULL; int retval = -1; @@ -3898,7 +3897,6 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, retval = 0; cleanup: - virStorageVolDefFree(vol); VIR_FREE(devpath); return retval; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 743d03ae8b..44ae711b5a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1025,7 +1025,7 @@ testOpenVolumesForPool(const char *file, size_t i; int num, ret = -1; xmlNodePtr *nodes = NULL; - virStorageVolDefPtr volDef = NULL; + VIR_AUTOPTR(virStorageVolDef) volDef = NULL; /* Find storage volumes */ if (virAsprintf(&vol_xpath, "/node/pool[%d]/volume", objidx) < 0) @@ -1064,7 +1064,6 @@ testOpenVolumesForPool(const char *file, ret = 0; error: - virStorageVolDefFree(volDef); VIR_FREE(nodes); return ret; } @@ -5053,7 +5052,7 @@ testStorageVolCreateXML(virStoragePoolPtr pool, testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr obj; virStoragePoolDefPtr def; - virStorageVolDefPtr privvol = NULL; + VIR_AUTOPTR(virStorageVolDef) privvol = NULL; virStorageVolPtr ret = NULL; virCheckFlags(0, NULL); @@ -5098,7 +5097,6 @@ testStorageVolCreateXML(virStoragePoolPtr pool, privvol = NULL; cleanup: - virStorageVolDefFree(privvol); virStoragePoolObjEndAPI(&obj); return ret; } @@ -5113,7 +5111,8 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr obj; virStoragePoolDefPtr def; - virStorageVolDefPtr privvol = NULL, origvol = NULL; + VIR_AUTOPTR(virStorageVolDef) privvol = NULL; + virStorageVolDefPtr origvol = NULL; virStorageVolPtr ret = NULL; virCheckFlags(0, NULL); @@ -5166,7 +5165,6 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, privvol = NULL; cleanup: - virStorageVolDefFree(privvol); virStoragePoolObjEndAPI(&obj); return ret; } diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c index 7047e54084..4ca342a449 100644 --- a/src/vbox/vbox_storage.c +++ b/src/vbox/vbox_storage.c @@ -401,7 +401,7 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool, const char *xml, unsigned int flags) { vboxDriverPtr data = pool->conn->privateData; - virStorageVolDefPtr def = NULL; + VIR_AUTOPTR(virStorageVolDef) def = NULL; PRUnichar *hddFormatUtf16 = NULL; PRUnichar *hddNameUtf16 = NULL; virStoragePoolDef poolDef; @@ -501,7 +501,6 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool, VBOX_RELEASE(progress); VBOX_UTF16_FREE(hddFormatUtf16); VBOX_UTF16_FREE(hddNameUtf16); - virStorageVolDefFree(def); return ret; } diff --git a/tests/storagebackendsheepdogtest.c b/tests/storagebackendsheepdogtest.c index 616af22d73..540f89c3ab 100644 --- a/tests/storagebackendsheepdogtest.c +++ b/tests/storagebackendsheepdogtest.c @@ -94,7 +94,7 @@ test_vdi_list_parser(const void *opaque) int ret = -1; char *output = NULL; virStoragePoolDefPtr pool = NULL; - virStorageVolDefPtr vol = NULL; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; if (!(pool = virStoragePoolDefParseFile(data->poolxml))) goto cleanup; @@ -121,7 +121,6 @@ test_vdi_list_parser(const void *opaque) cleanup: VIR_FREE(output); virStoragePoolDefFree(pool); - virStorageVolDefFree(vol); return ret; } diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index bc2da37410..8e19f10b73 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -48,7 +48,8 @@ testCompareXMLToArgvFiles(bool shouldFail, virCommandPtr cmd = NULL; - virStorageVolDefPtr vol = NULL, inputvol = NULL; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; + VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; virStoragePoolDefPtr def = NULL; virStoragePoolDefPtr inputpool = NULL; virStoragePoolObjPtr obj = NULL; @@ -139,8 +140,6 @@ testCompareXMLToArgvFiles(bool shouldFail, cleanup: virStoragePoolDefFree(inputpool); - virStorageVolDefFree(vol); - virStorageVolDefFree(inputvol); virCommandFree(cmd); VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&obj); diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index 040bbc7585..95e205a0ab 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -20,7 +20,7 @@ testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, char *actual = NULL; int ret = -1; virStoragePoolDefPtr pool = NULL; - virStorageVolDefPtr dev = NULL; + VIR_AUTOPTR(virStorageVolDef) dev = NULL; if (!(pool = virStoragePoolDefParseFile(poolxml))) goto fail; @@ -39,7 +39,6 @@ testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, fail: VIR_FREE(actual); virStoragePoolDefFree(pool); - virStorageVolDefFree(dev); return ret; } -- 2.20.1

Let's make use of the auto __cleanup capabilities cleaning up any now unnecessary goto paths. For virStoragePoolDefParseXML use the @def/@ret similarly as other methods. For storagePoolDefineXML make it clearer and use @objNewDef after adding @newDef to the object. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 3 +- src/conf/storage_conf.c | 114 ++++++++++++++--------------- src/conf/storage_conf.h | 1 + src/conf/virstorageobj.c | 27 +++---- src/phyp/phyp_driver.c | 3 +- src/storage/storage_driver.c | 14 ++-- src/test/test_driver.c | 6 +- tests/storagebackendsheepdogtest.c | 6 +- tests/storagepoolxml2argvtest.c | 21 ++++-- tests/storagepoolxml2xmltest.c | 3 +- tests/storagevolxml2argvtest.c | 16 ++-- tests/storagevolxml2xmltest.c | 3 +- 12 files changed, 103 insertions(+), 114 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5699a60549..ee0edff4b2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30996,7 +30996,7 @@ int virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) { virConnectPtr conn = NULL; - virStoragePoolDefPtr pooldef = NULL; + VIR_AUTOPTR(virStoragePoolDef) pooldef = NULL; virStoragePoolPtr pool = NULL; virStorageVolPtr vol = NULL; char *poolxml = NULL; @@ -31160,7 +31160,6 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) virObjectUnref(pool); virObjectUnref(vol); VIR_FREE(poolxml); - virStoragePoolDefFree(pooldef); return ret; } diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 83ca379217..4fb5fb9f57 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -737,159 +737,157 @@ virStoragePoolDefPtr virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) { virStoragePoolOptionsPtr options; - virStoragePoolDefPtr ret; + VIR_AUTOPTR(virStoragePoolDef) def = NULL; + virStoragePoolDefPtr ret = NULL; xmlNodePtr source_node; char *type = NULL; char *uuid = NULL; char *target_path = NULL; - if (VIR_ALLOC(ret) < 0) + if (VIR_ALLOC(def) < 0) return NULL; type = virXPathString("string(./@type)", ctxt); if (type == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("storage pool missing type attribute")); - goto error; + goto cleanup; } - if ((ret->type = virStoragePoolTypeFromString(type)) < 0) { + if ((def->type = virStoragePoolTypeFromString(type)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown storage pool type %s"), type); - goto error; + goto cleanup; } - if ((options = virStoragePoolOptionsForPoolType(ret->type)) == NULL) - goto error; + if ((options = virStoragePoolOptionsForPoolType(def->type)) == NULL) + goto cleanup; source_node = virXPathNode("./source", ctxt); if (source_node) { - if (virStoragePoolDefParseSource(ctxt, &ret->source, ret->type, + if (virStoragePoolDefParseSource(ctxt, &def->source, def->type, source_node) < 0) - goto error; + goto cleanup; } else { if (options->formatFromString) - ret->source.format = options->defaultFormat; + def->source.format = options->defaultFormat; } - ret->name = virXPathString("string(./name)", ctxt); - if (ret->name == NULL && + def->name = virXPathString("string(./name)", ctxt); + if (def->name == NULL && options->flags & VIR_STORAGE_POOL_SOURCE_NAME && - VIR_STRDUP(ret->name, ret->source.name) < 0) - goto error; - if (ret->name == NULL) { + VIR_STRDUP(def->name, def->source.name) < 0) + goto cleanup; + if (def->name == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing pool source name element")); - goto error; + goto cleanup; } - if (strchr(ret->name, '/')) { + if (strchr(def->name, '/')) { virReportError(VIR_ERR_XML_ERROR, - _("name %s cannot contain '/'"), ret->name); - goto error; + _("name %s cannot contain '/'"), def->name); + goto cleanup; } uuid = virXPathString("string(./uuid)", ctxt); if (uuid == NULL) { - if (virUUIDGenerate(ret->uuid) < 0) { + if (virUUIDGenerate(def->uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to generate uuid")); - goto error; + goto cleanup; } } else { - if (virUUIDParse(uuid, ret->uuid) < 0) { + if (virUUIDParse(uuid, def->uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("malformed uuid element")); - goto error; + goto cleanup; } } if (options->flags & VIR_STORAGE_POOL_SOURCE_HOST) { - if (!ret->source.nhost) { + if (!def->source.nhost) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool source host name")); - goto error; + goto cleanup; } } if (options->flags & VIR_STORAGE_POOL_SOURCE_DIR) { - if (!ret->source.dir) { + if (!def->source.dir) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool source path")); - goto error; + goto cleanup; } } if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) { - if (ret->source.name == NULL) { + if (def->source.name == NULL) { /* source name defaults to pool name */ - if (VIR_STRDUP(ret->source.name, ret->name) < 0) - goto error; + if (VIR_STRDUP(def->source.name, def->name) < 0) + goto cleanup; } } if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) && - (virStorageAdapterValidate(&ret->source.adapter)) < 0) - goto error; + (virStorageAdapterValidate(&def->source.adapter)) < 0) + goto cleanup; /* If DEVICE is the only source type, then its required */ if (options->flags == VIR_STORAGE_POOL_SOURCE_DEVICE) { - if (!ret->source.ndevice) { + if (!def->source.ndevice) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool source device name")); - goto error; + goto cleanup; } } /* When we are working with a virtual disk we can skip the target * path and permissions */ if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) { - if (ret->type == VIR_STORAGE_POOL_LOGICAL) { - if (virAsprintf(&target_path, "/dev/%s", ret->source.name) < 0) - goto error; - } else if (ret->type == VIR_STORAGE_POOL_ZFS) { - if (virAsprintf(&target_path, "/dev/zvol/%s", ret->source.name) < 0) - goto error; + if (def->type == VIR_STORAGE_POOL_LOGICAL) { + if (virAsprintf(&target_path, "/dev/%s", def->source.name) < 0) + goto cleanup; + } else if (def->type == VIR_STORAGE_POOL_ZFS) { + if (virAsprintf(&target_path, "/dev/zvol/%s", def->source.name) < 0) + goto cleanup; } else { target_path = virXPathString("string(./target/path)", ctxt); if (!target_path) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool target path")); - goto error; + goto cleanup; } } - ret->target.path = virFileSanitizePath(target_path); - if (!ret->target.path) - goto error; + def->target.path = virFileSanitizePath(target_path); + if (!def->target.path) + goto cleanup; - if (virStorageDefParsePerms(ctxt, &ret->target.perms, + if (virStorageDefParsePerms(ctxt, &def->target.perms, "./target/permissions") < 0) - goto error; + goto cleanup; } - if (ret->type == VIR_STORAGE_POOL_ISCSI_DIRECT && - !ret->source.initiator.iqn) { + if (def->type == VIR_STORAGE_POOL_ISCSI_DIRECT && + !def->source.initiator.iqn) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("missing initiator IQN")); - goto error; + goto cleanup; } /* Make a copy of all the callback pointers here for easier use, * especially during the virStoragePoolSourceClear method */ - ret->ns = options->ns; - if (ret->ns.parse && - (ret->ns.parse)(ctxt, &ret->namespaceData) < 0) - goto error; + def->ns = options->ns; + if (def->ns.parse && + (def->ns.parse)(ctxt, &def->namespaceData) < 0) + goto cleanup; + + VIR_STEAL_PTR(ret, def); cleanup: VIR_FREE(uuid); VIR_FREE(type); VIR_FREE(target_path); return ret; - - error: - virStoragePoolDefFree(ret); - ret = NULL; - goto cleanup; } diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index b8e73864c4..daf6f9b68c 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -461,6 +461,7 @@ VIR_ENUM_DECL(virStoragePartedFs); VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) VIR_DEFINE_AUTOPTR_FUNC(virStoragePoolSource, virStoragePoolSourceFree); +VIR_DEFINE_AUTOPTR_FUNC(virStoragePoolDef, virStoragePoolDefFree); VIR_DEFINE_AUTOPTR_FUNC(virStorageVolDef, virStorageVolDefFree); #endif /* LIBVIRT_STORAGE_CONF_H */ diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 7005de3c24..3a2f3fa77b 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1579,7 +1579,7 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, const char *path, const char *autostartLink) { - virStoragePoolDefPtr def; + VIR_AUTOPTR(virStoragePoolDef) def = NULL; virStoragePoolObjPtr obj; if (!(def = virStoragePoolDefParseFile(path))) @@ -1590,14 +1590,12 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, _("Storage pool config filename '%s' does " "not match pool name '%s'"), path, def->name); - virStoragePoolDefFree(def); return NULL; } - if (!(obj = virStoragePoolObjAssignDef(pools, def, false))) { - virStoragePoolDefFree(def); + if (!(obj = virStoragePoolObjAssignDef(pools, def, false))) return NULL; - } + def = NULL; VIR_FREE(obj->configFile); /* for driver reload */ if (VIR_STRDUP(obj->configFile, path) < 0) { @@ -1625,39 +1623,40 @@ virStoragePoolObjLoadState(virStoragePoolObjListPtr pools, const char *name) { char *stateFile = NULL; - virStoragePoolDefPtr def = NULL; + VIR_AUTOPTR(virStoragePoolDef) def = NULL; virStoragePoolObjPtr obj = NULL; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; xmlNodePtr node = NULL; if (!(stateFile = virFileBuildPath(stateDir, name, ".xml"))) - goto error; + return NULL; if (!(xml = virXMLParseCtxt(stateFile, NULL, _("(pool state)"), &ctxt))) - goto error; + goto cleanup; if (!(node = virXPathNode("//pool", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not find any 'pool' element in state file")); - goto error; + goto cleanup; } ctxt->node = node; if (!(def = virStoragePoolDefParseXML(ctxt))) - goto error; + goto cleanup; if (STRNEQ(name, def->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Storage pool state file '%s' does not match " "pool name '%s'"), stateFile, def->name); - goto error; + goto cleanup; } /* create the object */ if (!(obj = virStoragePoolObjAssignDef(pools, def, true))) - goto error; + goto cleanup; + def = NULL; /* XXX: future handling of some additional useful status data, * for now, if a status file for a pool exists, the pool will be marked @@ -1671,10 +1670,6 @@ virStoragePoolObjLoadState(virStoragePoolObjListPtr pools, xmlFreeDoc(xml); xmlXPathFreeContext(ctxt); return obj; - - error: - virStoragePoolDefFree(def); - goto cleanup; } diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 966e0b3c0f..2ebf4f3cfd 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1953,7 +1953,7 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, virCheckFlags(0, NULL); VIR_AUTOPTR(virStorageVolDef) voldef = NULL; - virStoragePoolDefPtr spdef = NULL; + VIR_AUTOPTR(virStoragePoolDef) spdef = NULL; virStorageVolPtr vol = NULL; virStorageVolPtr dup_vol = NULL; char *key = NULL; @@ -2036,7 +2036,6 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, err: VIR_FREE(key); - virStoragePoolDefFree(spdef); virObjectUnref(vol); return NULL; } diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 48b4c5127f..dcdd52bbbf 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -690,7 +690,7 @@ storagePoolCreateXML(virConnectPtr conn, const char *xml, unsigned int flags) { - virStoragePoolDefPtr newDef; + VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; virStoragePoolObjPtr obj = NULL; virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; @@ -762,7 +762,6 @@ storagePoolCreateXML(virConnectPtr conn, cleanup: VIR_FREE(stateFile); - virStoragePoolDefFree(newDef); virObjectEventStateQueue(driver->storageEventState, event); virStoragePoolObjEndAPI(&obj); return pool; @@ -779,9 +778,10 @@ storagePoolDefineXML(virConnectPtr conn, const char *xml, unsigned int flags) { - virStoragePoolDefPtr newDef; + VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; virStoragePoolObjPtr obj = NULL; virStoragePoolDefPtr def; + virStoragePoolDefPtr objNewDef; virStoragePoolPtr pool = NULL; virObjectEventPtr event = NULL; @@ -801,14 +801,14 @@ storagePoolDefineXML(virConnectPtr conn, if (!(obj = virStoragePoolObjAssignDef(driver->pools, newDef, false))) goto cleanup; - newDef = virStoragePoolObjGetNewDef(obj); + newDef = NULL; + objNewDef = virStoragePoolObjGetNewDef(obj); def = virStoragePoolObjGetDef(obj); - if (virStoragePoolObjSaveDef(driver, obj, newDef ? newDef : def) < 0) { + if (virStoragePoolObjSaveDef(driver, obj, objNewDef ? objNewDef : def) < 0) { virStoragePoolObjRemove(driver->pools, obj); virObjectUnref(obj); obj = NULL; - newDef = NULL; goto cleanup; } @@ -818,11 +818,9 @@ storagePoolDefineXML(virConnectPtr conn, VIR_INFO("Defining storage pool '%s'", def->name); pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); - newDef = NULL; cleanup: virObjectEventStateQueue(driver->storageEventState, event); - virStoragePoolDefFree(newDef); virStoragePoolObjEndAPI(&obj); return pool; } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 44ae711b5a..ff773f39b0 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4474,7 +4474,7 @@ testStoragePoolCreateXML(virConnectPtr conn, unsigned int flags) { testDriverPtr privconn = conn->privateData; - virStoragePoolDefPtr newDef; + VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; virStoragePoolObjPtr obj = NULL; virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; @@ -4527,7 +4527,6 @@ testStoragePoolCreateXML(virConnectPtr conn, pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); cleanup: - virStoragePoolDefFree(newDef); virObjectEventStateQueue(privconn->eventState, event); virStoragePoolObjEndAPI(&obj); virObjectUnlock(privconn); @@ -4541,7 +4540,7 @@ testStoragePoolDefineXML(virConnectPtr conn, unsigned int flags) { testDriverPtr privconn = conn->privateData; - virStoragePoolDefPtr newDef; + VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; virStoragePoolObjPtr obj = NULL; virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; @@ -4576,7 +4575,6 @@ testStoragePoolDefineXML(virConnectPtr conn, pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); cleanup: - virStoragePoolDefFree(newDef); virObjectEventStateQueue(privconn->eventState, event); virStoragePoolObjEndAPI(&obj); virObjectUnlock(privconn); diff --git a/tests/storagebackendsheepdogtest.c b/tests/storagebackendsheepdogtest.c index 540f89c3ab..03ddf76d65 100644 --- a/tests/storagebackendsheepdogtest.c +++ b/tests/storagebackendsheepdogtest.c @@ -59,7 +59,7 @@ test_node_info_parser(const void *opaque) collie_test test = data->data; int ret = -1; char *output = NULL; - virStoragePoolDefPtr pool = NULL; + VIR_AUTOPTR(virStoragePoolDef) pool = NULL; if (!(pool = virStoragePoolDefParseFile(data->poolxml))) goto cleanup; @@ -82,7 +82,6 @@ test_node_info_parser(const void *opaque) cleanup: VIR_FREE(output); - virStoragePoolDefFree(pool); return ret; } @@ -93,7 +92,7 @@ test_vdi_list_parser(const void *opaque) collie_test test = data->data; int ret = -1; char *output = NULL; - virStoragePoolDefPtr pool = NULL; + VIR_AUTOPTR(virStoragePoolDef) pool = NULL; VIR_AUTOPTR(virStorageVolDef) vol = NULL; if (!(pool = virStoragePoolDefParseFile(data->poolxml))) @@ -120,7 +119,6 @@ test_vdi_list_parser(const void *opaque) cleanup: VIR_FREE(output); - virStoragePoolDefFree(pool); return ret; } diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c index e7f42dc0f8..2dd87ab731 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -24,30 +24,35 @@ testCompareXMLToArgvFiles(bool shouldFail, { VIR_AUTOFREE(char *) actualCmdline = NULL; VIR_AUTOFREE(char *) src = NULL; + VIR_AUTOPTR(virStoragePoolDef) def = NULL; + int defType; int ret = -1; virCommandPtr cmd = NULL; - virStoragePoolDefPtr def = NULL; virStoragePoolObjPtr pool = NULL; + virStoragePoolDefPtr objDef; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; + defType = def->type; - switch ((virStoragePoolType)def->type) { + switch ((virStoragePoolType)defType) { 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); - virStoragePoolDefFree(def); + VIR_TEST_DEBUG("pool type %d alloc pool obj fails\n", defType); goto cleanup; } virStoragePoolObjSetDef(pool, def); + def = NULL; + objDef = virStoragePoolObjGetDef(pool); if (!(src = virStorageBackendFileSystemGetPoolSource(pool))) { - VIR_TEST_DEBUG("pool type %d has no pool source\n", def->type); + VIR_TEST_DEBUG("pool type %d has no pool source\n", defType); goto cleanup; } - cmd = virStorageBackendFileSystemMountCmd(MOUNT, def, src); + cmd = virStorageBackendFileSystemMountCmd(MOUNT, objDef, src); break; case VIR_STORAGE_POOL_LOGICAL: @@ -67,12 +72,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 %d has no xml2argv test\n", defType); 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 %d failed to get commandline\n", defType); goto cleanup; } diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index acb15f3a2c..c8d5c41cd4 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -20,7 +20,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { char *actual = NULL; int ret = -1; - virStoragePoolDefPtr dev = NULL; + VIR_AUTOPTR(virStoragePoolDef) dev = NULL; if (!(dev = virStoragePoolDefParseFile(inxml))) goto fail; @@ -35,7 +35,6 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml) fail: VIR_FREE(actual); - virStoragePoolDefFree(dev); return ret; } diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 8e19f10b73..2acbf567ca 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -50,18 +50,19 @@ testCompareXMLToArgvFiles(bool shouldFail, VIR_AUTOPTR(virStorageVolDef) vol = NULL; VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; - virStoragePoolDefPtr def = NULL; - virStoragePoolDefPtr inputpool = NULL; + VIR_AUTOPTR(virStoragePoolDef) inputpool = NULL; + VIR_AUTOPTR(virStoragePoolDef) def = NULL; virStoragePoolObjPtr obj = NULL; + virStoragePoolDefPtr objDef; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; - if (!(obj = virStoragePoolObjNew())) { - virStoragePoolDefFree(def); + if (!(obj = virStoragePoolObjNew())) goto cleanup; - } virStoragePoolObjSetDef(obj, def); + def = NULL; + objDef = virStoragePoolObjGetDef(obj); if (inputpoolxml) { if (!(inputpool = virStoragePoolDefParseFile(inputpoolxml))) @@ -71,14 +72,14 @@ testCompareXMLToArgvFiles(bool shouldFail, if (inputvolxml) parse_flags |= VIR_VOL_XML_PARSE_NO_CAPACITY; - if (!(vol = virStorageVolDefParseFile(def, volxml, parse_flags))) + if (!(vol = virStorageVolDefParseFile(objDef, volxml, parse_flags))) goto cleanup; if (inputvolxml && !(inputvol = virStorageVolDefParseFile(inputpool, inputvolxml, 0))) goto cleanup; - testSetVolumeType(vol, def); + testSetVolumeType(vol, objDef); testSetVolumeType(inputvol, inputpool); /* Using an input file for encryption requires a multi-step process @@ -139,7 +140,6 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: - virStoragePoolDefFree(inputpool); virCommandFree(cmd); VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&obj); diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index 95e205a0ab..cb78bd5b28 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -19,7 +19,7 @@ testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, { char *actual = NULL; int ret = -1; - virStoragePoolDefPtr pool = NULL; + VIR_AUTOPTR(virStoragePoolDef) pool = NULL; VIR_AUTOPTR(virStorageVolDef) dev = NULL; if (!(pool = virStoragePoolDefParseFile(poolxml))) @@ -38,7 +38,6 @@ testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, fail: VIR_FREE(actual); - virStoragePoolDefFree(pool); return ret; } -- 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> --- src/storage/storage_backend_disk.c | 85 +++++++++------------ src/storage/storage_backend_fs.c | 39 +++------- src/storage/storage_backend_logical.c | 101 +++++++------------------ src/storage/storage_backend_sheepdog.c | 50 +++++------- src/storage/storage_backend_vstorage.c | 14 +--- src/storage/storage_backend_zfs.c | 47 +++--------- src/storage/storage_driver.c | 3 +- src/storage/storage_util.c | 34 +++------ src/util/virstoragefile.c | 63 +++++++-------- tests/storagepoolxml2argvtest.c | 3 +- tests/storagevolxml2argvtest.c | 6 +- tests/virstoragetest.c | 6 +- 12 files changed, 148 insertions(+), 303 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 061c494b7d..230cf44b97 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -356,7 +356,7 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; struct virStorageBackendDiskPoolVolData cbdata = { .pool = pool, .vol = vol, @@ -392,7 +392,6 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, 6, virStorageBackendDiskMakeVol, &cbdata); - virCommandFree(cmd); VIR_FREE(parthelper_path); return ret; } @@ -421,7 +420,7 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", @@ -438,7 +437,6 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) 3, virStorageBackendDiskMakePoolGeometry, pool); - virCommandFree(cmd); VIR_FREE(parthelper_path); return ret; } @@ -502,51 +500,40 @@ virStorageBackendDiskBuildPool(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int format = def->source.format; const char *fmt; - bool ok_to_mklabel = false; - int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | - VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); + VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, -1); - VIR_EXCLUSIVE_FLAGS_GOTO(VIR_STORAGE_POOL_BUILD_OVERWRITE, - VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, - error); + VIR_EXCLUSIVE_FLAGS_RET(VIR_STORAGE_POOL_BUILD_OVERWRITE, + VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, + -1); fmt = virStoragePoolFormatDiskTypeToString(format); - if (flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) { - ok_to_mklabel = true; - } else { - if (virStorageBackendDeviceIsEmpty(def->source.devices[0].path, - fmt, true)) - ok_to_mklabel = true; - } + if (!(flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) && + !(virStorageBackendDeviceIsEmpty(def->source.devices[0].path, + fmt, true))) + return -1; - if (ok_to_mklabel) { - if (virStorageBackendZeroPartitionTable(def->source.devices[0].path, - 1024 * 1024) < 0) - goto error; + if (virStorageBackendZeroPartitionTable(def->source.devices[0].path, + 1024 * 1024) < 0) + return -1; - /* eg parted /dev/sda mklabel --script msdos */ - if (format == VIR_STORAGE_POOL_DISK_UNKNOWN) - format = def->source.format = VIR_STORAGE_POOL_DISK_DOS; - if (format == VIR_STORAGE_POOL_DISK_DOS) - fmt = "msdos"; - else - fmt = virStoragePoolFormatDiskTypeToString(format); - - cmd = virCommandNewArgList(PARTED, - def->source.devices[0].path, - "mklabel", - "--script", - fmt, - NULL); - ret = virCommandRun(cmd, NULL); - } + /* eg parted /dev/sda mklabel --script msdos */ + if (format == VIR_STORAGE_POOL_DISK_UNKNOWN) + format = def->source.format = VIR_STORAGE_POOL_DISK_DOS; + if (format == VIR_STORAGE_POOL_DISK_DOS) + fmt = "msdos"; + else + fmt = virStoragePoolFormatDiskTypeToString(format); - error: - virCommandFree(cmd); - return ret; + cmd = virCommandNewArgList(PARTED, + def->source.devices[0].path, + "mklabel", + "--script", + fmt, + NULL); + return virCommandRun(cmd, NULL); } @@ -787,7 +774,7 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src_path = def->source.devices[0].path; char *srcname = last_component(src_path); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; bool isDevMapperDevice; int rc = -1; @@ -862,7 +849,6 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, rc = 0; cleanup: VIR_FREE(devpath); - virCommandFree(cmd); return rc; } @@ -876,11 +862,13 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, unsigned long long startOffset = 0, endOffset = 0; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virErrorPtr save_err; - virCommandPtr cmd = virCommandNewArgList(PARTED, - def->source.devices[0].path, - "mkpart", - "--script", - NULL); + VIR_AUTOPTR(virCommand) cmd = NULL; + + cmd = virCommandNewArgList(PARTED, + def->source.devices[0].path, + "mkpart", + "--script", + NULL); if (vol->target.encryption && vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { @@ -934,7 +922,6 @@ virStorageBackendDiskCreateVol(virStoragePoolObjPtr pool, cleanup: VIR_FREE(partFormat); - virCommandFree(cmd); return res; error: diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 06c76fde4f..7d05ceeeb8 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -95,8 +95,6 @@ virStorageBackendFileSystemNetFindPoolSourcesFunc(char **const groups, static int virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) { - int ret = -1; - /* * # showmount --no-headers -e HOSTNAME * /tmp * @@ -112,7 +110,7 @@ virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) 1 }; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(SHOWMOUNT, "--no-headers", @@ -120,16 +118,9 @@ virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) state->host, NULL); - if (virCommandRunRegex(cmd, 1, regexes, vars, - virStorageBackendFileSystemNetFindPoolSourcesFunc, - state, NULL, NULL) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRunRegex(cmd, 1, regexes, vars, + virStorageBackendFileSystemNetFindPoolSourcesFunc, + state, NULL, NULL); } @@ -309,7 +300,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src = NULL; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; int rc; @@ -334,7 +325,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(src); return ret; } @@ -376,8 +366,7 @@ static int virStorageBackendFileSystemStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; int rc; if (virStorageBackendFileSystemIsValid(pool) < 0) @@ -388,13 +377,7 @@ virStorageBackendFileSystemStop(virStoragePoolObjPtr pool) return rc; cmd = virCommandNewArgList(UMOUNT, def->target.path, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } #endif /* WITH_STORAGE_FS */ @@ -432,8 +415,7 @@ static int virStorageBackendExecuteMKFS(const char *device, const char *format) { - int ret = 0; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(MKFS, "-t", format, NULL); @@ -456,11 +438,10 @@ virStorageBackendExecuteMKFS(const char *device, _("Failed to make filesystem of " "type '%s' on device '%s'"), format, device); - ret = -1; + return -1; } - virCommandFree(cmd); - return ret; + return 0; } #else /* #ifdef MKFS */ static int diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index fd95bd0d48..e0ddc962b1 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -48,13 +48,11 @@ static int virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool, bool on) { - int ret; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = virStorageBackendLogicalChangeCmd(VGCHANGE, def, on); + VIR_AUTOPTR(virCommand) cmd = NULL; - ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); - return ret; + cmd = virStorageBackendLogicalChangeCmd(VGCHANGE, def, on); + return virCommandRun(cmd, NULL); } @@ -67,11 +65,11 @@ virStorageBackendLogicalSetActive(virStoragePoolObjPtr pool, static void virStorageBackendLogicalRemoveDevice(const char *path) { - virCommandPtr cmd = virCommandNewArgList(PVREMOVE, path, NULL); + VIR_AUTOPTR(virCommand) cmd = NULL; + cmd = virCommandNewArgList(PVREMOVE, path, NULL); if (virCommandRun(cmd, NULL) < 0) VIR_INFO("Failed to pvremove logical device '%s'", path); - virCommandFree(cmd); } @@ -85,8 +83,7 @@ virStorageBackendLogicalRemoveDevice(const char *path) static int virStorageBackendLogicalInitializeDevice(const char *path) { - int ret = -1; - virCommandPtr pvcmd = NULL; + VIR_AUTOPTR(virCommand) pvcmd = NULL; /* * LVM requires that the first sector is blanked if using @@ -101,15 +98,7 @@ virStorageBackendLogicalInitializeDevice(const char *path) * clever enough todo this for us :-( */ pvcmd = virCommandNewArgList(PVCREATE, path, NULL); - if (virCommandRun(pvcmd, NULL) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virCommandFree(pvcmd); - - return ret; + return virCommandRun(pvcmd, NULL); } @@ -426,8 +415,7 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, int vars[] = { VIR_STORAGE_VOL_LOGICAL_REGEX_COUNT }; - int ret = -1; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); struct virStorageBackendLogicalPoolVolData cbdata = { .pool = pool, @@ -444,20 +432,9 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, "lv_name,origin,uuid,devices,segtype,stripes,seg_size,vg_extent_size,size,lv_attr", def->source.name, NULL); - if (virCommandRunRegex(cmd, - 1, - regexes, - vars, - virStorageBackendLogicalMakeVol, - &cbdata, - "lvs", - NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRunRegex(cmd, 1, regexes, vars, + virStorageBackendLogicalMakeVol, + &cbdata, "lvs", NULL); } static int @@ -550,8 +527,7 @@ virStorageBackendLogicalGetPoolSources(virStoragePoolSourceListPtr sourceList) int vars[] = { 2 }; - virCommandPtr cmd; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; /* * NOTE: ignoring errors here; this is just to "touch" any logical volumes @@ -567,15 +543,9 @@ virStorageBackendLogicalGetPoolSources(virStoragePoolSourceListPtr sourceList) "--noheadings", "-o", "pv_name,vg_name", NULL, NULL); - if (virCommandRunRegex(cmd, 1, regexes, vars, - virStorageBackendLogicalFindPoolSourcesFunc, - sourceList, "pvs", NULL) < 0) - goto cleanup; - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRunRegex(cmd, 1, regexes, vars, + virStorageBackendLogicalFindPoolSourcesFunc, + sourceList, "pvs", NULL); } @@ -737,7 +707,7 @@ virStorageBackendLogicalBuildPool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr vgcmd = NULL; + VIR_AUTOPTR(virCommand) vgcmd = NULL; int ret = -1; size_t i = 0; @@ -773,8 +743,6 @@ virStorageBackendLogicalBuildPool(virStoragePoolObjPtr pool, ret = 0; cleanup: - virCommandFree(vgcmd); - /* On any failure, run through the devices that had pvcreate run in * in order to run pvremove on the device; otherwise, subsequent build * will fail if a pvcreate had been run already. */ @@ -805,7 +773,7 @@ virStorageBackendLogicalRefreshPool(virStoragePoolObjPtr pool) 2 }; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; virWaitForDevices(); @@ -838,7 +806,6 @@ virStorageBackendLogicalRefreshPool(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); if (ret < 0) virStoragePoolObjClearVols(pool); return ret; @@ -863,9 +830,8 @@ virStorageBackendLogicalDeletePool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; size_t i; - int ret = -1; virCheckFlags(0, -1); @@ -874,17 +840,13 @@ virStorageBackendLogicalDeletePool(virStoragePoolObjPtr pool, "-f", def->source.name, NULL); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; /* now remove the pv devices and clear them out */ for (i = 0; i < def->source.ndevice; i++) virStorageBackendLogicalRemoveDevice(def->source.devices[i].path); - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return 0; } @@ -893,10 +855,8 @@ virStorageBackendLogicalDeleteVol(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr vol, unsigned int flags) { - int ret = -1; - - virCommandPtr lvchange_cmd = NULL; - virCommandPtr lvremove_cmd = NULL; + VIR_AUTOPTR(virCommand) lvchange_cmd = NULL; + VIR_AUTOPTR(virCommand) lvremove_cmd = NULL; virCheckFlags(0, -1); @@ -907,18 +867,14 @@ virStorageBackendLogicalDeleteVol(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, if (virCommandRun(lvremove_cmd, NULL) < 0) { if (virCommandRun(lvchange_cmd, NULL) < 0) { - goto cleanup; + return -1; } else { if (virCommandRun(lvremove_cmd, NULL) < 0) - goto cleanup; + return -1; } } - ret = 0; - cleanup: - virCommandFree(lvchange_cmd); - virCommandFree(lvremove_cmd); - return ret; + return 0; } @@ -926,9 +882,8 @@ static int virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, virStoragePoolDefPtr def) { - int ret; unsigned long long capacity = vol->target.capacity; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (vol->target.encryption && vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) { @@ -961,9 +916,7 @@ virStorageBackendLogicalLVCreate(virStorageVolDefPtr vol, else virCommandAddArg(cmd, def->source.name); - ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 458dbca58b..d9de74f52e 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -140,9 +140,10 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) char *output = NULL; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) cells = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; size_t i; - virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", "-r", NULL); + cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) < 0) @@ -172,7 +173,6 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(output); return ret; } @@ -183,7 +183,7 @@ virStorageBackendSheepdogRefreshPool(virStoragePoolObjPtr pool) { int ret = -1; char *output = NULL; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(SHEEPDOGCLI, "node", "info", "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); @@ -197,7 +197,6 @@ virStorageBackendSheepdogRefreshPool(virStoragePoolObjPtr pool) ret = virStorageBackendSheepdogRefreshAllVol(pool); cleanup: - virCommandFree(cmd); VIR_FREE(output); return ret; } @@ -208,14 +207,12 @@ virStorageBackendSheepdogDeleteVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); - virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "delete", vol->name, NULL); + cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "delete", vol->name, NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); - int ret = virCommandRun(cmd, NULL); - - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } @@ -252,27 +249,20 @@ virStorageBackendSheepdogBuildVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { - int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); if (!vol->target.capacity) { virReportError(VIR_ERR_NO_SUPPORT, "%s", _("volume capacity required for this pool")); - goto cleanup; + return -1; } cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "create", vol->name, NULL); virCommandAddArgFormat(cmd, "%llu", vol->target.capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } @@ -344,30 +334,30 @@ virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool, int ret; char *output = NULL; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", vol->name, "-r", NULL); + VIR_AUTOPTR(virCommand) cmd = NULL; + cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", vol->name, "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); ret = virCommandRun(cmd, NULL); if (ret < 0) - goto cleanup; + return ret; if ((ret = virStorageBackendSheepdogParseVdiList(vol, output)) < 0) - goto cleanup; + return ret; vol->type = VIR_STORAGE_VOL_NETWORK; VIR_FREE(vol->key); if (virAsprintf(&vol->key, "%s/%s", def->source.name, vol->name) < 0) - goto cleanup; + return -1; VIR_FREE(vol->target.path); ignore_value(VIR_STRDUP(vol->target.path, vol->name)); - cleanup: - virCommandFree(cmd); - return ret; + + return 0; } @@ -377,15 +367,13 @@ virStorageBackendSheepdogResizeVol(virStoragePoolObjPtr pool, unsigned long long capacity, unsigned int flags) { + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); - virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "resize", vol->name, NULL); + cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "resize", vol->name, NULL); virCommandAddArgFormat(cmd, "%llu", capacity); virStorageBackendSheepdogAddHostArg(cmd, pool); - int ret = virCommandRun(cmd, NULL); - - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c index 0a9abd446b..e62234fd70 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -39,7 +39,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *grp_name = NULL; char *usr_name = NULL; char *mode = NULL; @@ -75,7 +75,6 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(mode); VIR_FREE(grp_name); VIR_FREE(usr_name); @@ -125,8 +124,7 @@ static int virStorageBackendVzPoolStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; int rc; /* Short-circuit if already unmounted */ @@ -134,13 +132,7 @@ virStorageBackendVzPoolStop(virStoragePoolObjPtr pool) return rc; cmd = virCommandNewArgList(UMOUNT, def->target.path, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index f728477c2a..55f8a138cf 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -51,7 +51,7 @@ VIR_LOG_INIT("storage.storage_backend_zfs"); static int virStorageBackendZFSVolModeNeeded(void) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1, exit_code = -1; char *error = NULL; @@ -77,7 +77,6 @@ virStorageBackendZFSVolModeNeeded(void) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(error); return ret; } @@ -179,7 +178,7 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *volumes_list = NULL; VIR_AUTOPTR(virString) lines = NULL; size_t i; @@ -218,7 +217,6 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, } cleanup: - virCommandFree(cmd); VIR_FREE(volumes_list); return 0; @@ -228,8 +226,8 @@ static int virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; char *zpool_props = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) tokens = NULL; size_t i; @@ -292,7 +290,6 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) goto cleanup; cleanup: - virCommandFree(cmd); VIR_FREE(zpool_props); return 0; @@ -303,7 +300,7 @@ virStorageBackendZFSCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; int volmode_needed = -1; @@ -364,7 +361,6 @@ virStorageBackendZFSCreateVol(virStoragePoolObjPtr pool, ret = 0; cleanup: - virCommandFree(cmd); return ret; } @@ -374,9 +370,8 @@ virStorageBackendZFSDeleteVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol, unsigned int flags) { - int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr destroy_cmd = NULL; + VIR_AUTOPTR(virCommand) destroy_cmd = NULL; virCheckFlags(0, -1); @@ -385,13 +380,7 @@ virStorageBackendZFSDeleteVol(virStoragePoolObjPtr pool, virCommandAddArgFormat(destroy_cmd, "%s/%s", def->source.name, vol->name); - if (virCommandRun(destroy_cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - virCommandFree(destroy_cmd); - return ret; + return virCommandRun(destroy_cmd, NULL); } static int @@ -399,9 +388,8 @@ virStorageBackendZFSBuildPool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; size_t i; - int ret = -1; virCheckFlags(0, -1); @@ -417,14 +405,7 @@ virStorageBackendZFSBuildPool(virStoragePoolObjPtr pool, for (i = 0; i < def->source.ndevice; i++) virCommandAddArg(cmd, def->source.devices[i].path); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } static int @@ -432,22 +413,14 @@ virStorageBackendZFSDeletePool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); cmd = virCommandNewArgList(ZPOOL, "destroy", def->source.name, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } virStorageBackend virStorageBackendZFS = { diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index dcdd52bbbf..7c377439dc 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2207,7 +2207,7 @@ static int virStorageBackendPloopRestoreDesc(char *path) { int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *refresh_tool = NULL; char *desc = NULL; @@ -2238,7 +2238,6 @@ virStorageBackendPloopRestoreDesc(char *path) cleanup: VIR_FREE(refresh_tool); - virCommandFree(cmd); VIR_FREE(desc); return ret; } diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 158cf3adb6..b9dbd3048a 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -617,7 +617,7 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, unsigned int flags) { int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *create_tool = NULL; bool created = false; @@ -677,7 +677,6 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, created = true; ret = virCommandRun(cmd, NULL); cleanup: - virCommandFree(cmd); VIR_FREE(create_tool); if (ret < 0 && created) virFileDeleteTree(vol->target.path); @@ -690,7 +689,7 @@ storagePloopResize(virStorageVolDefPtr vol, unsigned long long capacity) { int ret = -1; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *resize_tool = NULL; resize_tool = virFindFileInPath("ploop"); @@ -705,7 +704,6 @@ storagePloopResize(virStorageVolDefPtr vol, virCommandAddArgFormat(cmd, "%s/DiskDescriptor.xml", vol->target.path); ret = virCommandRun(cmd, NULL); - virCommandFree(cmd); VIR_FREE(resize_tool); return ret; } @@ -1319,8 +1317,7 @@ storageBackendDoCreateQemuImg(virStoragePoolObjPtr pool, const char *inputSecretPath, virStorageVolEncryptConvertStep convertStep) { - int ret; - virCommandPtr cmd; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virStorageBackendCreateQemuImgCmdFromVol(pool, vol, inputvol, flags, create_tool, @@ -1329,11 +1326,7 @@ storageBackendDoCreateQemuImg(virStoragePoolObjPtr pool, if (!cmd) return -1; - ret = virStorageBackendCreateExecCommand(pool, vol, cmd); - - virCommandFree(cmd); - - return ret; + return virStorageBackendCreateExecCommand(pool, vol, cmd); } @@ -2324,7 +2317,7 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, { int ret = -1; char *img_tool = NULL; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; const char *type; char *secretPath = NULL; char *secretAlias = NULL; @@ -2395,7 +2388,6 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, VIR_FREE(secretPath); } VIR_FREE(secretAlias); - virCommandFree(cmd); return ret; } @@ -2449,7 +2441,7 @@ virStorageBackendVolResizeLocal(virStoragePoolObjPtr pool, static int storageBackendPloopHasSnapshots(char *path) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *output = NULL; char *snap_tool = NULL; int ret = -1; @@ -2477,7 +2469,6 @@ storageBackendPloopHasSnapshots(char *path) cleanup: VIR_FREE(output); - virCommandFree(cmd); return ret; } @@ -2685,7 +2676,7 @@ storageBackendVolWipeLocalFile(const char *path, int ret = -1, fd = -1; const char *alg_char = NULL; struct stat st; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; fd = open(path, O_RDWR); if (fd == -1) { @@ -2763,7 +2754,6 @@ storageBackendVolWipeLocalFile(const char *path, } cleanup: - virCommandFree(cmd); VIR_FORCE_CLOSE(fd); return ret; } @@ -2773,7 +2763,7 @@ static int storageBackendVolWipePloop(virStorageVolDefPtr vol, unsigned int algorithm) { - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *target_path = NULL; char *disk_desc = NULL; char *create_tool = NULL; @@ -2820,7 +2810,6 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol, VIR_FREE(disk_desc); VIR_FREE(target_path); VIR_FREE(create_tool); - virCommandFree(cmd); return ret; } @@ -3034,7 +3023,7 @@ virStorageBackendFindGlusterPoolSources(const char *host, { char *glusterpath = NULL; char *outbuf = NULL; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int rc; int ret = -1; @@ -3069,7 +3058,6 @@ virStorageBackendFindGlusterPoolSources(const char *host, cleanup: VIR_FREE(outbuf); - virCommandFree(cmd); VIR_FREE(glusterpath); return ret; } @@ -3309,12 +3297,13 @@ virStorageBackendPARTEDFindLabel(const char *device, const char *const args[] = { device, "print", "--script", NULL, }; - virCommandPtr cmd = virCommandNew(PARTED); + VIR_AUTOPTR(virCommand) cmd = NULL; char *output = NULL; char *error = NULL; char *start, *end; int ret = VIR_STORAGE_PARTED_ERROR; + cmd = virCommandNew(PARTED); virCommandAddArgSet(cmd, args); virCommandAddEnvString(cmd, "LC_ALL=C"); virCommandSetOutputBuffer(cmd, &output); @@ -3359,7 +3348,6 @@ virStorageBackendPARTEDFindLabel(const char *device, ret = VIR_STORAGE_PARTED_DIFFERENT; cleanup: - virCommandFree(cmd); VIR_FREE(output); VIR_FREE(error); return ret; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index cd2437a03e..2e5c64e034 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1376,13 +1376,14 @@ int virStorageFileGetLVMKey(const char *path, * 06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky */ int status; - virCommandPtr cmd = virCommandNewArgList(LVS, "--noheadings", - "--unbuffered", "--nosuffix", - "--options", "uuid", path, - NULL - ); + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; + cmd = virCommandNewArgList(LVS, "--noheadings", + "--unbuffered", "--nosuffix", + "--options", "uuid", path, + NULL + ); *key = NULL; /* Run the program and capture its output */ @@ -1417,8 +1418,6 @@ int virStorageFileGetLVMKey(const char *path, if (*key && STREQ(*key, "")) VIR_FREE(*key); - virCommandFree(cmd); - return ret; } #else @@ -1451,20 +1450,20 @@ virStorageFileGetSCSIKey(const char *path, bool ignoreError ATTRIBUTE_UNUSED) { int status; - virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id", - "--replace-whitespace", - "--whitelisted", - "--device", path, - NULL - ); - int ret = -2; - + VIR_AUTOPTR(virCommand) cmd = NULL; + + cmd = virCommandNewArgList("/lib/udev/scsi_id", + "--replace-whitespace", + "--whitelisted", + "--device", path, + NULL + ); *key = NULL; /* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, key); if (virCommandRun(cmd, &status) < 0) - goto cleanup; + return -2; /* Explicitly check status == 0, rather than passing NULL * to virCommandRun because we don't want to raise an actual @@ -1476,15 +1475,10 @@ virStorageFileGetSCSIKey(const char *path, *nl = '\0'; } - ret = 0; - - cleanup: if (*key && STREQ(*key, "")) VIR_FREE(*key); - virCommandFree(cmd); - - return ret; + return 0; } #else int virStorageFileGetSCSIKey(const char *path, @@ -1522,23 +1516,23 @@ virStorageFileGetNPIVKey(const char *path, { int status; VIR_AUTOFREE(char *) outbuf = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; const char *serial; const char *port; - virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id", - "--replace-whitespace", - "--whitelisted", - "--export", - "--device", path, - NULL - ); - int ret = -2; + cmd = virCommandNewArgList("/lib/udev/scsi_id", + "--replace-whitespace", + "--whitelisted", + "--export", + "--device", path, + NULL + ); *key = NULL; /* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, &status) < 0) - goto cleanup; + return -2; /* Explicitly check status == 0, rather than passing NULL * to virCommandRun because we don't want to raise an actual @@ -1562,12 +1556,7 @@ virStorageFileGetNPIVKey(const char *path, ignore_value(virAsprintf(key, "%s_PORT%s", serial, port)); } - ret = 0; - - cleanup: - virCommandFree(cmd); - - return ret; + return 0; } #else int virStorageFileGetNPIVKey(const char *path ATTRIBUTE_UNUSED, diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c index 2dd87ab731..116a75c3ea 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -25,9 +25,9 @@ testCompareXMLToArgvFiles(bool shouldFail, VIR_AUTOFREE(char *) actualCmdline = NULL; VIR_AUTOFREE(char *) src = NULL; VIR_AUTOPTR(virStoragePoolDef) def = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int defType; int ret = -1; - virCommandPtr cmd = NULL; virStoragePoolObjPtr pool = NULL; virStoragePoolDefPtr objDef; @@ -88,7 +88,6 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&pool); if (shouldFail) { diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 2acbf567ca..38bb2ae004 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -45,9 +45,7 @@ testCompareXMLToArgvFiles(bool shouldFail, char *actualCmdline = NULL; virStorageVolEncryptConvertStep convertStep = VIR_STORAGE_VOL_ENCRYPT_NONE; int ret = -1; - - virCommandPtr cmd = NULL; - + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOPTR(virStorageVolDef) vol = NULL; VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; VIR_AUTOPTR(virStoragePoolDef) inputpool = NULL; @@ -91,6 +89,7 @@ testCompareXMLToArgvFiles(bool shouldFail, convertStep = VIR_STORAGE_VOL_ENCRYPT_CREATE; do { + virCommandFree(cmd); cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol, inputvol, flags, create_tool, @@ -140,7 +139,6 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&obj); return ret; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index c7c40b16f8..c448d8b733 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -128,7 +128,7 @@ static int testPrepImages(void) { int ret = EXIT_FAILURE; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; char *buf = NULL; bool compat = false; @@ -246,7 +246,6 @@ testPrepImages(void) ret = 0; cleanup: VIR_FREE(buf); - virCommandFree(cmd); if (ret) testCleanupImages(); return ret; @@ -713,7 +712,7 @@ static int mymain(void) { int ret; - virCommandPtr cmd = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; struct testChainData data; struct testLookupData data2; struct testPathCanonicalizeData data3; @@ -1604,7 +1603,6 @@ mymain(void) /* Final cleanup */ virStorageSourceFree(chain); testCleanupImages(); - virCommandFree(cmd); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 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 | 73 +++++++--------------- src/storage/storage_backend_iscsi_direct.c | 37 ++++------- src/storage/storage_backend_logical.c | 35 ++++------- src/storage/storage_backend_mpath.c | 18 +++--- src/storage/storage_backend_rbd.c | 35 ++++------- src/storage/storage_backend_scsi.c | 63 +++++++------------ 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 | 16 ++--- 14 files changed, 153 insertions(+), 309 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 230cf44b97..f2f56ee3de 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -56,7 +56,8 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *tmp, *devpath, *partname; + char *tmp, *partname; + VIR_AUTOFREE(char *) devpath = NULL; bool addVol = false; /* Prepended path will be same for all partitions, so we can @@ -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,13 +355,12 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, */ virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *parthelper_path; + VIR_AUTOFREE(char *) parthelper_path = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; struct virStorageBackendDiskPoolVolData cbdata = { .pool = pool, .vol = vol, }; - int ret; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", abs_topbuilddir "/src", @@ -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,9 +413,8 @@ static int virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *parthelper_path; + VIR_AUTOFREE(char *) parthelper_path = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; - int ret; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", abs_topbuilddir "/src", @@ -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 @@ -769,14 +758,13 @@ 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); + VIR_AUTOFREE(char *) devpath = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; bool isDevMapperDevice; - int rc = -1; virCheckFlags(0, -1); @@ -799,7 +787,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); } @@ -810,7 +798,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); @@ -824,7 +812,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 */ @@ -835,7 +823,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 @@ -844,12 +832,9 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, */ virStoragePoolObjClearVols(pool); if (virStorageBackendDiskRefreshPool(pool) < 0) - goto cleanup; + return -1; - rc = 0; - cleanup: - VIR_FREE(devpath); - return rc; + return 0; } @@ -857,11 +842,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, @@ -874,11 +858,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 @@ -888,13 +872,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(); @@ -918,11 +902,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 @@ -932,7 +912,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 7d05ceeeb8..41d010dea0 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -246,7 +246,7 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *src = NULL; + VIR_AUTOFREE(char *) src = NULL; FILE *mtab; struct mntent ent; char buf[1024]; @@ -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; + VIR_AUTOFREE(char *) src = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; - int ret = -1; int rc; 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,7 +571,7 @@ virStoragePoolDefFSNamespaceParse(xmlXPathContextPtr ctxt, void **data) { virStoragePoolFSMountOptionsDefPtr cmdopts = NULL; - xmlNodePtr *nodes = NULL; + VIR_AUTOFREE(xmlNodePtr *)nodes = NULL; int nnodes; size_t i; int ret = -1; @@ -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 cb9f7e4735..0fe548f7e0 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,8 +185,7 @@ virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state, virStorageVolDefPtr vol, const char *name) { - int ret = -1; - char *path = NULL; + VIR_AUTOFREE(char *) path = NULL; char *tmp; VIR_FREE(vol->key); @@ -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; } @@ -243,9 +236,9 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, { int ret = -1; VIR_AUTOPTR(virStorageVolDef) vol = NULL; + VIR_AUTOFREE(char *) header = NULL; glfs_fd_t *fd = NULL; virStorageSourcePtr meta = NULL; - char *header = NULL; ssize_t len; int backingFormat; @@ -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 483ba15102..41fa5e128d 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; + VIR_AUTOFREE(char *) sysfs_path = NULL; uint32_t host; 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; } @@ -159,6 +152,7 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec, unsigned int flags) { VIR_AUTOPTR(virStoragePoolSource) source = NULL; + VIR_AUTOFREE(char *) portal = NULL; size_t ntargets = 0; char **targets = NULL; char *ret = NULL; @@ -168,7 +162,6 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec, .nsources = 0, .sources = NULL }; - char *portal = NULL; virCheckFlags(0, NULL); @@ -223,10 +216,10 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec, } VIR_FREE(list.sources); } + /* NB: Not virString -like, managed be VIR_APPEND_ELEMENT */ for (i = 0; i < ntargets; i++) VIR_FREE(targets[i]); VIR_FREE(targets); - VIR_FREE(portal); return ret; } @@ -235,7 +228,7 @@ virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool, bool *isActive) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *session = NULL; + VIR_AUTOFREE(char *) session = NULL; int ret = -1; *isActive = false; @@ -259,10 +252,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 +321,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 +345,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 +386,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 +398,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 82fa4d7a25..6458b0f835 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -421,15 +421,14 @@ 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; - } + target = NULL; } VIR_STEAL_PTR(*targets, tmp_targets); @@ -481,6 +480,7 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, unsigned int flags) { VIR_AUTOPTR(virStoragePoolSource) source = NULL; + VIR_AUTOFREE(char *) portal = NULL; size_t ntargets = 0; char **targets = NULL; char *ret = NULL; @@ -490,7 +490,6 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, .nsources = 0, .sources = NULL }; - char *portal = NULL; virCheckFlags(0, NULL); @@ -550,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; } @@ -560,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; @@ -577,7 +575,6 @@ virStorageBackendISCSIDirectSetConnection(virStoragePoolObjPtr pool, VIR_STEAL_PTR(*portalRet, portal); cleanup: - VIR_FREE(portal); return iscsi; error: @@ -590,19 +587,14 @@ static int virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool) { struct iscsi_context *iscsi = NULL; - char *portal = NULL; + VIR_AUTOFREE(char *) portal = NULL; int ret = -1; - if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal))) - goto cleanup; - if (virISCSIDirectReportLuns(pool, iscsi, portal) < 0) - goto disconect; - 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; } @@ -638,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; @@ -655,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 e0ddc962b1..2a205a4e95 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -117,9 +117,9 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, { int nextents, ret = -1; const char *regex_unit = "(\\S+)\\((\\S+)\\)"; - char *regex = NULL; - regex_t *reg = NULL; - regmatch_t *vars = NULL; + VIR_AUTOFREE(char *) regex = NULL; + VIR_AUTOFREE(regex_t *) reg = NULL; + VIR_AUTOFREE(regmatch_t *) vars = NULL; char *p = NULL; size_t i; int err, nvars; @@ -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,15 +454,15 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups, void *data) { virStoragePoolSourceListPtr sourceList = data; - char *pvname = NULL; - char *vgname = NULL; + VIR_AUTOFREE(char *) pvname = NULL; + VIR_AUTOFREE(char *) vgname = NULL; size_t i; virStoragePoolSourceDevicePtr dev; virStoragePoolSource *thisSource; 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++) { @@ -479,30 +474,22 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups, if (thisSource == NULL) { if (!(thisSource = virStoragePoolSourceListNewSource(sourceList))) - goto error; + return -1; - thisSource->name = vgname; + VIR_STEAL_PTR(thisSource->name, vgname); } - else - VIR_FREE(vgname); if (VIR_REALLOC_N(thisSource->devices, thisSource->ndevice + 1) != 0) - goto error; + return -1; dev = &thisSource->devices[thisSource->ndevice]; thisSource->ndevice++; thisSource->format = VIR_STORAGE_POOL_LOGICAL_LVM2; memset(dev, 0, sizeof(*dev)); - dev->path = pvname; + VIR_STEAL_PTR(dev->path, pvname); return 0; - - error: - VIR_FREE(pvname); - VIR_FREE(vgname); - - return -1; } /* diff --git a/src/storage/storage_backend_mpath.c b/src/storage/storage_backend_mpath.c index 423f945fbc..07b670f2f1 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -153,8 +153,8 @@ static int virStorageBackendCreateVols(virStoragePoolObjPtr pool, struct dm_names *names) { - int retval = -1, is_mpath = 0; - char *map_device = NULL; + int is_mpath = 0; + VIR_AUTOFREE(char *) map_device = NULL; uint32_t minor = -1; uint32_t next; @@ -162,24 +162,23 @@ virStorageBackendCreateVols(virStoragePoolObjPtr pool, 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); } /* Given the way libdevmapper returns its data, I don't see @@ -191,10 +190,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..99ddb2c922 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -86,7 +86,7 @@ virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt, void **data) { virStoragePoolRBDConfigOptionsDefPtr cmdopts = NULL; - xmlNodePtr *nodes = NULL; + VIR_AUTOFREE(xmlNodePtr *)nodes = NULL; int nnodes; size_t i; int ret = -1; @@ -145,7 +145,6 @@ virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt, ret = 0; cleanup: - VIR_FREE(nodes); virStoragePoolDefRBDNamespaceFree(cmdopts); return ret; } @@ -213,7 +212,7 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, char *rados_key = NULL; virBuffer mon_host = VIR_BUFFER_INITIALIZER; size_t i; - char *mon_buff = NULL; + VIR_AUTOFREE(char *) mon_buff = NULL; const char *client_mount_timeout = "30"; const char *mon_op_timeout = "30"; const char *osd_op_timeout = "30"; @@ -348,7 +347,6 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, virObjectUnref(conn); virBufferFreeAndReset(&mon_host); - VIR_FREE(mon_buff); return ret; } @@ -574,7 +572,8 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) int ret = -1; int len = -1; int r = 0; - char *name, *names = NULL; + char *name; + VIR_AUTOFREE(char *) names = NULL; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; struct rados_cluster_stat_t clusterstat; @@ -662,7 +661,6 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) ret = 0; cleanup: - VIR_FREE(names); virStorageBackendRBDFreeState(&ptr); return ret; } @@ -677,7 +675,7 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, int max_snaps = 128; int snap_count, protected; size_t i; - rbd_snap_info_t *snaps = NULL; + VIR_AUTOFREE(rbd_snap_info_t *) snaps = NULL; rbd_image_t image = NULL; if ((r = rbd_open(ioctx, vol->name, &image, NULL)) < 0) { @@ -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,7 +945,7 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, int max_snaps = 128; size_t i; int diff; - rbd_snap_info_t *snaps = NULL; + VIR_AUTOFREE(rbd_snap_info_t *) snaps = NULL; rbd_image_info_t info; if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { @@ -1023,8 +1019,6 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, if (snaps) rbd_snap_list_end(snaps); - VIR_FREE(snaps); - return ret; } @@ -1098,7 +1092,7 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, uint64_t stripe_count; uint64_t stripe_unit; virBuffer snapname = VIR_BUFFER_INITIALIZER; - char *snapname_buff = NULL; + VIR_AUTOFREE(char *) snapname_buff = NULL; rbd_image_t image = NULL; if ((r = rbd_open(io, origvol, &image, NULL)) < 0) { @@ -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 14f01f9ec0..1a10e43647 100644 --- a/src/storage/storage_backend_scsi.c +++ b/src/storage/storage_backend_scsi.c @@ -56,16 +56,14 @@ static int virStorageBackendSCSITriggerRescan(uint32_t host) { int fd = -1; - int retval = 0; - char *path; + int retval = -1; + 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) { - retval = -1; - goto out; - } + LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) + return -1; VIR_DEBUG("Scan trigger path is '%s'", path); @@ -75,8 +73,7 @@ virStorageBackendSCSITriggerRescan(uint32_t host) virReportSystemError(errno, _("Could not open '%s' to trigger host scan"), path); - retval = -1; - goto free_path; + goto cleanup; } if (safewrite(fd, @@ -86,13 +83,12 @@ virStorageBackendSCSITriggerRescan(uint32_t host) virReportSystemError(errno, _("Write to '%s' to trigger host scan failed"), path); - retval = -1; } + retval = 0; + + cleanup: VIR_FORCE_CLOSE(fd); - free_path: - VIR_FREE(path); - out: VIR_DEBUG("Rescan of host %d complete", host); return retval; } @@ -178,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; @@ -192,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)); } @@ -206,8 +201,6 @@ getAdapterName(virStorageAdapterPtr adapter) } } - cleanup: - VIR_FREE(parentaddr); return name; } @@ -248,8 +241,8 @@ checkParent(const char *name, const char *parent_name) { unsigned int host_num; - char *scsi_host_name = NULL; - char *vhba_parent = NULL; + VIR_AUTOFREE(char *) scsi_host_name = NULL; + VIR_AUTOFREE(char *) vhba_parent = NULL; bool retval = false; virConnectPtr conn = NULL; @@ -291,8 +284,6 @@ checkParent(const char *name, cleanup: virObjectUnref(conn); - VIR_FREE(vhba_parent); - VIR_FREE(scsi_host_name); return retval; } @@ -302,7 +293,7 @@ createVport(virStoragePoolDefPtr def, const char *configFile, virStorageAdapterFCHostPtr fchost) { - char *name = NULL; + VIR_AUTOFREE(char *) name = NULL; virStoragePoolFCRefreshInfoPtr cbdata = NULL; virThread thread; int ret = -1; @@ -363,7 +354,6 @@ createVport(virStoragePoolDefPtr def, ret = 0; cleanup: - VIR_FREE(name); return ret; } @@ -373,10 +363,9 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, bool *isActive) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *path = NULL; - char *name = NULL; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) name = NULL; unsigned int host; - int ret = -1; *isActive = false; @@ -394,28 +383,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; + VIR_AUTOFREE(char *) name = NULL; unsigned int host; - int ret = -1; def->allocation = def->capacity = def->available = 0; @@ -423,20 +407,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 d9de74f52e..1215b36b61 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -136,8 +136,7 @@ virStorageBackendSheepdogAddVolume(virStoragePoolObjPtr pool, const char *diskIn static int virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) { - int ret = -1; - char *output = NULL; + 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 e62234fd70..6979fac3c6 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -40,9 +40,9 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); VIR_AUTOPTR(virCommand) cmd = NULL; - 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; /* Check the permissions */ if (def->target.perms.mode == (mode_t)-1) @@ -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 55f8a138cf..8bbc4e01ed 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -53,7 +53,7 @@ virStorageBackendZFSVolModeNeeded(void) { VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1, exit_code = -1; - char *error = NULL; + VIR_AUTOFREE(char *) error = NULL; /* 'zfs get' without arguments prints out * usage information to stderr, including @@ -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; } @@ -179,7 +177,7 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); VIR_AUTOPTR(virCommand) cmd = NULL; - char *volumes_list = NULL; + VIR_AUTOFREE(char *) volumes_list = NULL; VIR_AUTOPTR(virString) lines = NULL; size_t i; @@ -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 f8bbde8cfe..6f87c77f1f 100644 --- a/src/storage/storage_file_gluster.c +++ b/src/storage/storage_file_gluster.c @@ -258,7 +258,7 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, void *data) { virStorageFileBackendGlusterPrivPtr priv = data; - char *buf = NULL; + VIR_AUTOFREE(char *) buf = NULL; size_t bufsiz = 0; ssize_t ret; struct stat st; @@ -277,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) @@ -291,13 +291,9 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, buf[ret] = '\0'; - *linkpath = buf; + VIR_STEAL_PTR(*linkpath, buf); return 0; - - error: - VIR_FREE(buf); - return -1; } @@ -305,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; @@ -321,8 +317,6 @@ virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src) src->volume, filePath)); - VIR_FREE(filePath); - return priv->canonpath; } -- 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> --- src/storage/storage_driver.c | 70 ++++++++++++------------------------ 1 file changed, 23 insertions(+), 47 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 7c377439dc..61e0756e3e 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -122,7 +122,7 @@ storagePoolUpdateStateCallback(virStoragePoolObjPtr obj, virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); bool active = false; virStorageBackendPtr backend; - char *stateFile; + VIR_AUTOFREE(char *) stateFile = NULL; if ((backend = virStorageBackendForType(def->type)) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -164,8 +164,6 @@ storagePoolUpdateStateCallback(virStoragePoolObjPtr obj, if (!virStoragePoolObjIsActive(obj)) virStoragePoolUpdateInactive(&obj); - VIR_FREE(stateFile); - return; } @@ -203,7 +201,7 @@ storageDriverAutostartCallback(virStoragePoolObjPtr obj, } if (started) { - char *stateFile; + VIR_AUTOFREE(char *) stateFile = NULL; virStoragePoolObjClearVols(obj); stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"); @@ -217,7 +215,6 @@ storageDriverAutostartCallback(virStoragePoolObjPtr obj, } else { virStoragePoolObjSetActive(obj, true); } - VIR_FREE(stateFile); } } @@ -240,16 +237,15 @@ storageStateInitialize(bool privileged, virStateInhibitCallback callback ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { - int ret = -1; - char *configdir = NULL; - char *rundir = NULL; + VIR_AUTOFREE(char *) configdir = NULL; + VIR_AUTOFREE(char *) rundir = NULL; if (VIR_ALLOC(driver) < 0) - return ret; + return -1; if (virMutexInit(&driver->lock) < 0) { VIR_FREE(driver); - return ret; + return -1; } storageDriverLock(); @@ -302,16 +298,12 @@ storageStateInitialize(bool privileged, storageDriverUnlock(); - ret = 0; - cleanup: - VIR_FREE(configdir); - VIR_FREE(rundir); - return ret; + return 0; error: storageDriverUnlock(); storageStateCleanup(); - goto cleanup; + return -1; } /** @@ -691,12 +683,12 @@ storagePoolCreateXML(virConnectPtr conn, unsigned int flags) { VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; + VIR_AUTOFREE(char *) stateFile = NULL; virStoragePoolObjPtr obj = NULL; virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; virStorageBackendPtr backend; virObjectEventPtr event = NULL; - char *stateFile = NULL; unsigned int build_flags = 0; virCheckFlags(VIR_STORAGE_POOL_CREATE_WITH_BUILD | @@ -761,7 +753,6 @@ storagePoolCreateXML(virConnectPtr conn, pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); cleanup: - VIR_FREE(stateFile); virObjectEventStateQueue(driver->storageEventState, event); virStoragePoolObjEndAPI(&obj); return pool; @@ -892,7 +883,7 @@ storagePoolCreate(virStoragePoolPtr pool, virStorageBackendPtr backend; virObjectEventPtr event = NULL; int ret = -1; - char *stateFile = NULL; + VIR_AUTOFREE(char *) stateFile = NULL; unsigned int build_flags = 0; virCheckFlags(VIR_STORAGE_POOL_CREATE_WITH_BUILD | @@ -955,7 +946,6 @@ storagePoolCreate(virStoragePoolPtr pool, ret = 0; cleanup: - VIR_FREE(stateFile); virObjectEventStateQueue(driver->storageEventState, event); virStoragePoolObjEndAPI(&obj); return ret; @@ -1013,7 +1003,7 @@ storagePoolDestroy(virStoragePoolPtr pool) virStoragePoolDefPtr def; virStorageBackendPtr backend; virObjectEventPtr event = NULL; - char *stateFile = NULL; + VIR_AUTOFREE(char *) stateFile = NULL; int ret = -1; if (!(obj = storagePoolObjFindByUUID(pool->uuid, pool->name))) @@ -1045,7 +1035,6 @@ storagePoolDestroy(virStoragePoolPtr pool) goto cleanup; unlink(stateFile); - VIR_FREE(stateFile); if (backend->stopPool && backend->stopPool(obj) < 0) @@ -1078,7 +1067,7 @@ storagePoolDelete(virStoragePoolPtr pool, virStoragePoolDefPtr def; virStorageBackendPtr backend; virObjectEventPtr event = NULL; - char *stateFile = NULL; + VIR_AUTOFREE(char *) stateFile = NULL; int ret = -1; if (!(obj = virStoragePoolObjFromStoragePool(pool))) @@ -1111,7 +1100,6 @@ storagePoolDelete(virStoragePoolPtr pool, goto cleanup; unlink(stateFile); - VIR_FREE(stateFile); if (!backend->deletePool) { virReportError(VIR_ERR_NO_SUPPORT, @@ -1172,10 +1160,10 @@ storagePoolRefresh(virStoragePoolPtr pool, virStoragePoolObjClearVols(obj); if (backend->refreshPool(obj) < 0) { - char *stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"); + VIR_AUTOFREE(char *) stateFile = NULL; + stateFile = virFileBuildPath(driver->stateDir, def->name, ".xml"); storagePoolRefreshFailCleanup(backend, obj, stateFile); - VIR_FREE(stateFile); event = virStoragePoolEventLifecycleNew(def->name, def->uuid, @@ -1540,7 +1528,7 @@ storageVolLookupByPathCallback(virStoragePoolObjPtr obj, { struct storageVolLookupData *data = (struct storageVolLookupData *)opaque; virStoragePoolDefPtr def; - char *stable_path = NULL; + VIR_AUTOFREE(char *) stable_path = NULL; if (!virStoragePoolObjIsActive(obj)) return false; @@ -1579,7 +1567,6 @@ storageVolLookupByPathCallback(virStoragePoolObjPtr obj, } data->voldef = virStorageVolDefFindByPath(obj, stable_path); - VIR_FREE(stable_path); return !!data->voldef; } @@ -1649,7 +1636,7 @@ storagePoolLookupByTargetPath(virConnectPtr conn, virStoragePoolObjPtr obj; virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; - char *cleanpath; + VIR_AUTOFREE(char *) cleanpath = NULL; cleanpath = virFileSanitizePath(path); if (!cleanpath) @@ -1660,7 +1647,7 @@ storagePoolLookupByTargetPath(virConnectPtr conn, cleanpath))) { def = virStoragePoolObjGetDef(obj); if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0) - goto cleanup; + return NULL; pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); virStoragePoolObjEndAPI(&obj); @@ -1678,8 +1665,6 @@ storagePoolLookupByTargetPath(virConnectPtr conn, } } - cleanup: - VIR_FREE(cleanpath); return pool; } @@ -2206,40 +2191,31 @@ virStorageVolPoolRefreshDataFree(void *opaque) static int virStorageBackendPloopRestoreDesc(char *path) { - int ret = -1; VIR_AUTOPTR(virCommand) cmd = NULL; - char *refresh_tool = NULL; - char *desc = NULL; + VIR_AUTOFREE(char *) refresh_tool = NULL; + VIR_AUTOFREE(char *) desc = NULL; if (virAsprintf(&desc, "%s/DiskDescriptor.xml", path) < 0) - return ret; + return -1; if (virFileRemove(desc, 0, 0) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("refresh ploop failed:" " unable to delete DiskDescriptor.xml")); - goto cleanup; + return -1; } refresh_tool = virFindFileInPath("ploop"); if (!refresh_tool) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to find ploop, please install ploop tools")); - goto cleanup; + return -1; } cmd = virCommandNewArgList(refresh_tool, "restore-descriptor", path, NULL); virCommandAddArgFormat(cmd, "%s/root.hds", path); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - - cleanup: - VIR_FREE(refresh_tool); - VIR_FREE(desc); - return ret; + return virCommandRun(cmd, NULL); } -- 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> --- src/conf/storage_conf.c | 179 +++++++++++++++++----------------------- 1 file changed, 74 insertions(+), 105 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 4fb5fb9f57..5f1122422a 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -453,15 +453,15 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, xmlNodePtr node) { int ret = -1; - xmlNodePtr relnode, authnode, *nodeset = NULL; + xmlNodePtr relnode, authnode; xmlNodePtr adapternode; int nsource; size_t i; virStoragePoolOptionsPtr options; VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; - char *name = NULL; - char *port = NULL; - char *ver = NULL; + VIR_AUTOFREE(char *) port = NULL; + VIR_AUTOFREE(char *) ver = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodeset = NULL; int n; relnode = ctxt->node; @@ -478,7 +478,9 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, } if (options->formatFromString) { - char *format = virXPathString("string(./format/@type)", ctxt); + VIR_AUTOFREE(char *) format = NULL; + + format = virXPathString("string(./format/@type)", ctxt); if (format == NULL) source->format = options->defaultFormat; else @@ -487,10 +489,8 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, if (source->format < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown pool format type %s"), format); - VIR_FREE(format); goto cleanup; } - VIR_FREE(format); } if ((n = virXPathNodeSet("./host", ctxt, &nodeset)) < 0) @@ -502,13 +502,12 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, source->nhost = n; for (i = 0; i < source->nhost; i++) { - name = virXMLPropString(nodeset[i], "name"); - if (name == NULL) { + source->hosts[i].name = virXMLPropString(nodeset[i], "name"); + if (!source->hosts[i].name) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool host name")); goto cleanup; } - source->hosts[i].name = name; port = virXMLPropString(nodeset[i], "port"); if (port) { @@ -518,7 +517,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, port); goto cleanup; } - VIR_FREE(port); } } } @@ -532,7 +530,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, goto cleanup; for (i = 0; i < nsource; i++) { - char *partsep; + VIR_AUTOFREE(char *) partsep = NULL; virStoragePoolSourceDevice dev = { .path = NULL }; dev.path = virXMLPropString(nodeset[i], "path"); @@ -550,10 +548,8 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, _("invalid part_separator setting '%s'"), partsep); virStoragePoolSourceDeviceClear(&dev); - VIR_FREE(partsep); goto cleanup; } - VIR_FREE(partsep); } if (VIR_APPEND_ELEMENT(source->devices, source->ndevice, dev) < 0) { @@ -612,8 +608,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, cleanup: ctxt->node = relnode; - VIR_FREE(port); - VIR_FREE(nodeset); return ret; } @@ -660,7 +654,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, virStoragePermsPtr perms, const char *permxpath) { - char *mode; + VIR_AUTOFREE(char *) mode = NULL; long long val; int ret = -1; xmlNodePtr relnode; @@ -683,13 +677,11 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, int tmp; if (virStrToLong_i(mode, NULL, 8, &tmp) < 0 || (tmp & ~0777)) { - VIR_FREE(mode); virReportError(VIR_ERR_XML_ERROR, "%s", _("malformed octal mode")); goto error; } perms->mode = tmp; - VIR_FREE(mode); } else { perms->mode = (mode_t) -1; } @@ -740,9 +732,9 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) VIR_AUTOPTR(virStoragePoolDef) def = NULL; virStoragePoolDefPtr ret = NULL; xmlNodePtr source_node; - char *type = NULL; - char *uuid = NULL; - char *target_path = NULL; + VIR_AUTOFREE(char *) type = NULL; + VIR_AUTOFREE(char *) uuid = NULL; + VIR_AUTOFREE(char *) target_path = NULL; if (VIR_ALLOC(def) < 0) return NULL; @@ -751,23 +743,23 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) if (type == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("storage pool missing type attribute")); - goto cleanup; + return NULL; } if ((def->type = virStoragePoolTypeFromString(type)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown storage pool type %s"), type); - goto cleanup; + return NULL; } if ((options = virStoragePoolOptionsForPoolType(def->type)) == NULL) - goto cleanup; + return NULL; source_node = virXPathNode("./source", ctxt); if (source_node) { if (virStoragePoolDefParseSource(ctxt, &def->source, def->type, source_node) < 0) - goto cleanup; + return NULL; } else { if (options->formatFromString) def->source.format = options->defaultFormat; @@ -777,17 +769,18 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) if (def->name == NULL && options->flags & VIR_STORAGE_POOL_SOURCE_NAME && VIR_STRDUP(def->name, def->source.name) < 0) - goto cleanup; + return NULL; + if (def->name == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing pool source name element")); - goto cleanup; + return NULL; } if (strchr(def->name, '/')) { virReportError(VIR_ERR_XML_ERROR, _("name %s cannot contain '/'"), def->name); - goto cleanup; + return NULL; } uuid = virXPathString("string(./uuid)", ctxt); @@ -795,13 +788,13 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) if (virUUIDGenerate(def->uuid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("unable to generate uuid")); - goto cleanup; + return NULL; } } else { if (virUUIDParse(uuid, def->uuid) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("malformed uuid element")); - goto cleanup; + return NULL; } } @@ -809,7 +802,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) if (!def->source.nhost) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool source host name")); - goto cleanup; + return NULL; } } @@ -817,27 +810,27 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) if (!def->source.dir) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool source path")); - goto cleanup; + return NULL; } } if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) { if (def->source.name == NULL) { /* source name defaults to pool name */ if (VIR_STRDUP(def->source.name, def->name) < 0) - goto cleanup; + return NULL; } } if ((options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) && (virStorageAdapterValidate(&def->source.adapter)) < 0) - goto cleanup; + return NULL; /* If DEVICE is the only source type, then its required */ if (options->flags == VIR_STORAGE_POOL_SOURCE_DEVICE) { if (!def->source.ndevice) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool source device name")); - goto cleanup; + return NULL; } } @@ -846,32 +839,32 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) if (!(options->flags & VIR_STORAGE_POOL_SOURCE_NETWORK)) { if (def->type == VIR_STORAGE_POOL_LOGICAL) { if (virAsprintf(&target_path, "/dev/%s", def->source.name) < 0) - goto cleanup; + return NULL; } else if (def->type == VIR_STORAGE_POOL_ZFS) { if (virAsprintf(&target_path, "/dev/zvol/%s", def->source.name) < 0) - goto cleanup; + return NULL; } else { target_path = virXPathString("string(./target/path)", ctxt); if (!target_path) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing storage pool target path")); - goto cleanup; + return NULL; } } def->target.path = virFileSanitizePath(target_path); if (!def->target.path) - goto cleanup; + return NULL; if (virStorageDefParsePerms(ctxt, &def->target.perms, "./target/permissions") < 0) - goto cleanup; + return NULL; } if (def->type == VIR_STORAGE_POOL_ISCSI_DIRECT && !def->source.initiator.iqn) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("missing initiator IQN")); - goto cleanup; + return NULL; } /* Make a copy of all the callback pointers here for easier use, @@ -879,14 +872,10 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) def->ns = options->ns; if (def->ns.parse && (def->ns.parse)(ctxt, &def->namespaceData) < 0) - goto cleanup; + return NULL; VIR_STEAL_PTR(ret, def); - cleanup: - VIR_FREE(uuid); - VIR_FREE(type); - VIR_FREE(target_path); return ret; } @@ -1169,13 +1158,13 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, VIR_AUTOPTR(virStorageVolDef) def = NULL; virStorageVolDefPtr ret = NULL; virStorageVolOptionsPtr options; - char *type = NULL; - char *allocation = NULL; - char *capacity = NULL; - char *unit = NULL; - char *backingStore = NULL; + VIR_AUTOFREE(char *) type = NULL; + VIR_AUTOFREE(char *) allocation = NULL; + VIR_AUTOFREE(char *) capacity = NULL; + VIR_AUTOFREE(char *) unit = NULL; + VIR_AUTOFREE(char *) backingStore = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; xmlNodePtr node; - xmlNodePtr *nodes = NULL; size_t i; int n; @@ -1195,7 +1184,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (def->name == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing volume name element")); - goto cleanup; + return NULL; } /* Normally generated by pool refresh, but useful for unit tests */ @@ -1207,13 +1196,13 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if ((def->type = virStorageVolTypeFromString(type)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume type '%s'"), type); - goto cleanup; + return NULL; } } if ((backingStore = virXPathString("string(./backingStore/path)", ctxt))) { if (VIR_ALLOC(def->target.backingStore) < 0) - goto cleanup; + return NULL; def->target.backingStore->type = VIR_STORAGE_TYPE_FILE; @@ -1221,7 +1210,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, backingStore = NULL; if (options->formatFromString) { - char *format = virXPathString("string(./backingStore/format/@type)", ctxt); + VIR_AUTOFREE(char *) format = NULL; + + format = virXPathString("string(./backingStore/format/@type)", ctxt); if (format == NULL) def->target.backingStore->format = options->defaultFormat; else @@ -1230,29 +1221,27 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (def->target.backingStore->format < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume format type %s"), format); - VIR_FREE(format); - goto cleanup; + return NULL; } - VIR_FREE(format); } if (VIR_ALLOC(def->target.backingStore->perms) < 0) - goto cleanup; + return NULL; if (virStorageDefParsePerms(ctxt, def->target.backingStore->perms, "./backingStore/permissions") < 0) - goto cleanup; + return NULL; } capacity = virXPathString("string(./capacity)", ctxt); unit = virXPathString("string(./capacity/@unit)", ctxt); if (capacity) { if (virStorageSize(unit, capacity, &def->target.capacity) < 0) - goto cleanup; + return NULL; } else if (!(flags & VIR_VOL_XML_PARSE_NO_CAPACITY) && !((flags & VIR_VOL_XML_PARSE_OPT_CAPACITY) && virStorageSourceHasBacking(&def->target))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing capacity element")); - goto cleanup; + return NULL; } VIR_FREE(unit); @@ -1260,7 +1249,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (allocation) { unit = virXPathString("string(./allocation/@unit)", ctxt); if (virStorageSize(unit, allocation, &def->target.allocation) < 0) - goto cleanup; + return NULL; def->target.has_allocation = true; } else { def->target.allocation = def->target.capacity; @@ -1268,7 +1257,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, def->target.path = virXPathString("string(./target/path)", ctxt); if (options->formatFromString) { - char *format = virXPathString("string(./target/format/@type)", ctxt); + VIR_AUTOFREE(char *) format = NULL; + + format = virXPathString("string(./target/format/@type)", ctxt); if (format == NULL) def->target.format = options->defaultFormat; else @@ -1277,41 +1268,39 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (def->target.format < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown volume format type %s"), format); - VIR_FREE(format); - goto cleanup; + return NULL; } - VIR_FREE(format); } if (VIR_ALLOC(def->target.perms) < 0) - goto cleanup; + return NULL; if (virStorageDefParsePerms(ctxt, def->target.perms, "./target/permissions") < 0) - goto cleanup; + return NULL; node = virXPathNode("./target/encryption", ctxt); if (node != NULL) { def->target.encryption = virStorageEncryptionParseNode(node, ctxt); if (def->target.encryption == NULL) - goto cleanup; + return NULL; } def->target.compat = virXPathString("string(./target/compat)", ctxt); if (virStorageFileCheckCompat(def->target.compat) < 0) - goto cleanup; + return NULL; if (virXPathNode("./target/nocow", ctxt)) def->target.nocow = true; if (virXPathNode("./target/features", ctxt)) { if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0) - goto cleanup; + return NULL; if (!def->target.compat && VIR_STRDUP(def->target.compat, "1.1") < 0) - goto cleanup; + return NULL; if (!(def->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) - goto cleanup; + return NULL; for (i = 0; i < n; i++) { int f = virStorageFileFeatureTypeFromString((const char*)nodes[i]->name); @@ -1319,7 +1308,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (f < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported feature %s"), (const char*)nodes[i]->name); - goto cleanup; + return NULL; } ignore_value(virBitmapSetBit(def->target.features, f)); } @@ -1327,14 +1316,6 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, } VIR_STEAL_PTR(ret, def); - - cleanup: - VIR_FREE(nodes); - VIR_FREE(allocation); - VIR_FREE(capacity); - VIR_FREE(unit); - VIR_FREE(type); - VIR_FREE(backingStore); return ret; } @@ -1616,32 +1597,27 @@ virStoragePoolSaveState(const char *stateFile, virStoragePoolDefPtr def) { virBuffer buf = VIR_BUFFER_INITIALIZER; - int ret = -1; - char *xml; + VIR_AUTOFREE(char *) xml = NULL; virBufferAddLit(&buf, "<poolstate>\n"); virBufferAdjustIndent(&buf, 2); if (virStoragePoolDefFormatBuf(&buf, def) < 0) - goto error; + return -1; virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</poolstate>\n"); if (virBufferCheckError(&buf) < 0) - goto error; + return -1; if (!(xml = virBufferContentAndReset(&buf))) - goto error; + return -1; if (virStoragePoolSaveXML(stateFile, def, xml)) - goto error; - - ret = 0; + return -1; - error: - VIR_FREE(xml); - return ret; + return 0; } @@ -1649,8 +1625,7 @@ int virStoragePoolSaveConfig(const char *configFile, virStoragePoolDefPtr def) { - char *xml; - int ret = -1; + VIR_AUTOFREE(char *) xml = NULL; if (!(xml = virStoragePoolDefFormat(def))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1658,13 +1633,7 @@ virStoragePoolSaveConfig(const char *configFile, return -1; } - if (virStoragePoolSaveXML(configFile, def, xml)) - goto cleanup; - - ret = 0; - cleanup: - VIR_FREE(xml); - return ret; + return virStoragePoolSaveXML(configFile, def, xml); } -- 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> --- src/util/virstoragefile.c | 130 ++++++++++++++------------------------ 1 file changed, 47 insertions(+), 83 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 2e5c64e034..c2f044d2db 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -595,7 +595,8 @@ vmdk4GetBackingStore(char **res, size_t buf_size) { static const char prefix[] = "parentFileNameHint=\""; - char *desc, *start, *end; + VIR_AUTOFREE(char *) desc = NULL; + char *start, *end; size_t len; int ret = BACKING_STORE_ERROR; @@ -645,7 +646,6 @@ vmdk4GetBackingStore(char **res, ret = BACKING_STORE_OK; cleanup: - VIR_FREE(desc); return ret; } @@ -1084,7 +1084,7 @@ virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid) int ret = -1; struct stat sb; ssize_t len = VIR_STORAGE_MAX_HEADER; - char *header = NULL; + VIR_AUTOFREE(char *) header = NULL; if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) { virReportSystemError(-fd, _("Failed to open file '%s'"), path); @@ -1115,7 +1115,6 @@ virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid) ret = virStorageFileProbeFormatFromBuf(path, header, len); cleanup: - VIR_FREE(header); VIR_FORCE_CLOSE(fd); return ret; @@ -1215,7 +1214,7 @@ virStorageFileGetMetadataFromFD(const char *path, { virStorageSourcePtr ret = NULL; virStorageSourcePtr meta = NULL; - char *buf = NULL; + VIR_AUTOFREE(char *) buf = NULL; ssize_t len = VIR_STORAGE_MAX_HEADER; struct stat sb; int dummy; @@ -1265,7 +1264,6 @@ virStorageFileGetMetadataFromFD(const char *path, cleanup: virStorageSourceFree(meta); - VIR_FREE(buf); return ret; } @@ -1614,8 +1612,7 @@ virStorageFileParseChainIndex(const char *diskTarget, unsigned int *chainIndex) { unsigned int idx = 0; - char *target = NULL; - int ret = 0; + VIR_AUTOFREE(char *) target = NULL; *chainIndex = 0; @@ -1626,21 +1623,18 @@ virStorageFileParseChainIndex(const char *diskTarget, return 0; if (idx == 0) - goto cleanup; + return 0; if (STRNEQ(diskTarget, target)) { virReportError(VIR_ERR_INVALID_ARG, _("requested target '%s' does not match target '%s'"), target, diskTarget); - ret = -1; - goto cleanup; + return -1; } *chainIndex = idx; - cleanup: - VIR_FREE(target); - return ret; + return 0; } @@ -1890,9 +1884,9 @@ virStorageAuthDefParse(xmlNodePtr node, { xmlNodePtr saveNode = ctxt->node; VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; + VIR_AUTOFREE(char *) authtype = NULL; virStorageAuthDefPtr ret = NULL; xmlNodePtr secretnode = NULL; - char *authtype = NULL; ctxt->node = node; @@ -1939,7 +1933,6 @@ virStorageAuthDefParse(xmlNodePtr node, VIR_STEAL_PTR(ret, authdef); cleanup: - VIR_FREE(authtype); ctxt->node = saveNode; return ret; @@ -1983,10 +1976,10 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt) { virStoragePRDefPtr prd; virStoragePRDefPtr ret = NULL; - char *managed = NULL; - char *type = NULL; - char *path = NULL; - char *mode = NULL; + VIR_AUTOFREE(char *) managed = NULL; + VIR_AUTOFREE(char *) type = NULL; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) mode = NULL; if (VIR_ALLOC(prd) < 0) return NULL; @@ -2045,10 +2038,6 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt) VIR_STEAL_PTR(ret, prd); cleanup: - VIR_FREE(mode); - VIR_FREE(path); - VIR_FREE(type); - VIR_FREE(managed); virStoragePRDefFree(prd); return ret; } @@ -2601,7 +2590,7 @@ static virStorageSourcePtr virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, const char *rel) { - char *dirname = NULL; + VIR_AUTOFREE(char *) dirname = NULL; virStorageSourcePtr ret; if (VIR_ALLOC(ret) < 0) @@ -2645,7 +2634,6 @@ virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, } cleanup: - VIR_FREE(dirname); return ret; error: @@ -2809,8 +2797,8 @@ int virStorageSourceParseRBDColonString(const char *rbdstr, virStorageSourcePtr src) { - char *options = NULL; char *p, *e, *next; + VIR_AUTOFREE(char *) options = NULL; VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; /* optionally skip the "rbd:" prefix if provided */ @@ -2818,19 +2806,19 @@ virStorageSourceParseRBDColonString(const char *rbdstr, rbdstr += strlen("rbd:"); if (VIR_STRDUP(src->path, rbdstr) < 0) - goto error; + return -1; p = strchr(src->path, ':'); if (p) { if (VIR_STRDUP(options, p + 1) < 0) - goto error; + return -1; *p = '\0'; } /* snapshot name */ if ((p = strchr(src->path, '@'))) { if (VIR_STRDUP(src->snapshot, p + 1) < 0) - goto error; + return -1; *p = '\0'; } @@ -2838,7 +2826,7 @@ virStorageSourceParseRBDColonString(const char *rbdstr, if ((p = strchr(src->path, '/'))) { VIR_STEAL_PTR(src->volume, src->path); if (VIR_STRDUP(src->path, p + 1) < 0) - goto error; + return -1; *p = '\0'; } @@ -2866,14 +2854,14 @@ virStorageSourceParseRBDColonString(const char *rbdstr, if (STRPREFIX(p, "id=")) { /* formulate authdef for src->auth */ if (VIR_ALLOC(authdef) < 0) - goto error; + return -1; if (VIR_STRDUP(authdef->username, p + strlen("id=")) < 0) - goto error; + return -1; if (VIR_STRDUP(authdef->secrettype, virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_CEPH)) < 0) - goto error; + return -1; VIR_STEAL_PTR(src->auth, authdef); src->authInherited = true; @@ -2897,7 +2885,7 @@ virStorageSourceParseRBDColonString(const char *rbdstr, } if (virStorageSourceRBDAddHost(src, h) < 0) - goto error; + return -1; h = sep; } @@ -2905,16 +2893,11 @@ virStorageSourceParseRBDColonString(const char *rbdstr, if (STRPREFIX(p, "conf=") && VIR_STRDUP(src->configFile, p + strlen("conf=")) < 0) - goto error; + return -1; p = next; } - VIR_FREE(options); return 0; - - error: - VIR_FREE(options); - return -1; } @@ -2983,36 +2966,35 @@ static int virStorageSourceParseBackingColon(virStorageSourcePtr src, const char *path) { - char *protocol = NULL; + VIR_AUTOFREE(char *) protocol = NULL; const char *p; - int ret = -1; if (!(p = strchr(path, ':'))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid backing protocol string '%s'"), path); - goto cleanup; + return -1; } if (VIR_STRNDUP(protocol, path, p - path) < 0) - goto cleanup; + return -1; if ((src->protocol = virStorageNetProtocolTypeFromString(protocol)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid backing protocol '%s'"), protocol); - goto cleanup; + return -1; } switch ((virStorageNetProtocol) src->protocol) { case VIR_STORAGE_NET_PROTOCOL_NBD: if (virStorageSourceParseNBDColonString(path, src) < 0) - goto cleanup; + return -1; break; case VIR_STORAGE_NET_PROTOCOL_RBD: if (virStorageSourceParseRBDColonString(path, src) < 0) - goto cleanup; + return -1; break; case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: @@ -3021,7 +3003,7 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, virReportError(VIR_ERR_INTERNAL_ERROR, _("backing store parser is not implemented for protocol %s"), protocol); - goto cleanup; + return -1; case VIR_STORAGE_NET_PROTOCOL_HTTP: case VIR_STORAGE_NET_PROTOCOL_HTTPS: @@ -3035,14 +3017,10 @@ virStorageSourceParseBackingColon(virStorageSourcePtr src, virReportError(VIR_ERR_INTERNAL_ERROR, _("malformed backing store path for protocol %s"), protocol); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(protocol); - return ret; + return 0; } @@ -3593,7 +3571,7 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, virJSONValuePtr deflattened = NULL; virJSONValuePtr file; const char *drvname; - char *str = NULL; + VIR_AUTOFREE(char *) str = NULL; size_t i; int ret = -1; @@ -3628,7 +3606,6 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, "driver '%s'"), drvname); cleanup: - VIR_FREE(str); virJSONValueFree(deflattened); return ret; } @@ -3997,8 +3974,8 @@ virStorageFileCanonicalizePath(const char *path, bool beginDoubleSlash = false; char **components = NULL; size_t ncomponents = 0; - char *linkpath = NULL; - char *currentpath = NULL; + VIR_AUTOFREE(char *) linkpath = NULL; + VIR_AUTOFREE(char *) currentpath = NULL; size_t i = 0; size_t j = 0; int rc; @@ -4131,8 +4108,6 @@ virStorageFileCanonicalizePath(const char *path, cleanup: virHashFree(cycle); virStringListFreeCount(components, ncomponents); - VIR_FREE(linkpath); - VIR_FREE(currentpath); return ret; } @@ -4175,25 +4150,22 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr top, char **relpath) { virStorageSourcePtr next; - char *tmp = NULL; - char *path = NULL; - char ret = -1; + VIR_AUTOFREE(char *) tmp = NULL; + VIR_AUTOFREE(char *) path = NULL; *relpath = NULL; for (next = top; virStorageSourceIsBacking(next); next = next->backingStore) { - if (!next->relPath) { - ret = 1; - goto cleanup; - } + if (!next->relPath) + return 1; if (!(tmp = virStorageFileRemoveLastPathComponent(path))) - goto cleanup; + return -1; VIR_FREE(path); if (virAsprintf(&path, "%s%s", tmp, next->relPath) < 0) - goto cleanup; + return -1; VIR_FREE(tmp); @@ -4205,18 +4177,12 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr top, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to resolve relative backing name: " "base image is not in backing chain")); - goto cleanup; + return -1; } - *relpath = path; - path = NULL; - - ret = 0; + VIR_STEAL_PTR(*relpath, path); - cleanup: - VIR_FREE(path); - VIR_FREE(tmp); - return ret; + return 0; } @@ -4867,7 +4833,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, { int ret = -1; const char *uniqueName; - char *buf = NULL; + VIR_AUTOFREE(char *) buf = NULL; ssize_t headerLen; virStorageSourcePtr backingStore = NULL; int backingFormat; @@ -4954,7 +4920,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, cleanup: if (virStorageSourceHasBacking(src)) src->backingStore->id = depth; - VIR_FREE(buf); virStorageFileDeinit(src); virStorageSourceFree(backingStore); return ret; @@ -5026,7 +4991,7 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src, char **backing) { virStorageSourcePtr tmp = NULL; - char *buf = NULL; + VIR_AUTOFREE(char *) buf = NULL; ssize_t headerLen; int ret = -1; int rv; @@ -5063,7 +5028,6 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src, ret = 0; cleanup: - VIR_FREE(buf); virStorageSourceFree(tmp); return ret; -- 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> --- src/test/test_driver.c | 179 ++++++++++++++++------------------------- 1 file changed, 69 insertions(+), 110 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index ff773f39b0..dc2b72d31a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -200,7 +200,7 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, void **data) { testDomainNamespaceDefPtr nsdata = NULL; - xmlNodePtr *nodes = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; int tmp, n; size_t i; unsigned int tmpuint; @@ -233,7 +233,6 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, nsdata->snap_nodes[nsdata->num_snap_nodes] = newnode; nsdata->num_snap_nodes++; } - VIR_FREE(nodes); tmp = virXPathBoolean("boolean(./test:transient)", ctxt); if (tmp == -1) { @@ -280,7 +279,6 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, return 0; error: - VIR_FREE(nodes); testDomainDefNamespaceFree(nsdata); return -1; } @@ -695,11 +693,10 @@ testParseXMLDocFromFile(xmlNodePtr node, const char *file, const char *type) xmlNodePtr ret = NULL; xmlDocPtr doc = NULL; char *absFile = NULL; - char *relFile = virXMLPropString(node, "file"); + VIR_AUTOFREE(char *) relFile = NULL; - if (relFile != NULL) { + if ((relFile = virXMLPropString(node, "file"))) { absFile = testBuildFilename(file, relFile); - VIR_FREE(relFile); if (!absFile) { virReportError(VIR_ERR_INTERNAL_ERROR, _("resolving %s filename"), type); @@ -722,14 +719,13 @@ testParseXMLDocFromFile(xmlNodePtr node, const char *file, const char *type) error: xmlFreeDoc(doc); - VIR_FREE(absFile); return ret; } static int testParseNodeInfo(virNodeInfoPtr nodeInfo, xmlXPathContextPtr ctxt) { - char *str; + VIR_AUTOFREE(char *) str = NULL; long l; int ret; @@ -794,10 +790,8 @@ testParseNodeInfo(virNodeInfoPtr nodeInfo, xmlXPathContextPtr ctxt) if (virStrcpyStatic(nodeInfo->model, str) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Model %s too big for destination"), str); - VIR_FREE(str); goto error; } - VIR_FREE(str); } ret = virXPathLong("string(/node/memory[1])", ctxt, &l); @@ -877,12 +871,12 @@ testParseDomains(testDriverPtr privconn, { int num, ret = -1; size_t i; - xmlNodePtr *nodes = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; virDomainObjPtr obj = NULL; num = virXPathNodeSet("/node/domain", ctxt, &nodes); if (num < 0) - goto error; + return -1; for (i = 0; i < num; i++) { virDomainDefPtr def; @@ -928,7 +922,6 @@ testParseDomains(testDriverPtr privconn, ret = 0; error: virDomainObjEndAPI(&obj); - VIR_FREE(nodes); return ret; } @@ -938,38 +931,35 @@ testParseNetworks(testDriverPtr privconn, const char *file, xmlXPathContextPtr ctxt) { - int num, ret = -1; + int num; size_t i; - xmlNodePtr *nodes = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; virNetworkObjPtr obj; num = virXPathNodeSet("/node/network", ctxt, &nodes); if (num < 0) - goto error; + return -1; for (i = 0; i < num; i++) { virNetworkDefPtr def; xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "network"); if (!node) - goto error; + return -1; def = virNetworkDefParseNode(ctxt->doc, node); if (!def) - goto error; + return -1; if (!(obj = virNetworkObjAssignDef(privconn->networks, def, 0))) { virNetworkDefFree(def); - goto error; + return -1; } virNetworkObjSetActive(obj, true); virNetworkObjEndAPI(&obj); } - ret = 0; - error: - VIR_FREE(nodes); - return ret; + return 0; } @@ -978,39 +968,36 @@ testParseInterfaces(testDriverPtr privconn, const char *file, xmlXPathContextPtr ctxt) { - int num, ret = -1; + int num; size_t i; - xmlNodePtr *nodes = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; virInterfaceObjPtr obj; num = virXPathNodeSet("/node/interface", ctxt, &nodes); if (num < 0) - goto error; + return -1; for (i = 0; i < num; i++) { virInterfaceDefPtr def; xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "interface"); if (!node) - goto error; + return -1; def = virInterfaceDefParseNode(ctxt->doc, node); if (!def) - goto error; + return -1; if (!(obj = virInterfaceObjListAssignDef(privconn->ifaces, def))) { virInterfaceDefFree(def); - goto error; + return -1; } virInterfaceObjSetActive(obj, true); virInterfaceObjEndAPI(&obj); } - ret = 0; - error: - VIR_FREE(nodes); - return ret; + return 0; } @@ -1021,51 +1008,47 @@ testOpenVolumesForPool(const char *file, int objidx) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(obj); - char *vol_xpath; size_t i; - int num, ret = -1; - xmlNodePtr *nodes = NULL; + int num; + VIR_AUTOFREE(char *) vol_xpath = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; VIR_AUTOPTR(virStorageVolDef) volDef = NULL; /* Find storage volumes */ if (virAsprintf(&vol_xpath, "/node/pool[%d]/volume", objidx) < 0) - goto error; + return -1; num = virXPathNodeSet(vol_xpath, ctxt, &nodes); - VIR_FREE(vol_xpath); if (num < 0) - goto error; + return -1; for (i = 0; i < num; i++) { xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "volume"); if (!node) - goto error; + return -1; if (!(volDef = virStorageVolDefParseNode(def, ctxt->doc, node, 0))) - goto error; + return -1; if (!volDef->target.path) { if (virAsprintf(&volDef->target.path, "%s/%s", def->target.path, volDef->name) < 0) - goto error; + return -1; } if (!volDef->key && VIR_STRDUP(volDef->key, volDef->target.path) < 0) - goto error; + return -1; if (virStoragePoolObjAddVol(obj, volDef) < 0) - goto error; + return -1; def->allocation += volDef->target.allocation; def->available = (def->capacity - def->allocation); volDef = NULL; } - ret = 0; - error: - VIR_FREE(nodes); - return ret; + return 0; } @@ -1074,50 +1057,47 @@ testParseStorage(testDriverPtr privconn, const char *file, xmlXPathContextPtr ctxt) { - int num, ret = -1; + int num; size_t i; - xmlNodePtr *nodes = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; virStoragePoolObjPtr obj; num = virXPathNodeSet("/node/pool", ctxt, &nodes); if (num < 0) - goto error; + return -1; for (i = 0; i < num; i++) { virStoragePoolDefPtr def; xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "pool"); if (!node) - goto error; + return -1; def = virStoragePoolDefParseNode(ctxt->doc, node); if (!def) - goto error; + return -1; if (!(obj = virStoragePoolObjAssignDef(privconn->pools, def, false))) { virStoragePoolDefFree(def); - goto error; + return -1; } if (testStoragePoolObjSetDefaults(obj) == -1) { virStoragePoolObjEndAPI(&obj); - goto error; + return -1; } virStoragePoolObjSetActive(obj, true); /* Find storage volumes */ if (testOpenVolumesForPool(file, ctxt, obj, i+1) < 0) { virStoragePoolObjEndAPI(&obj); - goto error; + return -1; } virStoragePoolObjEndAPI(&obj); } - ret = 0; - error: - VIR_FREE(nodes); - return ret; + return 0; } @@ -1126,79 +1106,70 @@ testParseNodedevs(testDriverPtr privconn, const char *file, xmlXPathContextPtr ctxt) { - int num, ret = -1; + int num; size_t i; - xmlNodePtr *nodes = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; virNodeDeviceObjPtr obj; num = virXPathNodeSet("/node/device", ctxt, &nodes); if (num < 0) - goto error; + return -1; for (i = 0; i < num; i++) { virNodeDeviceDefPtr def; xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "nodedev"); if (!node) - goto error; + return -1; def = virNodeDeviceDefParseNode(ctxt->doc, node, 0, NULL); if (!def) - goto error; + return -1; if (!(obj = virNodeDeviceObjListAssignDef(privconn->devs, def))) { virNodeDeviceDefFree(def); - goto error; + return -1; } virNodeDeviceObjSetSkipUpdateCaps(obj, true); virNodeDeviceObjEndAPI(&obj); } - ret = 0; - error: - VIR_FREE(nodes); - return ret; + return 0; } static int testParseAuthUsers(testDriverPtr privconn, xmlXPathContextPtr ctxt) { - int num, ret = -1; + int num; size_t i; - xmlNodePtr *nodes = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; num = virXPathNodeSet("/node/auth/user", ctxt, &nodes); if (num < 0) - goto error; + return -1; privconn->numAuths = num; if (num && VIR_ALLOC_N(privconn->auths, num) < 0) - goto error; + return -1; for (i = 0; i < num; i++) { - char *username, *password; + VIR_AUTOFREE(char *) username = NULL; ctxt->node = nodes[i]; username = virXPathString("string(.)", ctxt); if (!username || STREQ(username, "")) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing username in /node/auth/user field")); - VIR_FREE(username); - goto error; + return -1; } /* This field is optional. */ - password = virXMLPropString(nodes[i], "password"); - - privconn->auths[i].username = username; - privconn->auths[i].password = password; + privconn->auths[i].password = virXMLPropString(nodes[i], "password"); + VIR_STEAL_PTR(privconn->auths[i].username, username); } - ret = 0; - error: - VIR_FREE(nodes); - return ret; + return 0; } static int @@ -1348,7 +1319,8 @@ testConnectAuthenticate(virConnectPtr conn, testDriverPtr privconn = conn->privateData; int ret = -1; ssize_t i; - char *username = NULL, *password = NULL; + VIR_AUTOFREE(char *) username = NULL; + VIR_AUTOFREE(char *) password = NULL; virObjectLock(privconn); if (privconn->numAuths == 0) { @@ -1388,8 +1360,6 @@ testConnectAuthenticate(virConnectPtr conn, ret = 0; cleanup: virObjectUnlock(privconn); - VIR_FREE(username); - VIR_FREE(password); return ret; } @@ -1977,7 +1947,7 @@ testDomainSaveFlags(virDomainPtr domain, const char *path, const char *dxml, unsigned int flags) { testDriverPtr privconn = domain->conn->privateData; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; int fd = -1; int len; virDomainObjPtr privdom; @@ -2052,8 +2022,6 @@ testDomainSaveFlags(virDomainPtr domain, const char *path, ret = 0; cleanup: - VIR_FREE(xml); - /* Don't report failure in close or unlink, because * in either case we're already in a failure scenario * and have reported an earlier error */ @@ -2080,7 +2048,7 @@ testDomainRestoreFlags(virConnectPtr conn, unsigned int flags) { testDriverPtr privconn = conn->privateData; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; char magic[15]; int fd = -1; int len; @@ -2162,7 +2130,6 @@ testDomainRestoreFlags(virConnectPtr conn, cleanup: virDomainDefFree(def); - VIR_FREE(xml); VIR_FORCE_CLOSE(fd); virDomainObjEndAPI(&dom); virObjectEventStateQueue(privconn->eventState, event); @@ -2578,8 +2545,7 @@ testDomainRenameCallback(virDomainObjPtr privdom, virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; - char *new_dom_name = NULL; - char *old_dom_name = NULL; + VIR_AUTOFREE(char *) new_dom_name = NULL; virCheckFlags(0, -1); @@ -2597,9 +2563,8 @@ testDomainRenameCallback(virDomainObjPtr privdom, VIR_DOMAIN_EVENT_UNDEFINED_RENAMED); /* Switch name in domain definition. */ - old_dom_name = privdom->def->name; - privdom->def->name = new_dom_name; - new_dom_name = NULL; + VIR_FREE(privdom->def->name); + VIR_STEAL_PTR(privdom->def->name, new_dom_name); event_new = virDomainEventLifecycleNewFromObj(privdom, VIR_DOMAIN_EVENT_DEFINED, @@ -2607,8 +2572,6 @@ testDomainRenameCallback(virDomainObjPtr privdom, ret = 0; cleanup: - VIR_FREE(old_dom_name); - VIR_FREE(new_dom_name); virObjectEventStateQueue(driver->eventState, event_old); virObjectEventStateQueue(driver->eventState, event_new); return ret; @@ -5471,7 +5434,7 @@ testNodeDeviceMockCreateVport(testDriverPtr driver, const char *wwnn, const char *wwpn) { - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; virNodeDeviceDefPtr def = NULL; virNodeDevCapsDefPtr caps; virNodeDeviceObjPtr obj = NULL, objcopy = NULL; @@ -5540,7 +5503,6 @@ testNodeDeviceMockCreateVport(testDriverPtr driver, virObjectEventStateQueue(driver->eventState, event); cleanup: - VIR_FREE(xml); virNodeDeviceDefFree(def); return obj; } @@ -5553,7 +5515,8 @@ testNodeDeviceCreateXML(virConnectPtr conn, { testDriverPtr driver = conn->privateData; virNodeDeviceDefPtr def = NULL; - char *wwnn = NULL, *wwpn = NULL; + VIR_AUTOFREE(char *) wwnn = NULL; + VIR_AUTOFREE(char *) wwpn = NULL; virNodeDevicePtr dev = NULL, ret = NULL; virNodeDeviceObjPtr obj = NULL; virNodeDeviceDefPtr objdef; @@ -5597,8 +5560,6 @@ testNodeDeviceCreateXML(virConnectPtr conn, virNodeDeviceObjEndAPI(&obj); virNodeDeviceDefFree(def); virObjectUnref(dev); - VIR_FREE(wwnn); - VIR_FREE(wwpn); return ret; } @@ -5610,7 +5571,8 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) virNodeDeviceObjPtr obj = NULL; virNodeDeviceObjPtr parentobj = NULL; virNodeDeviceDefPtr def; - char *wwnn = NULL, *wwpn = NULL; + VIR_AUTOFREE(char *) wwnn = NULL; + VIR_AUTOFREE(char *) wwpn = NULL; virObjectEventPtr event = NULL; if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) @@ -5649,8 +5611,6 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) cleanup: virNodeDeviceObjEndAPI(&obj); virObjectEventStateQueue(driver->eventState, event); - VIR_FREE(wwnn); - VIR_FREE(wwpn); return ret; } @@ -6342,7 +6302,7 @@ testDomainSnapshotCreateXML(virDomainPtr domain, virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virObjectEventPtr event = NULL; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; bool update_current = true; bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; @@ -6427,7 +6387,6 @@ testDomainSnapshotCreateXML(virDomainPtr domain, snapshot = virGetDomainSnapshot(domain, snap->def->name); cleanup: - VIR_FREE(xml); if (vm) { if (snapshot) { virDomainSnapshotObjPtr other; -- 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> --- tests/storagebackendsheepdogtest.c | 50 ++++++++++++------------------ tests/storagepoolxml2argvtest.c | 18 +++-------- tests/storagepoolxml2xmltest.c | 33 ++++++-------------- tests/storagevolxml2argvtest.c | 42 +++++++++---------------- tests/storagevolxml2xmltest.c | 38 +++++++---------------- tests/virstoragetest.c | 45 +++++++-------------------- tests/virstorageutiltest.c | 7 ++--- 7 files changed, 75 insertions(+), 158 deletions(-) diff --git a/tests/storagebackendsheepdogtest.c b/tests/storagebackendsheepdogtest.c index 03ddf76d65..1806f9725f 100644 --- a/tests/storagebackendsheepdogtest.c +++ b/tests/storagebackendsheepdogtest.c @@ -57,32 +57,27 @@ test_node_info_parser(const void *opaque) { const struct testNodeInfoParserData *data = opaque; collie_test test = data->data; - int ret = -1; - char *output = NULL; + VIR_AUTOFREE(char *) output = NULL; VIR_AUTOPTR(virStoragePoolDef) pool = NULL; if (!(pool = virStoragePoolDefParseFile(data->poolxml))) - goto cleanup; + return -1; if (VIR_STRDUP(output, test.output) < 0) - goto cleanup; + return -1; if (virStorageBackendSheepdogParseNodeInfo(pool, output) != test.expected_return) - goto cleanup; + return -1; - if (test.expected_return) { - ret = 0; - goto cleanup; - } + if (test.expected_return) + return 0; if (pool->capacity == test.expected_capacity && pool->allocation == test.expected_allocation) - ret = 0; + return 0; - cleanup: - VIR_FREE(output); - return ret; + return -1; } static int @@ -90,36 +85,31 @@ test_vdi_list_parser(const void *opaque) { const struct testVDIListParserData *data = opaque; collie_test test = data->data; - int ret = -1; - char *output = NULL; + VIR_AUTOFREE(char *) output = NULL; VIR_AUTOPTR(virStoragePoolDef) pool = NULL; VIR_AUTOPTR(virStorageVolDef) vol = NULL; if (!(pool = virStoragePoolDefParseFile(data->poolxml))) - goto cleanup; + return -1; if (!(vol = virStorageVolDefParseFile(pool, data->volxml, 0))) - goto cleanup; + return -1; if (VIR_STRDUP(output, test.output) < 0) - goto cleanup; + return -1; if (virStorageBackendSheepdogParseVdiList(vol, output) != test.expected_return) - goto cleanup; + return -1; - if (test.expected_return) { - ret = 0; - goto cleanup; - } + if (test.expected_return) + return 0; if (vol->target.capacity == test.expected_capacity && vol->target.allocation == test.expected_allocation) - ret = 0; + return 0; - cleanup: - VIR_FREE(output); - return ret; + return -1; } @@ -127,8 +117,8 @@ static int mymain(void) { int ret = 0; - char *poolxml = NULL; - char *volxml = NULL; + VIR_AUTOFREE(char *) poolxml = NULL; + VIR_AUTOFREE(char *) volxml = NULL; collie_test node_info_tests[] = { {"", -1, 0, 0}, @@ -215,8 +205,6 @@ mymain(void) } cleanup: - VIR_FREE(poolxml); - VIR_FREE(volxml); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c index 116a75c3ea..e76e86527f 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -88,7 +88,6 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: - VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&pool); if (shouldFail) { virResetLastError(); @@ -106,27 +105,20 @@ struct testInfo { static int testCompareXMLToArgvHelper(const void *data) { - int result = -1; const struct testInfo *info = data; - char *poolxml = NULL; - char *cmdline = NULL; + VIR_AUTOFREE(char *) poolxml = NULL; + VIR_AUTOFREE(char *) cmdline = NULL; if (virAsprintf(&poolxml, "%s/storagepoolxml2xmlin/%s.xml", abs_srcdir, info->pool) < 0) - goto cleanup; + return -1; if (virAsprintf(&cmdline, "%s/storagepoolxml2argvdata/%s%s.argv", abs_srcdir, info->pool, info->platformSuffix) < 0 && !info->shouldFail) - goto cleanup; - - result = testCompareXMLToArgvFiles(info->shouldFail, poolxml, cmdline); - - cleanup: - VIR_FREE(poolxml); - VIR_FREE(cmdline); + return -1; - return result; + return testCompareXMLToArgvFiles(info->shouldFail, poolxml, cmdline); } diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index c8d5c41cd4..bd3408e8b8 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -18,47 +18,34 @@ static int testCompareXMLToXMLFiles(const char *inxml, const char *outxml) { - char *actual = NULL; - int ret = -1; + VIR_AUTOFREE(char *) actual = NULL; VIR_AUTOPTR(virStoragePoolDef) dev = NULL; if (!(dev = virStoragePoolDefParseFile(inxml))) - goto fail; + return -1; if (!(actual = virStoragePoolDefFormat(dev))) - goto fail; + return -1; if (virTestCompareToFile(actual, outxml) < 0) - goto fail; + return -1; - ret = 0; - - fail: - VIR_FREE(actual); - return ret; + return 0; } static int testCompareXMLToXMLHelper(const void *data) { - int result = -1; - char *inxml = NULL; - char *outxml = NULL; + VIR_AUTOFREE(char *) inxml = NULL; + VIR_AUTOFREE(char *) outxml = NULL; if (virAsprintf(&inxml, "%s/storagepoolxml2xmlin/%s.xml", abs_srcdir, (const char*)data) < 0 || virAsprintf(&outxml, "%s/storagepoolxml2xmlout/%s.xml", - abs_srcdir, (const char*)data) < 0) { - goto cleanup; - } - - result = testCompareXMLToXMLFiles(inxml, outxml); - - cleanup: - VIR_FREE(inxml); - VIR_FREE(outxml); + abs_srcdir, (const char*)data) < 0) + return -1; - return result; + return testCompareXMLToXMLFiles(inxml, outxml); } static int diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 38bb2ae004..3a4c020f68 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -42,9 +42,9 @@ testCompareXMLToArgvFiles(bool shouldFail, unsigned int flags, unsigned long parse_flags) { - char *actualCmdline = NULL; virStorageVolEncryptConvertStep convertStep = VIR_STORAGE_VOL_ENCRYPT_NONE; int ret = -1; + VIR_AUTOFREE(char *) actualCmdline = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOPTR(virStorageVolDef) vol = NULL; VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; @@ -109,7 +109,7 @@ testCompareXMLToArgvFiles(bool shouldFail, goto cleanup; } else { char *createCmdline = actualCmdline; - char *cvtCmdline; + VIR_AUTOFREE(char *) cvtCmdline = NULL; int rc; if (!(cvtCmdline = virCommandToString(cmd, false))) @@ -119,7 +119,6 @@ testCompareXMLToArgvFiles(bool shouldFail, createCmdline, cvtCmdline); VIR_FREE(createCmdline); - VIR_FREE(cvtCmdline); if (rc < 0) goto cleanup; } @@ -139,7 +138,6 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: - VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&obj); return ret; } @@ -158,45 +156,35 @@ struct testInfo { static int testCompareXMLToArgvHelper(const void *data) { - int result = -1; const struct testInfo *info = data; - char *poolxml = NULL; - char *inputpoolxml = NULL; - char *volxml = NULL; - char *inputvolxml = NULL; - char *cmdline = NULL; + VIR_AUTOFREE(char *) poolxml = NULL; + VIR_AUTOFREE(char *) inputpoolxml = NULL; + VIR_AUTOFREE(char *) volxml = NULL; + VIR_AUTOFREE(char *) inputvolxml = NULL; + VIR_AUTOFREE(char *) cmdline = NULL; if (info->inputvol && virAsprintf(&inputvolxml, "%s/storagevolxml2xmlin/%s.xml", abs_srcdir, info->inputvol) < 0) - goto cleanup; + return -1; if (info->inputpool && virAsprintf(&inputpoolxml, "%s/storagepoolxml2xmlin/%s.xml", abs_srcdir, info->inputpool) < 0) - goto cleanup; + return -1; if (virAsprintf(&poolxml, "%s/storagepoolxml2xmlin/%s.xml", abs_srcdir, info->pool) < 0 || virAsprintf(&volxml, "%s/storagevolxml2xmlin/%s.xml", abs_srcdir, info->vol) < 0) { - goto cleanup; + return -1; } if (virAsprintf(&cmdline, "%s/storagevolxml2argvdata/%s.argv", abs_srcdir, info->cmdline) < 0 && !info->shouldFail) - goto cleanup; - - result = testCompareXMLToArgvFiles(info->shouldFail, poolxml, volxml, - inputpoolxml, inputvolxml, - cmdline, info->flags, - info->parseflags); - - cleanup: - VIR_FREE(poolxml); - VIR_FREE(volxml); - VIR_FREE(inputvolxml); - VIR_FREE(inputpoolxml); - VIR_FREE(cmdline); + return -1; - return result; + return testCompareXMLToArgvFiles(info->shouldFail, poolxml, volxml, + inputpoolxml, inputvolxml, + cmdline, info->flags, + info->parseflags); } diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index cb78bd5b28..7c5d8e7e38 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -17,28 +17,23 @@ static int testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, const char *outxml, unsigned int flags) { - char *actual = NULL; - int ret = -1; + VIR_AUTOFREE(char *) actual = NULL; VIR_AUTOPTR(virStoragePoolDef) pool = NULL; VIR_AUTOPTR(virStorageVolDef) dev = NULL; if (!(pool = virStoragePoolDefParseFile(poolxml))) - goto fail; + return -1; if (!(dev = virStorageVolDefParseFile(pool, inxml, flags))) - goto fail; + return -1; if (!(actual = virStorageVolDefFormat(pool, dev))) - goto fail; + return -1; if (virTestCompareToFile(actual, outxml) < 0) - goto fail; + return -1; - ret = 0; - - fail: - VIR_FREE(actual); - return ret; + return 0; } struct testInfo { @@ -50,29 +45,20 @@ struct testInfo { static int testCompareXMLToXMLHelper(const void *data) { - int result = -1; const struct testInfo *info = data; - char *poolxml = NULL; - char *inxml = NULL; - char *outxml = NULL; + VIR_AUTOFREE(char *) poolxml = NULL; + VIR_AUTOFREE(char *) inxml = NULL; + VIR_AUTOFREE(char *) outxml = NULL; if (virAsprintf(&poolxml, "%s/storagepoolxml2xmlin/%s.xml", abs_srcdir, info->pool) < 0 || virAsprintf(&inxml, "%s/storagevolxml2xmlin/%s.xml", abs_srcdir, info->name) < 0 || virAsprintf(&outxml, "%s/storagevolxml2xmlout/%s.xml", - abs_srcdir, info->name) < 0) { - goto cleanup; - } - - result = testCompareXMLToXMLFiles(poolxml, inxml, outxml, info->flags); - - cleanup: - VIR_FREE(poolxml); - VIR_FREE(inxml); - VIR_FREE(outxml); + abs_srcdir, info->name) < 0) + return -1; - return result; + return testCompareXMLToXMLFiles(poolxml, inxml, outxml, info->flags); } diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index c448d8b733..71c371891f 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -129,7 +129,7 @@ testPrepImages(void) { int ret = EXIT_FAILURE; VIR_AUTOPTR(virCommand) cmd = NULL; - char *buf = NULL; + VIR_AUTOFREE(char *) buf = NULL; bool compat = false; qemuimg = virFindFileInPath("qemu-img"); @@ -245,7 +245,6 @@ testPrepImages(void) ret = 0; cleanup: - VIR_FREE(buf); if (ret) testCleanupImages(); return ret; @@ -313,7 +312,7 @@ testStorageChain(const void *args) virStorageSourcePtr meta; virStorageSourcePtr elt; size_t i = 0; - char *broken = NULL; + VIR_AUTOFREE(char *) broken = NULL; meta = testStorageFileGetMetadata(data->start, data->format, -1, -1); if (!meta) { @@ -349,8 +348,8 @@ testStorageChain(const void *args) elt = meta; while (virStorageSourceIsBacking(elt)) { - char *expect = NULL; - char *actual = NULL; + VIR_AUTOFREE(char *) expect = NULL; + VIR_AUTOFREE(char *) actual = NULL; if (i == data->nfiles) { fprintf(stderr, "probed chain was too long\n"); @@ -379,18 +378,12 @@ testStorageChain(const void *args) elt->format, virStorageNetProtocolTypeToString(elt->protocol), NULLSTR(elt->nhosts ? elt->hosts[0].name : NULL)) < 0) { - VIR_FREE(expect); - VIR_FREE(actual); goto cleanup; } if (STRNEQ(expect, actual)) { virTestDifference(stderr, expect, actual); - VIR_FREE(expect); - VIR_FREE(actual); goto cleanup; } - VIR_FREE(expect); - VIR_FREE(actual); elt = elt->backingStore; i++; } @@ -401,7 +394,6 @@ testStorageChain(const void *args) ret = 0; cleanup: - VIR_FREE(broken); virStorageSourceFree(meta); return ret; } @@ -539,8 +531,7 @@ static int testPathCanonicalize(const void *args) { const struct testPathCanonicalizeData *data = args; - char *canon = NULL; - int ret = -1; + VIR_AUTOFREE(char *) canon = NULL; canon = virStorageFileCanonicalizePath(data->path, testPathCanonicalizeReadlink, @@ -551,15 +542,10 @@ testPathCanonicalize(const void *args) "path canonicalization of '%s' failed: expected '%s' got '%s'\n", data->path, NULLSTR(data->expect), NULLSTR(canon)); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(canon); - - return ret; + return 0; } static virStorageSource backingchain[12]; @@ -629,14 +615,13 @@ static int testPathRelative(const void *args) { const struct testPathRelativeBacking *data = args; - char *actual = NULL; - int ret = -1; + VIR_AUTOFREE(char *) actual = NULL; if (virStorageFileGetRelativeBackingPath(data->top, data->base, &actual) < 0) { fprintf(stderr, "relative backing path resolution failed\n"); - goto cleanup; + return -1; } if (STRNEQ_NULLABLE(data->expect, actual)) { @@ -644,15 +629,10 @@ testPathRelative(const void *args) "expected '%s', got '%s'\n", data->top->path, data->base->path, NULLSTR(data->expect), NULLSTR(actual)); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FREE(actual); - - return ret; + return 0; } @@ -667,7 +647,7 @@ testBackingParse(const void *args) const struct testBackingParseData *data = args; virBuffer buf = VIR_BUFFER_INITIALIZER; virStorageSourcePtr src = NULL; - char *xml = NULL; + VIR_AUTOFREE(char *) xml = NULL; int ret = -1; if (!(src = virStorageSourceNewFromBackingAbsolute(data->backing))) { @@ -702,7 +682,6 @@ testBackingParse(const void *args) cleanup: virStorageSourceFree(src); virBufferFreeAndReset(&buf); - VIR_FREE(xml); return ret; } diff --git a/tests/virstorageutiltest.c b/tests/virstorageutiltest.c index d91c5d4d6f..766a910975 100644 --- a/tests/virstorageutiltest.c +++ b/tests/virstorageutiltest.c @@ -45,8 +45,8 @@ testGlusterExtractPoolSources(const void *opaque) .sources = NULL }; size_t i; - char *srcxmldata = NULL; - char *actual = NULL; + VIR_AUTOFREE(char *) srcxmldata = NULL; + VIR_AUTOFREE(char *) actual = NULL; int ret = -1; if (virTestLoadFile(data->srcxml, &srcxmldata) < 0) @@ -62,9 +62,6 @@ testGlusterExtractPoolSources(const void *opaque) ret = virTestCompareToFile(actual, data->dstxml); cleanup: - VIR_FREE(srcxmldata); - VIR_FREE(actual); - for (i = 0; i < list.nsources; i++) virStoragePoolSourceClear(&list.sources[i]); VIR_FREE(list.sources); -- 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> --- src/storage/storage_backend_logical.c | 3 +- src/storage/storage_backend_scsi.c | 12 +-- src/storage/storage_file_fs.c | 15 +-- src/storage/storage_util.c | 150 ++++++++++---------------- src/util/virstoragefile.c | 39 +++---- 5 files changed, 77 insertions(+), 142 deletions(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 2a205a4e95..8ae866f23e 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -911,7 +911,7 @@ static int virStorageBackendLogicalCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - int fd = -1; + VIR_AUTOCLOSE fd = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virErrorPtr err; struct stat sb; @@ -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 1a10e43647..2ae55af6c9 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,24 +72,19 @@ virStorageBackendSCSITriggerRescan(uint32_t host) virReportSystemError(errno, _("Could not open '%s' to trigger host scan"), path); - goto cleanup; + return -1; } if (safewrite(fd, LINUX_SYSFS_SCSI_HOST_SCAN_STRING, sizeof(LINUX_SYSFS_SCSI_HOST_SCAN_STRING)) < 0) { - VIR_FORCE_CLOSE(fd); virReportSystemError(errno, _("Write to '%s' to trigger host scan failed"), path); } - 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..0028389e60 100644 --- a/src/storage/storage_file_fs.c +++ b/src/storage/storage_file_fs.c @@ -83,7 +83,7 @@ virStorageFileBackendFileInit(virStorageSourcePtr src) static int virStorageFileBackendFileCreate(virStorageSourcePtr src) { - int fd = -1; + VIR_AUTOCLOSE fd = -1; mode_t mode = S_IRUSR; if (!src->readonly) @@ -95,7 +95,6 @@ virStorageFileBackendFileCreate(virStorageSourcePtr src) return -1; } - VIR_FORCE_CLOSE(fd); return 0; } @@ -121,7 +120,7 @@ virStorageFileBackendFileRead(virStorageSourcePtr src, size_t len, char **buf) { - int fd = -1; + VIR_AUTOCLOSE fd = -1; ssize_t ret = -1; if ((fd = virFileOpenAs(src->path, O_RDONLY, 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 d87d2eca1e..345566a3af 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; @@ -138,6 +137,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, int interval; VIR_AUTOFREE(char *) zerobuf = NULL; VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOCLOSE inputfd = -1; struct stat st; if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { @@ -145,7 +145,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, 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,8 +241,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr inputvol, unsigned int flags) { - int fd = -1; - int ret = -1; + VIR_AUTOCLOSE fd = -1; unsigned long long remain; struct stat st; gid_t gid; @@ -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,7 +404,7 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool, { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int ret = -1; - int fd = -1; + VIR_AUTOCLOSE fd = -1; int operation_flags; bool reflink_copy = false; mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE; @@ -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,7 +1210,7 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, { virStorageEncryptionPtr enc = vol->target.encryption; char *secretPath = NULL; - int fd = -1; + VIR_AUTOCLOSE fd = -1; uint8_t *secret = NULL; size_t secretlen = 0; virConnectPtr conn = NULL; @@ -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; @@ -1751,17 +1732,17 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype, unsigned int openflags, unsigned int readflags) { - int ret, fd = -1; + int ret; struct stat sb; + VIR_AUTOCLOSE fd = -1; VIR_AUTOFREE(char *) buf = NULL; ssize_t len = VIR_STORAGE_MAX_HEADER; - if ((ret = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) - goto cleanup; - fd = ret; + if ((fd = virStorageBackendVolOpen(target->path, &sb, openflags)) < 0) + return -1; if ((ret = virStorageBackendUpdateVolTargetInfoFD(target, fd, &sb)) < 0) - goto cleanup; + return ret; if ((voltype == VIR_STORAGE_VOL_FILE || voltype == VIR_STORAGE_VOL_BLOCK) && target->format != VIR_STORAGE_FILE_NONE) { @@ -1769,49 +1750,39 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype, if (storageBackendIsPloopDir(target->path)) { if ((ret = storageBackendRedoPloopUpdate(target, &sb, &fd, openflags)) < 0) - goto cleanup; + return ret; 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); - ret = -1; - 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); - ret = -1; + return -1; } - goto cleanup; } - if (virStorageSourceUpdateCapacity(target, buf, len, false) < 0) { - ret = -1; - goto cleanup; - } + if (virStorageSourceUpdateCapacity(target, buf, len, false) < 0) + return -1; } - if (withBlockVolFormat) { - if ((ret = virStorageBackendDetectBlockVolFormatFD(target, fd, - readflags)) < 0) - goto cleanup; - } + if (withBlockVolFormat) + return virStorageBackendDetectBlockVolFormatFD(target, fd, readflags); - cleanup: - VIR_FORCE_CLOSE(fd); - return ret; + return 0; } /* @@ -2617,9 +2588,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); @@ -2627,14 +2598,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) { @@ -2668,12 +2639,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); @@ -2682,24 +2653,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); } @@ -3392,7 +3353,7 @@ storageBackendProbeTarget(virStorageSourcePtr target, virStorageEncryptionPtr *encryption) { int backingStoreFormat; - int fd = -1; + VIR_AUTOCLOSE fd = -1; int ret = -1; int rc; virStorageSourcePtr meta = NULL; @@ -3499,7 +3460,6 @@ storageBackendProbeTarget(virStorageSourcePtr target, } cleanup: - VIR_FORCE_CLOSE(fd); virStorageSourceFree(meta); return ret; } @@ -3570,9 +3530,10 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) struct statvfs sb; struct stat statbuf; VIR_AUTOPTR(virStorageVolDef) vol = NULL; + VIR_AUTOCLOSE fd = -1; virStorageSourcePtr target = NULL; int direrr; - int fd = -1, ret = -1; + int ret = -1; if (virDirOpen(&dir, def->target.path) < 0) goto cleanup; @@ -3663,7 +3624,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 c2f044d2db..1d77f6ac4c 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; + VIR_AUTOCLOSE fd = -1; int rc; 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

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