Rather than having an error path, let's rework the code to allocate and fill into an @authdef variable and then steal that into @ret when we are successful leaving just a cleanup: path. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/util/virstoragefile.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 98f9bc803f..7fbeea78eb 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1879,26 +1879,26 @@ virStorageAuthDefFree(virStorageAuthDefPtr authdef) virStorageAuthDefPtr virStorageAuthDefCopy(const virStorageAuthDef *src) { - virStorageAuthDefPtr ret; + virStorageAuthDefPtr authdef; + 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) + goto cleanup; /* 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) + goto cleanup; + authdef->authType = src->authType; - if (virSecretLookupDefCopy(&ret->seclookupdef, &src->seclookupdef) < 0) - goto error; + if (virSecretLookupDefCopy(&authdef->seclookupdef, &src->seclookupdef) < 0) + goto cleanup; + VIR_STEAL_PTR(ret, authdef); + cleanup: + virStorageAuthDefFree(authdef); return ret; - - error: - virStorageAuthDefFree(ret); - return 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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/conf/domain_conf.c | 26 ++++++++++---------------- src/conf/storage_conf.c | 3 +-- src/qemu/qemu_parse_command.c | 3 +-- src/util/virstoragefile.c | 16 ++++++---------- src/util/virstoragefile.h | 2 ++ 5 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a33f18c957..2b1389035e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7578,11 +7578,10 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode, virDomainHostdevSubsysSCSIPtr def, xmlXPathContextPtr ctxt) { - int ret = -1; int auth_secret_usage = -1; xmlNodePtr cur; - virStorageAuthDefPtr authdef = NULL; virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &def->u.iscsi; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; /* For the purposes of command line creation, this needs to look * like a disk storage source */ @@ -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,29 +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; } VIR_STEAL_PTR(iscsisrc->src->auth, authdef); } cur = cur->next; } - ret = 0; - - cleanup: - virStorageAuthDefFree(authdef); - return ret; + return 0; } static int @@ -9683,7 +9678,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, virStorageEncryptionPtr encryption = NULL; char *serial = NULL; char *startupPolicy = NULL; - virStorageAuthDefPtr authdef = NULL; char *tray = NULL; char *removable = NULL; char *logical_block_size = NULL; @@ -9692,6 +9686,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, char *vendor = NULL; char *product = NULL; char *domain_name = NULL; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; if (!(def = virDomainDiskDefNew(xmlopt))) return NULL; @@ -10093,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 fbd62e1305..ead24816bc 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -458,11 +458,11 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, int nsource; size_t i; virStoragePoolOptionsPtr options; - virStorageAuthDefPtr authdef = NULL; char *name = NULL; char *port = NULL; char *ver = NULL; int n; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; relnode = ctxt->node; ctxt->node = node; @@ -614,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 679d49d442..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; @@ -151,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 7fbeea78eb..1eb1ede59d 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1879,25 +1879,23 @@ virStorageAuthDefFree(virStorageAuthDefPtr authdef) virStorageAuthDefPtr virStorageAuthDefCopy(const virStorageAuthDef *src) { - virStorageAuthDefPtr authdef; virStorageAuthDefPtr ret = NULL; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; if (VIR_ALLOC(authdef) < 0) return NULL; if (VIR_STRDUP(authdef->username, src->username) < 0) - goto cleanup; + return NULL; /* Not present for storage pool, but used for disk source */ if (VIR_STRDUP(authdef->secrettype, src->secrettype) < 0) - goto cleanup; + return NULL; authdef->authType = src->authType; if (virSecretLookupDefCopy(&authdef->seclookupdef, &src->seclookupdef) < 0) - goto cleanup; + return NULL; VIR_STEAL_PTR(ret, authdef); - cleanup: - virStorageAuthDefFree(authdef); return ret; } @@ -1907,10 +1905,10 @@ virStorageAuthDefParse(xmlNodePtr node, xmlXPathContextPtr ctxt) { xmlNodePtr saveNode = ctxt->node; - virStorageAuthDefPtr authdef = NULL; virStorageAuthDefPtr ret = NULL; xmlNodePtr secretnode = NULL; char *authtype = NULL; + VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; ctxt->node = node; @@ -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:")) @@ -2935,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..eacc927ea6 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -543,4 +543,6 @@ 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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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 ead24816bc..9563d2bc6b 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; + virStoragePoolSourcePtr ret = NULL; + VIR_AUTOPTR(virStoragePoolSource) def = 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..420601a303 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -145,11 +145,11 @@ virStorageBackendFileSystemNetFindPoolSources(const char *srcSpec, .sources = NULL } }; - virStoragePoolSourcePtr source = NULL; char *ret = NULL; size_t i; int retNFS = -1; int retGluster = 0; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; virCheckFlags(0, NULL); @@ -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..559189fd1d 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -489,10 +489,10 @@ virStorageBackendGlusterFindPoolSources(const char *srcSpec, .nsources = 0, .sources = NULL }; - virStoragePoolSourcePtr source = NULL; char *ret = NULL; int rc; size_t i; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; virCheckFlags(0, NULL); @@ -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..dc1a983b58 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -158,7 +158,6 @@ static char * virStorageBackendISCSIFindPoolSources(const char *srcSpec, unsigned int flags) { - virStoragePoolSourcePtr source = NULL; size_t ntargets = 0; char **targets = NULL; char *ret = NULL; @@ -169,6 +168,7 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec, .sources = NULL }; char *portal = NULL; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; virCheckFlags(0, 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..8d12c1a274 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -486,7 +486,6 @@ static char * virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, unsigned int flags) { - virStoragePoolSourcePtr source = NULL; size_t ntargets = 0; char **targets = NULL; char *ret = NULL; @@ -497,6 +496,7 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, .sources = NULL }; char *portal = NULL; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; virCheckFlags(0, 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..de221b4190 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4394,9 +4394,9 @@ testConnectFindStoragePoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, const char *srcSpec, unsigned int flags) { - virStoragePoolSourcePtr source = NULL; int pool_type; char *ret = NULL; + VIR_AUTOPTR(virStoragePoolSource) source = NULL; virCheckFlags(0, 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

Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/storage/storage_backend_gluster.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 559189fd1d..6759d7f1e5 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -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,8 +327,7 @@ 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); -- 2.20.1

Rather than having an error path, let's rework the code to allocate and fill into an @def variable and then steal that into @ret when we are successful leaving just a cleanup: path. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/conf/storage_conf.c | 113 ++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 57 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9563d2bc6b..0ebdbafa0c 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; + virStorageVolDefPtr def; + 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,14 +1321,17 @@ 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: + virStorageVolDefFree(def); VIR_FREE(nodes); VIR_FREE(allocation); VIR_FREE(capacity); @@ -1335,11 +1339,6 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, VIR_FREE(type); VIR_FREE(backingStore); return ret; - - error: - virStorageVolDefFree(ret); - ret = NULL; - goto cleanup; } -- 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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/conf/storage_conf.c | 3 +-- 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 | 3 +-- 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, 44 insertions(+), 75 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 0ebdbafa0c..e6accb14c6 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1168,7 +1168,6 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, xmlXPathContextPtr ctxt, unsigned int flags) { - virStorageVolDefPtr def; virStorageVolDefPtr ret = NULL; virStorageVolOptionsPtr options; char *type = NULL; @@ -1180,6 +1179,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, xmlNodePtr *nodes = NULL; size_t i; int n; + VIR_AUTOPTR(virStorageVolDef) def = NULL; virCheckFlags(VIR_VOL_XML_PARSE_NO_CAPACITY | VIR_VOL_XML_PARSE_OPT_CAPACITY, NULL); @@ -1331,7 +1331,6 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, VIR_STEAL_PTR(ret, def); cleanup: - virStorageVolDefFree(def); VIR_FREE(nodes); VIR_FREE(allocation); VIR_FREE(capacity); 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..6d6bd1a6ce 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -836,7 +836,6 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, virStorageVolPtr volume = NULL; esxPrivate *priv = pool->conn->privateData; virStoragePoolDef poolDef; - virStorageVolDefPtr def = NULL; char *tmp; char *unescapedDatastorePath = NULL; char *unescapedDirectoryName = NULL; @@ -852,6 +851,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, char *taskInfoErrorMessage = NULL; char *uuid_string = NULL; char *key = NULL; + VIR_AUTOPTR(virStorageVolDef) def = NULL; virCheckFlags(0, 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,6 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, esxPrivate *priv = pool->conn->privateData; virStoragePoolDef poolDef; char *sourceDatastorePath = NULL; - virStorageVolDefPtr def = NULL; char *tmp; char *unescapedDatastorePath = NULL; char *unescapedDirectoryName = NULL; @@ -1069,6 +1067,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, char *taskInfoErrorMessage = NULL; char *uuid_string = NULL; char *key = NULL; + VIR_AUTOPTR(virStorageVolDef) def = NULL; virCheckFlags(0, 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..cac5642afd 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1952,11 +1952,11 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, { virCheckFlags(0, NULL); - virStorageVolDefPtr voldef = NULL; virStoragePoolDefPtr spdef = NULL; virStorageVolPtr vol = NULL; virStorageVolPtr dup_vol = NULL; char *key = NULL; + VIR_AUTOPTR(virStorageVolDef) voldef = NULL; if (VIR_ALLOC(spdef) < 0) return 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 6759d7f1e5..5428bb92ba 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -242,12 +242,12 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, virStorageVolDefPtr *volptr) { int ret = -1; - virStorageVolDefPtr vol = NULL; glfs_fd_t *fd = NULL; virStorageSourcePtr meta = NULL; char *header = NULL; ssize_t len; int backingFormat; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; *volptr = NULL; @@ -331,7 +331,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, 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 8d12c1a274..42060dd758 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; uint32_t block_size; uint32_t nb_block; - int ret = -1; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; 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..c50a8ef5d4 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1820,8 +1820,8 @@ storageVolCreateXML(virStoragePoolPtr pool, virStoragePoolObjPtr obj; virStoragePoolDefPtr def; virStorageBackendPtr backend; - virStorageVolDefPtr voldef = NULL; virStorageVolPtr vol = NULL, newvol = NULL; + VIR_AUTOPTR(virStorageVolDef) voldef = 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,11 +1956,11 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, virStoragePoolObjPtr objsrc = NULL; virStorageBackendPtr backend; virStorageVolDefPtr voldefsrc = NULL; - virStorageVolDefPtr voldef = NULL; virStorageVolDefPtr shadowvol = NULL; virStorageVolPtr newvol = NULL; virStorageVolPtr vol = NULL; int buildret; + VIR_AUTOPTR(virStorageVolDef) voldef = NULL; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA | VIR_STORAGE_VOL_CREATE_REFLINK, @@ -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..1770d21f33 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3672,10 +3672,10 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) struct dirent *ent; struct statvfs sb; struct stat statbuf; - virStorageVolDefPtr vol = NULL; virStorageSourcePtr target = NULL; int direrr; int fd = -1, ret = -1; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; if (virDirOpen(&dir, def->target.path) < 0) goto cleanup; @@ -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,9 +3815,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, const char *dev) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virStorageVolDefPtr vol = NULL; char *devpath = NULL; int retval = -1; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; /* Check if the pool is using a stable target path. The call to * virStorageBackendStablePath will fail if the pool target path @@ -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 de221b4190..f07a075755 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,8 +5052,8 @@ testStorageVolCreateXML(virStoragePoolPtr pool, testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr obj; virStoragePoolDefPtr def; - virStorageVolDefPtr privvol = NULL; virStorageVolPtr ret = NULL; + VIR_AUTOPTR(virStorageVolDef) privvol = NULL; virCheckFlags(0, NULL); @@ -5098,7 +5097,6 @@ testStorageVolCreateXML(virStoragePoolPtr pool, privvol = NULL; cleanup: - virStorageVolDefFree(privvol); virStoragePoolObjEndAPI(&obj); return ret; } @@ -5113,8 +5111,9 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr obj; virStoragePoolDefPtr def; - virStorageVolDefPtr privvol = NULL, origvol = NULL; + virStorageVolDefPtr origvol = NULL; virStorageVolPtr ret = NULL; + VIR_AUTOPTR(virStorageVolDef) privvol = 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..ae16094dbe 100644 --- a/src/vbox/vbox_storage.c +++ b/src/vbox/vbox_storage.c @@ -401,7 +401,6 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool, const char *xml, unsigned int flags) { vboxDriverPtr data = pool->conn->privateData; - virStorageVolDefPtr def = NULL; PRUnichar *hddFormatUtf16 = NULL; PRUnichar *hddNameUtf16 = NULL; virStoragePoolDef poolDef; @@ -415,6 +414,7 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool, PRUint32 variant = HardDiskVariant_Standard; resultCodeUnion resultCode; virStorageVolPtr ret = NULL; + VIR_AUTOPTR(virStorageVolDef) def = NULL; if (!data->vboxObj) return ret; @@ -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..4c98decc4d 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -48,10 +48,11 @@ testCompareXMLToArgvFiles(bool shouldFail, virCommandPtr cmd = NULL; - virStorageVolDefPtr vol = NULL, inputvol = NULL; virStoragePoolDefPtr def = NULL; virStoragePoolDefPtr inputpool = NULL; virStoragePoolObjPtr obj = NULL; + VIR_AUTOPTR(virStorageVolDef) vol = NULL; + VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; @@ -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

Rather than having an error path, let's rework the code to allocate and fill into an @def variable and then steal that into @ret when we are successful leaving just a cleanup: path. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/storage_conf.c | 114 ++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 58 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index e6accb14c6..4a52b1497b 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; + virStoragePoolDefPtr def; + 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: + virStoragePoolDefFree(def); VIR_FREE(uuid); VIR_FREE(type); VIR_FREE(target_path); return ret; - - error: - virStoragePoolDefFree(ret); - ret = NULL; - goto cleanup; } -- 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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/conf/domain_conf.c | 3 +-- src/conf/storage_conf.c | 3 +-- src/conf/storage_conf.h | 1 + src/conf/virstorageobj.c | 27 +++++++++++---------------- src/phyp/phyp_driver.c | 3 +-- src/storage/storage_driver.c | 6 ++---- src/test/test_driver.c | 6 ++---- tests/storagebackendsheepdogtest.c | 6 ++---- tests/storagepoolxml2xmltest.c | 3 +-- tests/storagevolxml2argvtest.c | 3 +-- tests/storagevolxml2xmltest.c | 3 +-- 11 files changed, 24 insertions(+), 40 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2b1389035e..9e46cf721b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -30988,12 +30988,12 @@ int virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def) { virConnectPtr conn = NULL; - virStoragePoolDefPtr pooldef = NULL; virStoragePoolPtr pool = NULL; virStorageVolPtr vol = NULL; char *poolxml = NULL; virStorageVolInfo info; int ret = -1; + VIR_AUTOPTR(virStoragePoolDef) pooldef = NULL; if (def->src->type != VIR_STORAGE_TYPE_VOLUME) return 0; @@ -31152,7 +31152,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 4a52b1497b..177ea63076 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -737,12 +737,12 @@ virStoragePoolDefPtr virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) { virStoragePoolOptionsPtr options; - virStoragePoolDefPtr def; virStoragePoolDefPtr ret = NULL; xmlNodePtr source_node; char *type = NULL; char *uuid = NULL; char *target_path = NULL; + VIR_AUTOPTR(virStoragePoolDef) def = NULL; if (VIR_ALLOC(def) < 0) return NULL; @@ -883,7 +883,6 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) VIR_STEAL_PTR(ret, def); cleanup: - virStoragePoolDefFree(def); VIR_FREE(uuid); VIR_FREE(type); VIR_FREE(target_path); 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..2286857acf 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1579,8 +1579,8 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, const char *path, const char *autostartLink) { - virStoragePoolDefPtr def; virStoragePoolObjPtr obj; + VIR_AUTOPTR(virStoragePoolDef) def = NULL; if (!(def = virStoragePoolDefParseFile(path))) return NULL; @@ -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; virStoragePoolObjPtr obj = NULL; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; xmlNodePtr node = NULL; + VIR_AUTOPTR(virStoragePoolDef) def = 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 cac5642afd..dc082b1d08 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1952,11 +1952,11 @@ phypStorageVolCreateXML(virStoragePoolPtr pool, { virCheckFlags(0, NULL); - virStoragePoolDefPtr spdef = NULL; virStorageVolPtr vol = NULL; virStorageVolPtr dup_vol = NULL; char *key = NULL; VIR_AUTOPTR(virStorageVolDef) voldef = NULL; + VIR_AUTOPTR(virStoragePoolDef) spdef = NULL; if (VIR_ALLOC(spdef) < 0) return 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 c50a8ef5d4..c684f44475 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -690,7 +690,6 @@ storagePoolCreateXML(virConnectPtr conn, const char *xml, unsigned int flags) { - virStoragePoolDefPtr newDef; virStoragePoolObjPtr obj = NULL; virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; @@ -698,6 +697,7 @@ storagePoolCreateXML(virConnectPtr conn, virObjectEventPtr event = NULL; char *stateFile = NULL; unsigned int build_flags = 0; + VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; virCheckFlags(VIR_STORAGE_POOL_CREATE_WITH_BUILD | VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE | @@ -762,7 +762,6 @@ storagePoolCreateXML(virConnectPtr conn, cleanup: VIR_FREE(stateFile); - virStoragePoolDefFree(newDef); virObjectEventStateQueue(driver->storageEventState, event); virStoragePoolObjEndAPI(&obj); return pool; @@ -779,11 +778,11 @@ storagePoolDefineXML(virConnectPtr conn, const char *xml, unsigned int flags) { - virStoragePoolDefPtr newDef; virStoragePoolObjPtr obj = NULL; virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; virObjectEventPtr event = NULL; + VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; virCheckFlags(0, NULL); @@ -822,7 +821,6 @@ storagePoolDefineXML(virConnectPtr conn, 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 f07a075755..df10a02bbc 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -4474,11 +4474,11 @@ testStoragePoolCreateXML(virConnectPtr conn, unsigned int flags) { testDriverPtr privconn = conn->privateData; - virStoragePoolDefPtr newDef; virStoragePoolObjPtr obj = NULL; virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; virObjectEventPtr event = NULL; + VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; virCheckFlags(0, 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,11 +4540,11 @@ testStoragePoolDefineXML(virConnectPtr conn, unsigned int flags) { testDriverPtr privconn = conn->privateData; - virStoragePoolDefPtr newDef; virStoragePoolObjPtr obj = NULL; virStoragePoolDefPtr def; virStoragePoolPtr pool = NULL; virObjectEventPtr event = NULL; + VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; virCheckFlags(0, 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/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 4c98decc4d..edff8d8477 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -49,10 +49,10 @@ testCompareXMLToArgvFiles(bool shouldFail, virCommandPtr cmd = NULL; virStoragePoolDefPtr def = NULL; - virStoragePoolDefPtr inputpool = NULL; virStoragePoolObjPtr obj = NULL; VIR_AUTOPTR(virStorageVolDef) vol = NULL; VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; + VIR_AUTOPTR(virStoragePoolDef) inputpool = NULL; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; @@ -139,7 +139,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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/storage/storage_backend_iscsi_direct.c | 3 +- src/storage/storage_backend_sheepdog.c | 6 +-- src/storage/storage_backend_zfs.c | 15 ++---- src/util/virstoragefile.c | 58 ++++++++-------------- 4 files changed, 28 insertions(+), 54 deletions(-) diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index 42060dd758..cf48c29cde 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -411,7 +411,7 @@ virISCSIDirectUpdateTargets(struct iscsi_context *iscsi, struct iscsi_discovery_address *addr; struct iscsi_discovery_address *tmp_addr; size_t tmp_ntargets = 0; - char **tmp_targets = NULL; + VIR_AUTOPTR(virString) tmp_targets = NULL; if (!(addr = iscsi_discovery_sync(iscsi))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -439,7 +439,6 @@ virISCSIDirectUpdateTargets(struct iscsi_context *iscsi, ret = 0; cleanup: iscsi_free_discovery_data(iscsi, addr); - virStringListFreeCount(tmp_targets, tmp_ntargets); return ret; } diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 7cba6e02aa..9ab318bb4d 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -138,9 +138,9 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) { int ret = -1; char *output = NULL; - char **lines = NULL; - char **cells = NULL; size_t i; + VIR_AUTOPTR(virString) lines = NULL; + VIR_AUTOPTR(virString) cells = NULL; virCommandPtr cmd = virCommandNewArgList(SHEEPDOGCLI, "vdi", "list", "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); @@ -173,8 +173,6 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) cleanup: virCommandFree(cmd); - virStringListFree(lines); - virStringListFree(cells); VIR_FREE(output); return ret; } diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index ec39d9403b..4235b48c14 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -104,13 +104,13 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool, const char *volume_string) { int ret = -1; - char **tokens; size_t count; - char **name_tokens = NULL; char *vol_name; bool is_new_vol = false; virStorageVolDefPtr volume = NULL; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); + VIR_AUTOPTR(virString) tokens = NULL; + VIR_AUTOPTR(virString) name_tokens = NULL; if (!(tokens = virStringSplitCount(volume_string, "\t", 0, &count))) return -1; @@ -169,8 +169,6 @@ virStorageBackendZFSParseVol(virStoragePoolObjPtr pool, ret = 0; cleanup: - virStringListFree(tokens); - virStringListFree(name_tokens); if (is_new_vol) virStorageVolDefFree(volume); return ret; @@ -183,8 +181,8 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; char *volumes_list = NULL; - char **lines = NULL; size_t i; + VIR_AUTOPTR(virString) lines = NULL; /** * $ zfs list -Hp -t volume -o name,volsize -r test @@ -221,7 +219,6 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, cleanup: virCommandFree(cmd); - virStringListFree(lines); VIR_FREE(volumes_list); return 0; @@ -233,9 +230,9 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virCommandPtr cmd = NULL; char *zpool_props = NULL; - char **lines = NULL; - char **tokens = NULL; size_t i; + VIR_AUTOPTR(virString) lines = NULL; + VIR_AUTOPTR(virString) tokens = NULL; /** * $ zpool get -Hp health,size,free,allocated test @@ -296,8 +293,6 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) cleanup: virCommandFree(cmd); - virStringListFree(lines); - virStringListFree(tokens); VIR_FREE(zpool_props); return 0; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 1eb1ede59d..fc26c2f22e 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1594,11 +1594,10 @@ virStorageFileParseBackingStoreStr(const char *str, char **target, unsigned int *chainIndex) { - char **strings = NULL; size_t nstrings; unsigned int idx = 0; char *suffix; - int ret = -1; + VIR_AUTOPTR(virString) strings = NULL; *chainIndex = 0; @@ -1608,19 +1607,15 @@ virStorageFileParseBackingStoreStr(const char *str, if (nstrings == 2) { if (virStrToLong_uip(strings[1], &suffix, 10, &idx) < 0 || STRNEQ(suffix, "]")) - goto cleanup; + return -1; } if (target && VIR_STRDUP(*target, strings[0]) < 0) - goto cleanup; + return -1; *chainIndex = idx; - ret = 0; - - cleanup: - virStringListFreeCount(strings, nstrings); - return ret; + return 0; } @@ -2677,8 +2672,8 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src, { virURIPtr uri = NULL; const char *path = NULL; - char **scheme = NULL; int ret = -1; + VIR_AUTOPTR(virString) scheme = NULL; if (!(uri = virURIParse(uristr))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2772,7 +2767,6 @@ virStorageSourceParseBackingURI(virStorageSourcePtr src, cleanup: virURIFree(uri); - virStringListFree(scheme); return ret; } @@ -2783,7 +2777,7 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src, { char *port; size_t skip; - char **parts; + VIR_AUTOPTR(virString) parts = NULL; if (VIR_EXPAND_N(src->hosts, src->nhosts, 1) < 0) return -1; @@ -2808,7 +2802,6 @@ virStorageSourceRBDAddHost(virStorageSourcePtr src, if (!parts) goto error; src->hosts[src->nhosts-1].name = virStringListJoin((const char **)parts, ":"); - virStringListFree(parts); if (!src->hosts[src->nhosts-1].name) goto error; @@ -2940,16 +2933,15 @@ static int virStorageSourceParseNBDColonString(const char *nbdstr, virStorageSourcePtr src) { - char **backing = NULL; - int ret = -1; + VIR_AUTOPTR(virString) backing = NULL; if (!(backing = virStringSplit(nbdstr, ":", 0))) - goto cleanup; + return -1; /* we know that backing[0] now equals to "nbd" */ if (VIR_ALLOC_N(src->hosts, 1) < 0) - goto cleanup; + return -1; src->nhosts = 1; src->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP; @@ -2962,44 +2954,39 @@ virStorageSourceParseNBDColonString(const char *nbdstr, virReportError(VIR_ERR_INTERNAL_ERROR, _("missing remote information in '%s' for protocol nbd"), nbdstr); - goto cleanup; + return -1; } else if (STREQ(backing[1], "unix")) { if (!backing[2]) { virReportError(VIR_ERR_INTERNAL_ERROR, _("missing unix socket path in nbd backing string %s"), nbdstr); - goto cleanup; + return -1; } if (VIR_STRDUP(src->hosts->socket, backing[2]) < 0) - goto cleanup; + return -1; } else { if (VIR_STRDUP(src->hosts->name, backing[1]) < 0) - goto cleanup; + return -1; if (!backing[2]) { virReportError(VIR_ERR_INTERNAL_ERROR, _("missing port in nbd string '%s'"), nbdstr); - goto cleanup; + return -1; } if (virStringParsePort(backing[2], &src->hosts->port) < 0) - goto cleanup; + return -1; } if (backing[3] && STRPREFIX(backing[3], "exportname=")) { if (VIR_STRDUP(src->path, backing[3] + strlen("exportname=")) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - virStringListFree(backing); - - return ret; + return 0; } @@ -4250,9 +4237,8 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr top, int virStorageFileCheckCompat(const char *compat) { - char **version; unsigned int result; - int ret = -1; + VIR_AUTOPTR(virString) version = NULL; if (!compat) return 0; @@ -4263,13 +4249,9 @@ virStorageFileCheckCompat(const char *compat) virStrToLong_ui(version[1], NULL, 10, &result) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("forbidden characters in 'compat' attribute")); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - virStringListFree(version); - return ret; + return 0; } -- 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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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 | 59 ++++++--------- 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 | 67 +++++++--------- tests/storagepoolxml2argvtest.c | 7 +- tests/storagevolxml2argvtest.c | 6 +- tests/virstoragetest.c | 6 +- 12 files changed, 156 insertions(+), 312 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 061c494b7d..4fb38178b2 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -356,12 +356,12 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; - virCommandPtr cmd; struct virStorageBackendDiskPoolVolData cbdata = { .pool = pool, .vol = vol, }; int ret; + VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", abs_topbuilddir "/src", @@ -392,7 +392,6 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, 6, virStorageBackendDiskMakeVol, &cbdata); - virCommandFree(cmd); VIR_FREE(parthelper_path); return ret; } @@ -421,8 +420,8 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *parthelper_path; - virCommandPtr cmd; int ret; + VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", abs_topbuilddir "/src", @@ -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,9 +774,9 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src_path = def->source.devices[0].path; char *srcname = last_component(src_path); - virCommandPtr cmd = NULL; bool isDevMapperDevice; int rc = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -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 420601a303..0436c25af0 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,9 +300,9 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src = NULL; - virCommandPtr cmd = NULL; int ret = -1; int rc; + VIR_AUTOPTR(virCommand) cmd = NULL; if (virStorageBackendFileSystemIsValid(pool) < 0) return -1; @@ -334,7 +325,6 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(src); return ret; } @@ -376,9 +366,8 @@ static int virStorageBackendFileSystemStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; int rc; + VIR_AUTOPTR(virCommand) cmd = NULL; if (virStorageBackendFileSystemIsValid(pool) < 0) return -1; @@ -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..9ebc560a46 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,13 +415,12 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, int vars[] = { VIR_STORAGE_VOL_LOGICAL_REGEX_COUNT }; - int ret = -1; - virCommandPtr cmd; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); struct virStorageBackendLogicalPoolVolData cbdata = { .pool = pool, .vol = vol, }; + VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(LVS, "--separator", "#", @@ -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,9 +707,9 @@ virStorageBackendLogicalBuildPool(virStoragePoolObjPtr pool, unsigned int flags) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr vgcmd = NULL; int ret = -1; size_t i = 0; + VIR_AUTOPTR(virCommand) vgcmd = NULL; virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); @@ -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,8 +773,8 @@ virStorageBackendLogicalRefreshPool(virStoragePoolObjPtr pool) 2 }; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; 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; size_t i; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; 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 9ab318bb4d..73dcfb2f40 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -141,8 +141,9 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) size_t i; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) cells = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; - 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,13 @@ 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 +250,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); } @@ -341,33 +332,30 @@ static int virStorageBackendSheepdogRefreshVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - 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; + if (virCommandRun(cmd, NULL) < 0) + return -1; - if ((ret = virStorageBackendSheepdogParseVdiList(vol, output)) < 0) - goto cleanup; + if (virStorageBackendSheepdogParseVdiList(vol, output) < 0) + return -1; 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 +365,14 @@ 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..8c5becd4c4 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -39,10 +39,10 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; char *grp_name = NULL; char *usr_name = NULL; char *mode = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; /* Check the permissions */ if (def->target.perms.mode == (mode_t)-1) @@ -75,7 +75,6 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(mode); VIR_FREE(grp_name); VIR_FREE(usr_name); @@ -125,22 +124,15 @@ static int virStorageBackendVzPoolStop(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; - int ret = -1; int rc; + VIR_AUTOPTR(virCommand) cmd = NULL; /* Short-circuit if already unmounted */ if ((rc = virStorageBackendVzIsMounted(pool)) != 1) 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 4235b48c14..7d1a3dd2cd 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -51,9 +51,9 @@ VIR_LOG_INIT("storage.storage_backend_zfs"); static int virStorageBackendZFSVolModeNeeded(void) { - virCommandPtr cmd = NULL; int ret = -1, exit_code = -1; char *error = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; /* 'zfs get' without arguments prints out * usage information to stderr, including @@ -77,7 +77,6 @@ virStorageBackendZFSVolModeNeeded(void) ret = 0; cleanup: - virCommandFree(cmd); VIR_FREE(error); return ret; } @@ -179,10 +178,10 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; char *volumes_list = NULL; size_t i; VIR_AUTOPTR(virString) lines = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; /** * $ zfs list -Hp -t volume -o name,volsize -r test @@ -218,7 +217,6 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, } cleanup: - virCommandFree(cmd); VIR_FREE(volumes_list); return 0; @@ -228,9 +226,9 @@ static int virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; char *zpool_props = NULL; size_t i; + VIR_AUTOPTR(virCommand) cmd = NULL; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) tokens = NULL; @@ -292,7 +290,6 @@ virStorageBackendZFSRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED) goto cleanup; cleanup: - virCommandFree(cmd); VIR_FREE(zpool_props); return 0; @@ -303,9 +300,9 @@ virStorageBackendZFSCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - virCommandPtr cmd = NULL; int ret = -1; int volmode_needed = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; if (vol->target.encryption != NULL) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -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; size_t i; - int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; 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 c684f44475..a71a16add1 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2207,9 +2207,9 @@ static int virStorageBackendPloopRestoreDesc(char *path) { int ret = -1; - virCommandPtr cmd = NULL; char *refresh_tool = NULL; char *desc = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (virAsprintf(&desc, "%s/DiskDescriptor.xml", path) < 0) return ret; @@ -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 1770d21f33..fa364941c5 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -617,9 +617,9 @@ storageBackendCreatePloop(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, unsigned int flags) { int ret = -1; - virCommandPtr cmd = NULL; char *create_tool = NULL; bool created = false; + VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); @@ -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,8 +689,8 @@ storagePloopResize(virStorageVolDefPtr vol, unsigned long long capacity) { int ret = -1; - virCommandPtr cmd = NULL; char *resize_tool = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; resize_tool = virFindFileInPath("ploop"); if (!resize_tool) { @@ -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,11 +2317,11 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, { int ret = -1; char *img_tool = NULL; - virCommandPtr cmd = NULL; const char *type; char *secretPath = NULL; char *secretAlias = NULL; virStorageEncryptionPtr enc = vol->target.encryption; + VIR_AUTOPTR(virCommand) cmd = NULL; if (enc && (enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_QCOW || enc->format == VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT) && @@ -2395,7 +2388,6 @@ storageBackendResizeQemuImg(virStoragePoolObjPtr pool, VIR_FREE(secretPath); } VIR_FREE(secretAlias); - virCommandFree(cmd); return ret; } @@ -2449,10 +2441,10 @@ virStorageBackendVolResizeLocal(virStoragePoolObjPtr pool, static int storageBackendPloopHasSnapshots(char *path) { - virCommandPtr cmd = NULL; char *output = NULL; char *snap_tool = NULL; int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; snap_tool = virFindFileInPath("ploop"); if (!snap_tool) { @@ -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,10 +2763,10 @@ static int storageBackendVolWipePloop(virStorageVolDefPtr vol, unsigned int algorithm) { - virCommandPtr cmd = NULL; char *target_path = NULL; char *disk_desc = NULL; char *create_tool = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; int ret = -1; @@ -2820,7 +2810,6 @@ storageBackendVolWipePloop(virStorageVolDefPtr vol, VIR_FREE(disk_desc); VIR_FREE(target_path); VIR_FREE(create_tool); - virCommandFree(cmd); return ret; } @@ -3034,8 +3023,8 @@ virStorageBackendFindGlusterPoolSources(const char *host, { char *glusterpath = NULL; char *outbuf = NULL; - virCommandPtr cmd = NULL; int rc; + VIR_AUTOPTR(virCommand) cmd = NULL; 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); char *output = NULL; char *error = NULL; char *start, *end; int ret = VIR_STORAGE_PARTED_ERROR; + VIR_AUTOPTR(virCommand) cmd = NULL; + 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 fc26c2f22e..828e95d5d3 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 - ); int ret = -1; + VIR_AUTOPTR(virCommand) cmd = NULL; + 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, @@ -1521,24 +1515,24 @@ virStorageFileGetNPIVKey(const char *path, char **key) { int status; - VIR_AUTOFREE(char *) outbuf = NULL; const char *serial; const char *port; - virCommandPtr cmd = virCommandNewArgList("/lib/udev/scsi_id", - "--replace-whitespace", - "--whitelisted", - "--export", - "--device", path, - NULL - ); - int ret = -2; - + VIR_AUTOFREE(char *) outbuf = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; + + 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 e7f42dc0f8..c9ba9b3fde 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -22,12 +22,12 @@ testCompareXMLToArgvFiles(bool shouldFail, const char *poolxml, const char *cmdline) { - VIR_AUTOFREE(char *) actualCmdline = NULL; - VIR_AUTOFREE(char *) src = NULL; int ret = -1; - virCommandPtr cmd = NULL; virStoragePoolDefPtr def = NULL; virStoragePoolObjPtr pool = NULL; + VIR_AUTOFREE(char *) actualCmdline = NULL; + VIR_AUTOFREE(char *) src = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; @@ -83,7 +83,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 edff8d8477..d4ac02b31d 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -45,14 +45,12 @@ testCompareXMLToArgvFiles(bool shouldFail, char *actualCmdline = NULL; virStorageVolEncryptConvertStep convertStep = VIR_STORAGE_VOL_ENCRYPT_NONE; int ret = -1; - - virCommandPtr cmd = NULL; - virStoragePoolDefPtr def = NULL; virStoragePoolObjPtr obj = NULL; VIR_AUTOPTR(virStorageVolDef) vol = NULL; VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; VIR_AUTOPTR(virStoragePoolDef) inputpool = NULL; + VIR_AUTOPTR(virCommand) cmd = NULL; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; @@ -90,6 +88,7 @@ testCompareXMLToArgvFiles(bool shouldFail, convertStep = VIR_STORAGE_VOL_ENCRYPT_CREATE; do { + virCommandFree(cmd); cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol, inputvol, flags, create_tool, @@ -139,7 +138,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..7272df7e33 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -128,9 +128,9 @@ static int testPrepImages(void) { int ret = EXIT_FAILURE; - virCommandPtr cmd = NULL; char *buf = NULL; bool compat = false; + VIR_AUTOPTR(virCommand) cmd = NULL; qemuimg = virFindFileInPath("qemu-img"); if (!qemuimg) @@ -246,7 +246,6 @@ testPrepImages(void) ret = 0; cleanup: VIR_FREE(buf); - virCommandFree(cmd); if (ret) testCleanupImages(); return ret; @@ -713,7 +712,6 @@ static int mymain(void) { int ret; - virCommandPtr cmd = NULL; struct testChainData data; struct testLookupData data2; struct testPathCanonicalizeData data3; @@ -722,6 +720,7 @@ mymain(void) virStorageSourcePtr chain = NULL; virStorageSourcePtr chain2; /* short for chain->backingStore */ virStorageSourcePtr chain3; /* short for chain2->backingStore */ + VIR_AUTOPTR(virCommand) cmd = NULL; if (storageRegisterAll() < 0) return EXIT_FAILURE; @@ -1604,7 +1603,6 @@ mymain(void) /* Final cleanup */ virStorageSourceFree(chain); testCleanupImages(); - virCommandFree(cmd); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.20.1

On 2/11/19 7:33 AM, Ján Tomko wrote: Just so it's clear what's being requested, does this mean taking the current code and adding the: + if (!(flags & VIR_STORAGE_POOL_BUILD_OVERWRITE) && + !(virStorageBackendDeviceIsEmpty(def->source.devices[0].path, + fmt, true))) + goto error; and then reformatting the rest inline as is done here (more or less)? John

On 2/11/19 9:52 AM, Ján Tomko wrote: Not sure what was meant about we seem to exit without logging an error. If virStorageBackendDeviceIsEmpty an error message is generated if false is returned. Here's what I have for the difference (attached with any luck): John

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(a)redhat.com&gt; --- 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 | 36 ++++------ 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 | 79 ++++++++-------------- 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, 158 insertions(+), 319 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 4fb38178b2..4103f2d039 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -56,8 +56,9 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *tmp, *devpath, *partname; + char *tmp, *partname; bool addVol = false; + VIR_AUTOFREE(char *) devpath = NULL; /* Prepended path will be same for all partitions, so we can * strip the path to form a reasonable pool-unique name @@ -89,7 +90,6 @@ virStorageBackendDiskMakeDataVol(virStoragePoolObjPtr pool, * way of doing this... */ vol->target.path = virStorageBackendStablePath(pool, devpath, true); - VIR_FREE(devpath); if (vol->target.path == NULL) goto error; } @@ -355,12 +355,11 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, */ virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *parthelper_path; struct virStorageBackendDiskPoolVolData cbdata = { .pool = pool, .vol = vol, }; - int ret; + VIR_AUTOFREE(char *) parthelper_path = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", @@ -388,12 +387,7 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, def->allocation = 0; def->capacity = def->available = 0; - ret = virCommandRunNul(cmd, - 6, - virStorageBackendDiskMakeVol, - &cbdata); - VIR_FREE(parthelper_path); - return ret; + return virCommandRunNul(cmd, 6, virStorageBackendDiskMakeVol, &cbdata); } static int @@ -419,8 +413,7 @@ static int virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *parthelper_path; - int ret; + VIR_AUTOFREE(char *) parthelper_path = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", @@ -433,12 +426,8 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) "-g", NULL); - ret = virCommandRunNul(cmd, - 3, - virStorageBackendDiskMakePoolGeometry, - pool); - VIR_FREE(parthelper_path); - return ret; + return virCommandRunNul(cmd, 3, virStorageBackendDiskMakePoolGeometry, + pool); } static int @@ -769,13 +758,12 @@ virStorageBackendDiskDeleteVol(virStoragePoolObjPtr pool, unsigned int flags) { char *part_num = NULL; - char *devpath = NULL; char *dev_name; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); char *src_path = def->source.devices[0].path; char *srcname = last_component(src_path); bool isDevMapperDevice; - int rc = -1; + VIR_AUTOFREE(char *) devpath = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; virCheckFlags(0, -1); @@ -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 0436c25af0..97148acebe 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -246,11 +246,11 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *src = NULL; FILE *mtab; struct mntent ent; char buf[1024]; int rc1, rc2; + VIR_AUTOFREE(char *) src = NULL; if ((mtab = fopen(_PATH_MOUNTED, "r")) == NULL) { virReportSystemError(errno, @@ -282,7 +282,6 @@ virStorageBackendFileSystemIsMounted(virStoragePoolObjPtr pool) cleanup: VIR_FORCE_FCLOSE(mtab); - VIR_FREE(src); return ret; } @@ -299,9 +298,8 @@ static int virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *src = NULL; - int ret = -1; int rc; + VIR_AUTOFREE(char *) src = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; if (virStorageBackendFileSystemIsValid(pool) < 0) @@ -320,13 +318,7 @@ virStorageBackendFileSystemMount(virStoragePoolObjPtr pool) return -1; cmd = virStorageBackendFileSystemMountCmd(MOUNT, def, src); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - - ret = 0; - cleanup: - VIR_FREE(src); - return ret; + return virCommandRun(cmd, NULL); } @@ -579,10 +571,10 @@ virStoragePoolDefFSNamespaceParse(xmlXPathContextPtr ctxt, void **data) { virStoragePoolFSMountOptionsDefPtr cmdopts = NULL; - xmlNodePtr *nodes = NULL; int nnodes; size_t i; int ret = -1; + VIR_AUTOFREE(xmlNodePtr *)nodes = NULL; if (xmlXPathRegisterNs(ctxt, BAD_CAST "fs", BAD_CAST STORAGE_POOL_FS_NAMESPACE_HREF) < 0) { @@ -617,7 +609,6 @@ virStoragePoolDefFSNamespaceParse(xmlXPathContextPtr ctxt, ret = 0; cleanup: - VIR_FREE(nodes); virStoragePoolDefFSNamespaceFree(cmdopts); return ret; } diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 5428bb92ba..1888314d95 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -127,11 +127,9 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool) if (glfs_set_volfile_server(ret->vol, "tcp", ret->uri->server, ret->uri->port) < 0 || glfs_init(ret->vol) < 0) { - char *uri = virURIFormat(ret->uri); - - virReportSystemError(errno, _("failed to connect to %s"), - NULLSTR(uri)); - VIR_FREE(uri); + VIR_AUTOFREE(char *) uri = NULL; + uri = virURIFormat(ret->uri); + virReportSystemError(errno, _("failed to connect to %s"), NULLSTR(uri)); goto error; } @@ -187,9 +185,8 @@ virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state, virStorageVolDefPtr vol, const char *name) { - int ret = -1; - char *path = NULL; char *tmp; + VIR_AUTOFREE(char *) path = NULL; VIR_FREE(vol->key); VIR_FREE(vol->target.path); @@ -200,35 +197,31 @@ virStorageBackendGlusterSetMetadata(virStorageBackendGlusterStatePtr state, if (name) { VIR_FREE(vol->name); if (VIR_STRDUP(vol->name, name) < 0) - goto cleanup; + return -1; } if (virAsprintf(&path, "%s%s%s", state->volname, state->dir, vol->name) < 0) - goto cleanup; + return -1; tmp = state->uri->path; if (virAsprintf(&state->uri->path, "/%s", path) < 0) { state->uri->path = tmp; - goto cleanup; + return -1; } if (!(vol->target.path = virURIFormat(state->uri))) { VIR_FREE(state->uri->path); state->uri->path = tmp; - goto cleanup; + return -1; } VIR_FREE(state->uri->path); state->uri->path = tmp; /* the path is unique enough to serve as a volume key */ if (VIR_STRDUP(vol->key, vol->target.path) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - VIR_FREE(path); - return ret; + return 0; } @@ -244,10 +237,10 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, int ret = -1; glfs_fd_t *fd = NULL; virStorageSourcePtr meta = NULL; - char *header = NULL; ssize_t len; int backingFormat; VIR_AUTOPTR(virStorageVolDef) vol = NULL; + VIR_AUTOFREE(char *) header = NULL; *volptr = NULL; @@ -333,7 +326,6 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state, virStorageSourceFree(meta); if (fd) glfs_close(fd); - VIR_FREE(header); return ret; } diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index dc1a983b58..9314427cda 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -130,27 +130,20 @@ static int virStorageBackendISCSIFindLUs(virStoragePoolObjPtr pool, const char *session) { - char *sysfs_path; - int retval = -1; uint32_t host; + VIR_AUTOFREE(char *) sysfs_path = NULL; if (virAsprintf(&sysfs_path, "/sys/class/iscsi_session/session%s/device", session) < 0) - goto cleanup; + return -1; if (virStorageBackendISCSIGetHostNumber(sysfs_path, &host) < 0) - goto cleanup; + return -1; if (virStorageBackendSCSIFindLUs(pool, host) < 0) - goto cleanup; - - retval = 0; - - cleanup: - - VIR_FREE(sysfs_path); + return -1; - return retval; + return 0; } @@ -167,7 +160,7 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec, .nsources = 0, .sources = NULL }; - char *portal = NULL; + VIR_AUTOFREE(char *) portal = NULL; VIR_AUTOPTR(virStoragePoolSource) source = NULL; virCheckFlags(0, NULL); @@ -223,10 +216,10 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec, } VIR_FREE(list.sources); } + /* NB: Not virString -like, managed by VIR_APPEND_ELEMENT */ for (i = 0; i < ntargets; i++) VIR_FREE(targets[i]); VIR_FREE(targets); - VIR_FREE(portal); return ret; } @@ -235,8 +228,8 @@ virStorageBackendISCSICheckPool(virStoragePoolObjPtr pool, bool *isActive) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *session = NULL; int ret = -1; + VIR_AUTOFREE(char *) session = NULL; *isActive = false; @@ -259,10 +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 cf48c29cde..464732f7ab 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -421,15 +421,13 @@ virISCSIDirectUpdateTargets(struct iscsi_context *iscsi, } for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) { - char *target = NULL; + VIR_AUTOFREE(char *) target = NULL; if (VIR_STRDUP(target, tmp_addr->target_name) < 0) goto cleanup; - if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) { - VIR_FREE(target); + if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) goto cleanup; - } } VIR_STEAL_PTR(*targets, tmp_targets); @@ -489,7 +487,7 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, .nsources = 0, .sources = NULL }; - char *portal = NULL; + VIR_AUTOFREE(char *) portal = NULL; VIR_AUTOPTR(virStoragePoolSource) source = NULL; virCheckFlags(0, NULL); @@ -550,7 +548,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 +557,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 +574,6 @@ virStorageBackendISCSIDirectSetConnection(virStoragePoolObjPtr pool, VIR_STEAL_PTR(*portalRet, portal); cleanup: - VIR_FREE(portal); return iscsi; error: @@ -590,19 +586,14 @@ static int virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool) { struct iscsi_context *iscsi = NULL; - char *portal = NULL; int ret = -1; - if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal))) - goto cleanup; - if (virISCSIDirectReportLuns(pool, iscsi, portal) < 0) - goto disconect; + VIR_AUTOFREE(char *) portal = NULL; - ret = 0; - disconect: + if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal))) + return -1; + ret = virISCSIDirectReportLuns(pool, iscsi, portal); virISCSIDirectDisconnect(iscsi); iscsi_destroy_context(iscsi); - cleanup: - VIR_FREE(portal); return ret; } @@ -638,7 +629,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 +646,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 9ebc560a46..c61d03519f 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -117,14 +117,14 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, { int nextents, ret = -1; const char *regex_unit = "(\\S+)\\((\\S+)\\)"; - char *regex = NULL; - regex_t *reg = NULL; - regmatch_t *vars = NULL; char *p = NULL; size_t i; int err, nvars; unsigned long long offset, size, length; virStorageVolSourceExtent extent; + VIR_AUTOFREE(char *) regex = NULL; + VIR_AUTOFREE(regex_t *) reg = NULL; + VIR_AUTOFREE(regmatch_t *) vars = NULL; memset(&extent, 0, sizeof(extent)); @@ -202,7 +202,7 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, for (i = 0; i < nextents; i++) { size_t j; int len; - char *offset_str = NULL; + VIR_AUTOFREE(char *) offset_str = NULL; j = (i * 2) + 1; len = vars[j].rm_eo - vars[j].rm_so; @@ -219,10 +219,8 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, if (virStrToLong_ull(offset_str, NULL, 10, &offset) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed volume extent offset value")); - VIR_FREE(offset_str); goto cleanup; } - VIR_FREE(offset_str); extent.start = offset * size; extent.end = (offset * size) + length; @@ -234,9 +232,6 @@ virStorageBackendLogicalParseVolExtents(virStorageVolDefPtr vol, ret = 0; cleanup: - VIR_FREE(regex); - VIR_FREE(reg); - VIR_FREE(vars); VIR_FREE(extent.path); return ret; } @@ -459,15 +454,15 @@ virStorageBackendLogicalFindPoolSourcesFunc(char **const groups, void *data) { virStoragePoolSourceListPtr sourceList = data; - char *pvname = NULL; - char *vgname = NULL; size_t i; virStoragePoolSourceDevicePtr dev; virStoragePoolSource *thisSource; + VIR_AUTOFREE(char *) pvname = NULL; + VIR_AUTOFREE(char *) vgname = NULL; if (VIR_STRDUP(pvname, groups[0]) < 0 || VIR_STRDUP(vgname, groups[1]) < 0) - goto error; + return -1; thisSource = NULL; for (i = 0; i < sourceList->nsources; i++) { @@ -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..b78eb726b2 100644 --- a/src/storage/storage_backend_mpath.c +++ b/src/storage/storage_backend_mpath.c @@ -153,33 +153,32 @@ static int virStorageBackendCreateVols(virStoragePoolObjPtr pool, struct dm_names *names) { - int retval = -1, is_mpath = 0; - char *map_device = NULL; + int is_mpath = 0; uint32_t minor = -1; uint32_t next; + VIR_AUTOFREE(char *) map_device = NULL; do { is_mpath = virStorageBackendIsMultipath(names->name); if (is_mpath < 0) - goto out; + return -1; if (is_mpath == 1) { if (virAsprintf(&map_device, "mapper/%s", names->name) < 0) - goto out; + return -1; if (virStorageBackendGetMinorNumber(names->name, &minor) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to get %s minor number"), names->name); - goto out; + return -1; } if (virStorageBackendMpathNewVol(pool, minor, map_device) < 0) - goto out; + return -1; - VIR_FREE(map_device); } /* 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..2b7af1db23 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -86,10 +86,10 @@ virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt, void **data) { virStoragePoolRBDConfigOptionsDefPtr cmdopts = NULL; - xmlNodePtr *nodes = NULL; int nnodes; size_t i; int ret = -1; + VIR_AUTOFREE(xmlNodePtr *)nodes = NULL; if (xmlXPathRegisterNs(ctxt, BAD_CAST "rbd", BAD_CAST STORAGE_POOL_RBD_NAMESPACE_HREF) < 0) { @@ -145,7 +145,6 @@ virStoragePoolDefRBDNamespaceParse(xmlXPathContextPtr ctxt, ret = 0; cleanup: - VIR_FREE(nodes); virStoragePoolDefRBDNamespaceFree(cmdopts); return ret; } @@ -213,12 +212,12 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, char *rados_key = NULL; virBuffer mon_host = VIR_BUFFER_INITIALIZER; size_t i; - char *mon_buff = NULL; const char *client_mount_timeout = "30"; const char *mon_op_timeout = "30"; const char *osd_op_timeout = "30"; const char *rbd_default_format = "2"; virConnectPtr conn = NULL; + VIR_AUTOFREE(char *) mon_buff = NULL; if (authdef) { VIR_DEBUG("Using cephx authorization, username: %s", authdef->username); @@ -348,7 +347,6 @@ virStorageBackendRBDOpenRADOSConn(virStorageBackendRBDStatePtr ptr, virObjectUnref(conn); virBufferFreeAndReset(&mon_host); - VIR_FREE(mon_buff); return ret; } @@ -574,11 +572,12 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) int ret = -1; int len = -1; int r = 0; - char *name, *names = NULL; + char *name; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virStorageBackendRBDStatePtr ptr = NULL; struct rados_cluster_stat_t clusterstat; struct rados_pool_stat_t poolstat; + VIR_AUTOFREE(char *) names = NULL; if (!(ptr = virStorageBackendRBDNewState(pool))) goto cleanup; @@ -662,7 +661,6 @@ virStorageBackendRBDRefreshPool(virStoragePoolObjPtr pool) ret = 0; cleanup: - VIR_FREE(names); virStorageBackendRBDFreeState(&ptr); return ret; } @@ -677,8 +675,8 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, int max_snaps = 128; int snap_count, protected; size_t i; - rbd_snap_info_t *snaps = NULL; rbd_image_t image = NULL; + VIR_AUTOFREE(rbd_snap_info_t *) snaps = NULL; if ((r = rbd_open(ioctx, vol->name, &image, NULL)) < 0) { virReportSystemError(-r, _("failed to open the RBD image '%s'"), @@ -737,8 +735,6 @@ virStorageBackendRBDCleanupSnapshots(rados_ioctx_t ioctx, if (snaps) rbd_snap_list_end(snaps); - VIR_FREE(snaps); - if (image) rbd_close(image); @@ -949,8 +945,8 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, int max_snaps = 128; size_t i; int diff; - rbd_snap_info_t *snaps = NULL; rbd_image_info_t info; + VIR_AUTOFREE(rbd_snap_info_t *) snaps = NULL; if ((r = rbd_stat(image, &info, sizeof(info))) < 0) { virReportSystemError(-r, _("failed to stat the RBD image %s"), @@ -1023,8 +1019,6 @@ virStorageBackendRBDSnapshotFindNoDiff(rbd_image_t image, if (snaps) rbd_snap_list_end(snaps); - VIR_FREE(snaps); - return ret; } @@ -1098,8 +1092,8 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, uint64_t stripe_count; uint64_t stripe_unit; virBuffer snapname = VIR_BUFFER_INITIALIZER; - char *snapname_buff = NULL; rbd_image_t image = NULL; + VIR_AUTOFREE(char *) snapname_buff = NULL; if ((r = rbd_open(io, origvol, &image, NULL)) < 0) { virReportSystemError(-r, _("failed to open the RBD image %s"), @@ -1170,7 +1164,6 @@ virStorageBackendRBDCloneImage(rados_ioctx_t io, cleanup: virBufferFreeAndReset(&snapname); - VIR_FREE(snapname_buff); if (image) rbd_close(image); @@ -1271,13 +1264,12 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, uint64_t stripe_count) { int r = -1; - int ret = -1; unsigned long long offset = 0; unsigned long long length; - char *writebuf; + VIR_AUTOFREE(char *) writebuf = NULL; if (VIR_ALLOC_N(writebuf, info->obj_size * stripe_count) < 0) - goto cleanup; + return -1; while (offset < info->size) { length = MIN((info->size - offset), (info->obj_size * stripe_count)); @@ -1286,7 +1278,7 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, virReportSystemError(-r, _("writing %llu bytes failed on " "RBD image %s at offset %llu"), length, imgname, offset); - goto cleanup; + return -1; } VIR_DEBUG("Wrote %llu bytes to RBD image %s at offset %llu", @@ -1295,12 +1287,7 @@ virStorageBackendRBDVolWipeZero(rbd_image_t image, offset += length; } - ret = 0; - - cleanup: - VIR_FREE(writebuf); - - return ret; + return 0; } static int diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 14f01f9ec0..7460349c81 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,10 +241,10 @@ checkParent(const char *name, const char *parent_name) { unsigned int host_num; - char *scsi_host_name = NULL; - char *vhba_parent = NULL; bool retval = false; virConnectPtr conn = NULL; + VIR_AUTOFREE(char *) scsi_host_name = NULL; + VIR_AUTOFREE(char *) vhba_parent = NULL; VIR_DEBUG("name=%s, parent_name=%s", name, parent_name); @@ -291,8 +284,6 @@ checkParent(const char *name, cleanup: virObjectUnref(conn); - VIR_FREE(vhba_parent); - VIR_FREE(scsi_host_name); return retval; } @@ -302,10 +293,9 @@ createVport(virStoragePoolDefPtr def, const char *configFile, virStorageAdapterFCHostPtr fchost) { - char *name = NULL; virStoragePoolFCRefreshInfoPtr cbdata = NULL; virThread thread; - int ret = -1; + VIR_AUTOFREE(char *) name = NULL; VIR_DEBUG("configFile='%s' parent='%s', wwnn='%s' wwpn='%s'", NULLSTR(configFile), NULLSTR(fchost->parent), @@ -317,14 +307,14 @@ createVport(virStoragePoolDefPtr def, */ if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { if (!(checkName(name))) - goto cleanup; + return -1; /* If a parent was provided, let's make sure the 'name' we've * retrieved has the same parent. If not this will cause failure. */ if (!fchost->parent || checkParent(name, fchost->parent)) - ret = 0; + return 0; - goto cleanup; + return -1; } /* Since we're creating the vHBA, then we need to manage removing it @@ -336,12 +326,12 @@ createVport(virStoragePoolDefPtr def, fchost->managed = VIR_TRISTATE_BOOL_YES; if (configFile) { if (virStoragePoolSaveConfig(configFile, def) < 0) - goto cleanup; + return -1; } } if (!(name = virNodeDeviceCreateVport(fchost))) - goto cleanup; + return -1; /* Creating our own VPORT didn't leave enough time to find any LUN's, * so, let's create a thread whose job it is to call the FindLU's with @@ -360,11 +350,7 @@ createVport(virStoragePoolDefPtr def, } } - ret = 0; - - cleanup: - VIR_FREE(name); - return ret; + return 0; } @@ -373,10 +359,9 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, bool *isActive) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *path = NULL; - char *name = NULL; unsigned int host; - int ret = -1; + VIR_AUTOFREE(char *) path = NULL; + VIR_AUTOFREE(char *) name = NULL; *isActive = false; @@ -394,28 +379,23 @@ virStorageBackendSCSICheckPool(virStoragePoolObjPtr pool, } if (virSCSIHostGetNumber(name, &host) < 0) - goto cleanup; + return -1; if (virAsprintf(&path, "%s/host%d", LINUX_SYSFS_SCSI_HOST_PREFIX, host) < 0) - goto cleanup; + return -1; *isActive = virFileExists(path); - ret = 0; - cleanup: - VIR_FREE(path); - VIR_FREE(name); - return ret; + return 0; } static int virStorageBackendSCSIRefreshPool(virStoragePoolObjPtr pool) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *name = NULL; unsigned int host; - int ret = -1; + VIR_AUTOFREE(char *) name = NULL; def->allocation = def->capacity = def->available = 0; @@ -423,20 +403,17 @@ virStorageBackendSCSIRefreshPool(virStoragePoolObjPtr pool) return -1; if (virSCSIHostGetNumber(name, &host) < 0) - goto out; + return -1; VIR_DEBUG("Scanning host%u", host); if (virStorageBackendSCSITriggerRescan(host) < 0) - goto out; + return -1; if (virStorageBackendSCSIFindLUs(pool, host) < 0) - goto out; + return -1; - ret = 0; - out: - VIR_FREE(name); - return ret; + return 0; } diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 73dcfb2f40..d3f126da8d 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -136,9 +136,8 @@ virStorageBackendSheepdogAddVolume(virStoragePoolObjPtr pool, const char *diskIn static int virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) { - int ret = -1; - char *output = NULL; size_t i; + VIR_AUTOFREE(char *) output = NULL; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virString) cells = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; @@ -147,11 +146,11 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; lines = virStringSplit(output, "\n", 0); if (lines == NULL) - goto cleanup; + return -1; for (i = 0; lines[i]; i++) { const char *line = lines[i]; @@ -163,42 +162,34 @@ virStorageBackendSheepdogRefreshAllVol(virStoragePoolObjPtr pool) if (cells != NULL && virStringListLength((const char * const *)cells) > 2) { if (virStorageBackendSheepdogAddVolume(pool, cells[1]) < 0) - goto cleanup; + return -1; } virStringListFree(cells); cells = NULL; } - ret = 0; - - cleanup: - VIR_FREE(output); - return ret; + return 0; } static int virStorageBackendSheepdogRefreshPool(virStoragePoolObjPtr pool) { - int ret = -1; - char *output = NULL; + VIR_AUTOFREE(char *) output = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; cmd = virCommandNewArgList(SHEEPDOGCLI, "node", "info", "-r", NULL); virStorageBackendSheepdogAddHostArg(cmd, pool); virCommandSetOutputBuffer(cmd, &output); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; if (virStorageBackendSheepdogParseNodeInfo(virStoragePoolObjGetDef(pool), output) < 0) - goto cleanup; + return -1; - ret = virStorageBackendSheepdogRefreshAllVol(pool); - cleanup: - VIR_FREE(output); - return ret; + return virStorageBackendSheepdogRefreshAllVol(pool); } diff --git a/src/storage/storage_backend_vstorage.c b/src/storage/storage_backend_vstorage.c index 8c5becd4c4..df157a48b1 100644 --- a/src/storage/storage_backend_vstorage.c +++ b/src/storage/storage_backend_vstorage.c @@ -39,9 +39,9 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) { int ret = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *grp_name = NULL; - char *usr_name = NULL; - char *mode = NULL; + VIR_AUTOFREE(char *) grp_name = NULL; + VIR_AUTOFREE(char *) usr_name = NULL; + VIR_AUTOFREE(char *) mode = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; /* Check the permissions */ @@ -55,13 +55,13 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) /* Convert ids to names because vstorage uses names */ if (!(grp_name = virGetGroupName(def->target.perms.gid))) - goto cleanup; + return -1; if (!(usr_name = virGetUserName(def->target.perms.uid))) - goto cleanup; + return -1; if (virAsprintf(&mode, "%o", def->target.perms.mode) < 0) - goto cleanup; + return -1; cmd = virCommandNewArgList(VSTORAGE_MOUNT, "-c", def->source.name, @@ -70,15 +70,7 @@ virStorageBackendVzPoolStart(virStoragePoolObjPtr pool) "-g", grp_name, "-u", usr_name, NULL); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - ret = 0; - - cleanup: - VIR_FREE(mode); - VIR_FREE(grp_name); - VIR_FREE(usr_name); - return ret; + return virCommandRun(cmd, NULL); } @@ -90,7 +82,7 @@ virStorageBackendVzIsMounted(virStoragePoolObjPtr pool) FILE *mtab; struct mntent ent; char buf[1024]; - char *cluster = NULL; + VIR_AUTOFREE(char *) cluster = NULL; if (virAsprintf(&cluster, "vstorage://%s", def->source.name) < 0) return -1; @@ -115,7 +107,6 @@ virStorageBackendVzIsMounted(virStoragePoolObjPtr pool) cleanup: VIR_FORCE_FCLOSE(mtab); - VIR_FREE(cluster); return ret; } diff --git a/src/storage/storage_backend_zfs.c b/src/storage/storage_backend_zfs.c index 7d1a3dd2cd..7ffdff638e 100644 --- a/src/storage/storage_backend_zfs.c +++ b/src/storage/storage_backend_zfs.c @@ -52,7 +52,7 @@ static int virStorageBackendZFSVolModeNeeded(void) { int ret = -1, exit_code = -1; - char *error = NULL; + VIR_AUTOFREE(char *) error = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; /* 'zfs get' without arguments prints out @@ -77,7 +77,6 @@ virStorageBackendZFSVolModeNeeded(void) ret = 0; cleanup: - VIR_FREE(error); return ret; } @@ -86,13 +85,12 @@ virStorageBackendZFSCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, bool *isActive) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *devpath; + VIR_AUTOFREE(char *) devpath = NULL; if (virAsprintf(&devpath, "/dev/zvol/%s", def->source.name) < 0) return -1; *isActive = virFileIsDir(devpath); - VIR_FREE(devpath); return 0; } @@ -178,10 +176,10 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); - char *volumes_list = NULL; size_t i; VIR_AUTOPTR(virString) lines = NULL; VIR_AUTOPTR(virCommand) cmd = NULL; + VIR_AUTOFREE(char *) volumes_list = NULL; /** * $ zfs list -Hp -t volume -o name,volsize -r test @@ -203,10 +201,10 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, NULL); virCommandSetOutputBuffer(cmd, &volumes_list); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; if (!(lines = virStringSplit(volumes_list, "\n", 0))) - goto cleanup; + return -1; for (i = 0; lines[i]; i++) { if (STREQ(lines[i], "")) @@ -216,9 +214,6 @@ virStorageBackendZFSFindVols(virStoragePoolObjPtr pool, continue; } - cleanup: - VIR_FREE(volumes_list); - return 0; } diff --git a/src/storage/storage_file_gluster.c b/src/storage/storage_file_gluster.c index f8bbde8cfe..7c2189d297 100644 --- a/src/storage/storage_file_gluster.c +++ b/src/storage/storage_file_gluster.c @@ -258,10 +258,10 @@ virStorageFileBackendGlusterReadlinkCallback(const char *path, void *data) { virStorageFileBackendGlusterPrivPtr priv = data; - char *buf = NULL; size_t bufsiz = 0; ssize_t ret; struct stat st; + VIR_AUTOFREE(char *) buf = NULL; *linkpath = NULL; @@ -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

On 2/11/19 5:39 AM, Erik Skultety wrote: Because in my view/mind - previously there was no problem for pvname in the success path; however, with this change and without a VIR_STEAL_PTR there would be a problem. Moving the VIR_STEAL_PTR to a/the previous patch for this line is a noop since we never fall through to the err: label. I suppose if you insist I can move it as it really doesn't matter that much to me. Same reasoning here. John

On 2/11/19 7:52 AM, Ján Tomko wrote: [...] So two pre-patches are attached with any luck... John

Let's make use of the auto __cleanup capabilities cleaning up any now unnecessary goto paths. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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 a71a16add1..34634e97d9 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; } /** @@ -695,9 +687,9 @@ storagePoolCreateXML(virConnectPtr conn, virStoragePoolPtr pool = NULL; virStorageBackendPtr backend; virObjectEventPtr event = NULL; - char *stateFile = NULL; unsigned int build_flags = 0; VIR_AUTOPTR(virStoragePoolDef) newDef = NULL; + VIR_AUTOFREE(char *) stateFile = NULL; virCheckFlags(VIR_STORAGE_POOL_CREATE_WITH_BUILD | VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE | @@ -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,8 +883,8 @@ storagePoolCreate(virStoragePoolPtr pool, virStorageBackendPtr backend; virObjectEventPtr event = NULL; int ret = -1; - char *stateFile = NULL; unsigned int build_flags = 0; + VIR_AUTOFREE(char *) stateFile = NULL; virCheckFlags(VIR_STORAGE_POOL_CREATE_WITH_BUILD | VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE | @@ -955,7 +946,6 @@ storagePoolCreate(virStoragePoolPtr pool, ret = 0; cleanup: - VIR_FREE(stateFile); virObjectEventStateQueue(driver->storageEventState, event); virStoragePoolObjEndAPI(&obj); return ret; @@ -1013,8 +1003,8 @@ storagePoolDestroy(virStoragePoolPtr pool) virStoragePoolDefPtr def; virStorageBackendPtr backend; virObjectEventPtr event = NULL; - char *stateFile = NULL; int ret = -1; + VIR_AUTOFREE(char *) stateFile = NULL; if (!(obj = storagePoolObjFindByUUID(pool->uuid, pool->name))) goto cleanup; @@ -1045,7 +1035,6 @@ storagePoolDestroy(virStoragePoolPtr pool) goto cleanup; unlink(stateFile); - VIR_FREE(stateFile); if (backend->stopPool && backend->stopPool(obj) < 0) @@ -1078,8 +1067,8 @@ storagePoolDelete(virStoragePoolPtr pool, virStoragePoolDefPtr def; virStorageBackendPtr backend; virObjectEventPtr event = NULL; - char *stateFile = NULL; int ret = -1; + VIR_AUTOFREE(char *) stateFile = NULL; if (!(obj = virStoragePoolObjFromStoragePool(pool))) return -1; @@ -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; - char *refresh_tool = NULL; - char *desc = NULL; VIR_AUTOPTR(virCommand) cmd = 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

Commit a523770c3 added @retval return processing for virStorageBackendUpdateVolInfo in order to allow a -2 to be return; however, upon successful completion @retval = 0 and if either the virStorageBackendSCSISerial or the virStoragePoolObjAddVol failed, the method would return 0, but not add the @vol to the pool. So let's just reset retval = -1 and continue processing. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/storage/storage_util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index fa364941c5..e4913b3405 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -3869,6 +3869,9 @@ virStorageBackendSCSINewLun(virStoragePoolObjPtr pool, VIR_STORAGE_VOL_READ_NOERROR)) < 0) goto cleanup; + /* Reset retval for proper error handling */ + retval = -1; + vol->key = virStorageBackendSCSISerial(vol->target.path, (def->source.adapter.type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST)); -- 2.20.1

On Fri, Feb 08, 2019 at 01:37:08PM -0500, John Ferlan wrote: The neater fix is not to mess with the implied return value in the first place: if ((rc = UpdateVolInfo()) < 0) { retval = rc; goto cleanup; } But this goes away in the next patch, so: Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; Jano

Let's make use of the auto __cleanup capabilities cleaning up any now unnecessary goto paths. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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 177ea63076..a2ddecf0f2 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -453,16 +453,16 @@ 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; - char *name = NULL; - char *port = NULL; - char *ver = NULL; int n; VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; + VIR_AUTOFREE(char *) port = NULL; + VIR_AUTOFREE(char *) ver = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodeset = NULL; relnode = ctxt->node; ctxt->node = 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,11 +654,11 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, virStoragePermsPtr perms, const char *permxpath) { - char *mode; long long val; int ret = -1; xmlNodePtr relnode; xmlNodePtr node; + VIR_AUTOFREE(char *) mode = NULL; node = virXPathNode(permxpath, ctxt); if (node == NULL) { @@ -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; } @@ -739,10 +731,10 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) virStoragePoolOptionsPtr options; virStoragePoolDefPtr ret = NULL; xmlNodePtr source_node; - char *type = NULL; - char *uuid = NULL; - char *target_path = NULL; VIR_AUTOPTR(virStoragePoolDef) def = 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,13 +872,9 @@ 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; } @@ -1167,16 +1156,16 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, { virStorageVolDefPtr ret = NULL; virStorageVolOptionsPtr options; - char *type = NULL; - char *allocation = NULL; - char *capacity = NULL; - char *unit = NULL; - char *backingStore = NULL; xmlNodePtr node; - xmlNodePtr *nodes = NULL; size_t i; int n; VIR_AUTOPTR(virStorageVolDef) def = 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; virCheckFlags(VIR_VOL_XML_PARSE_NO_CAPACITY | VIR_VOL_XML_PARSE_OPT_CAPACITY, NULL); @@ -1194,7 +1183,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 */ @@ -1206,13 +1195,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; @@ -1220,7 +1209,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 @@ -1229,29 +1220,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); @@ -1259,7 +1248,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; @@ -1267,7 +1256,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 @@ -1276,41 +1267,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); @@ -1318,7 +1307,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)); } @@ -1326,14 +1315,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; } @@ -1615,32 +1596,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; } @@ -1648,8 +1624,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", @@ -1657,13 +1632,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

virStorageFileGetRelativeBackingPath and virStorageFileGetMetadataRecurse open coded. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/util/virstoragefile.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 828e95d5d3..c6425308fb 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -4208,8 +4208,7 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr top, goto cleanup; } - *relpath = path; - path = NULL; + VIR_STEAL_PTR(*relpath, path); ret = 0; @@ -4947,8 +4946,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, goto cleanup; } - src->backingStore = backingStore; - backingStore = NULL; + VIR_STEAL_PTR(src->backingStore, backingStore); ret = 0; cleanup: -- 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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/util/virstoragefile.c | 128 ++++++++++++++------------------------ 1 file changed, 46 insertions(+), 82 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c6425308fb..fddfea65db 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -595,9 +595,10 @@ vmdk4GetBackingStore(char **res, size_t buf_size) { static const char prefix[] = "parentFileNameHint=\""; - char *desc, *start, *end; + char *start, *end; size_t len; int ret = BACKING_STORE_ERROR; + VIR_AUTOFREE(char *) desc = NULL; if (VIR_ALLOC_N(desc, VIR_STORAGE_MAX_HEADER) < 0) goto cleanup; @@ -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,10 +1214,10 @@ virStorageFileGetMetadataFromFD(const char *path, { virStorageSourcePtr ret = NULL; virStorageSourcePtr meta = NULL; - char *buf = NULL; ssize_t len = VIR_STORAGE_MAX_HEADER; struct stat sb; int dummy; + VIR_AUTOFREE(char *) buf = NULL; if (!backingFormat) backingFormat = &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; } @@ -1891,8 +1885,8 @@ virStorageAuthDefParse(xmlNodePtr node, xmlNodePtr saveNode = ctxt->node; virStorageAuthDefPtr ret = NULL; xmlNodePtr secretnode = NULL; - char *authtype = NULL; VIR_AUTOPTR(virStorageAuthDef) authdef = NULL; + VIR_AUTOFREE(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,8 +2590,8 @@ static virStorageSourcePtr virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent, const char *rel) { - char *dirname = NULL; virStorageSourcePtr ret; + VIR_AUTOFREE(char *) dirname = NULL; if (VIR_ALLOC(ret) < 0) return NULL; @@ -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; const char *p; - int ret = -1; + VIR_AUTOFREE(char *) protocol = NULL; 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,9 +3571,9 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, virJSONValuePtr deflattened = NULL; virJSONValuePtr file; const char *drvname; - char *str = NULL; size_t i; int ret = -1; + VIR_AUTOFREE(char *) str = NULL; if (!(deflattened = virJSONValueObjectDeflatten(json))) goto cleanup; @@ -3628,7 +3606,6 @@ virStorageSourceParseBackingJSONInternal(virStorageSourcePtr src, "driver '%s'"), drvname); cleanup: - VIR_FREE(str); virJSONValueFree(deflattened); return ret; } @@ -3997,12 +3974,12 @@ virStorageFileCanonicalizePath(const char *path, bool beginDoubleSlash = false; char **components = NULL; size_t ncomponents = 0; - char *linkpath = NULL; - char *currentpath = NULL; size_t i = 0; size_t j = 0; int rc; char *ret = NULL; + VIR_AUTOFREE(char *) linkpath = NULL; + VIR_AUTOFREE(char *) currentpath = NULL; if (path[0] == '/') { beginSlash = true; @@ -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,17 +4177,11 @@ 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; } VIR_STEAL_PTR(*relpath, path); - - ret = 0; - - cleanup: - VIR_FREE(path); - VIR_FREE(tmp); - return ret; + return 0; } @@ -4866,11 +4832,11 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, { int ret = -1; const char *uniqueName; - char *buf = NULL; ssize_t headerLen; virStorageSourcePtr backingStore = NULL; int backingFormat; int rv; + VIR_AUTOFREE(char *) buf = NULL; VIR_DEBUG("path=%s format=%d uid=%u gid=%u", src->path, src->format, @@ -4952,7 +4918,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, cleanup: if (virStorageSourceHasBacking(src)) src->backingStore->id = depth; - VIR_FREE(buf); virStorageFileDeinit(src); virStorageSourceFree(backingStore); return ret; @@ -5024,10 +4989,10 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src, char **backing) { virStorageSourcePtr tmp = NULL; - char *buf = NULL; ssize_t headerLen; int ret = -1; int rv; + VIR_AUTOFREE(char *) buf = NULL; *backing = NULL; @@ -5061,7 +5026,6 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src, ret = 0; cleanup: - VIR_FREE(buf); virStorageSourceFree(tmp); return ret; -- 2.20.1

Rather than have a need for old_dom_name, let's just VIR_FREE the old name first, then use VIR_STEAL_PTR to handle the swap from the old name to the new name. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/test/test_driver.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index df10a02bbc..f51efa51b7 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2579,7 +2579,6 @@ testDomainRenameCallback(virDomainObjPtr privdom, virObjectEventPtr event_old = NULL; int ret = -1; char *new_dom_name = NULL; - char *old_dom_name = NULL; virCheckFlags(0, -1); @@ -2597,9 +2596,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,7 +2605,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); -- 2.20.1

Commit 390c06b67 added @xml, but it was never used. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/test/test_driver.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index f51efa51b7..1dde92e961 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6339,7 +6339,6 @@ testDomainSnapshotCreateXML(virDomainPtr domain, virDomainSnapshotObjPtr snap = NULL; virDomainSnapshotPtr snapshot = NULL; virObjectEventPtr event = NULL; - char *xml = NULL; bool update_current = true; bool redefine = flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; unsigned int parse_flags = VIR_DOMAIN_SNAPSHOT_PARSE_DISKS; @@ -6424,7 +6423,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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/test/test_driver.c | 169 ++++++++++++++++------------------------- 1 file changed, 66 insertions(+), 103 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1dde92e961..f748cc32f0 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -200,10 +200,10 @@ testDomainDefNamespaceParse(xmlDocPtr xml ATTRIBUTE_UNUSED, void **data) { testDomainNamespaceDefPtr nsdata = NULL; - xmlNodePtr *nodes = NULL; int tmp, n; size_t i; unsigned int tmpuint; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; if (xmlXPathRegisterNs(ctxt, BAD_CAST "test", BAD_CAST TEST_NAMESPACE_HREF) < 0) { @@ -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,16 +719,15 @@ 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; long l; int ret; + VIR_AUTOFREE(char *) str = NULL; ret = virXPathLong("string(/node/cpu/nodes[1])", ctxt, &l); if (ret == 0) { @@ -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; virDomainObjPtr obj = NULL; + VIR_AUTOFREE(xmlNodePtr *) nodes = 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; virNetworkObjPtr obj; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; 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; virInterfaceObjPtr obj; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; 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; virStoragePoolObjPtr obj; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; 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; virNodeDeviceObjPtr obj; + VIR_AUTOFREE(xmlNodePtr *) nodes = NULL; 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,12 +1947,12 @@ testDomainSaveFlags(virDomainPtr domain, const char *path, const char *dxml, unsigned int flags) { testDriverPtr privconn = domain->conn->privateData; - char *xml = NULL; int fd = -1; int len; virDomainObjPtr privdom; virObjectEventPtr event = NULL; int ret = -1; + VIR_AUTOFREE(char *) xml = NULL; virCheckFlags(0, -1); if (dxml) { @@ -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,6 @@ testDomainRestoreFlags(virConnectPtr conn, unsigned int flags) { testDriverPtr privconn = conn->privateData; - char *xml = NULL; char magic[15]; int fd = -1; int len; @@ -2088,6 +2055,7 @@ testDomainRestoreFlags(virConnectPtr conn, virDomainObjPtr dom = NULL; virObjectEventPtr event = NULL; int ret = -1; + VIR_AUTOFREE(char *) xml = NULL; virCheckFlags(0, -1); if (dxml) { @@ -2162,7 +2130,6 @@ testDomainRestoreFlags(virConnectPtr conn, cleanup: virDomainDefFree(def); - VIR_FREE(xml); VIR_FORCE_CLOSE(fd); virDomainObjEndAPI(&dom); virObjectEventStateQueue(privconn->eventState, event); @@ -2578,7 +2545,7 @@ testDomainRenameCallback(virDomainObjPtr privdom, virObjectEventPtr event_new = NULL; virObjectEventPtr event_old = NULL; int ret = -1; - char *new_dom_name = NULL; + VIR_AUTOFREE(char *) new_dom_name = NULL; virCheckFlags(0, -1); @@ -2605,7 +2572,6 @@ testDomainRenameCallback(virDomainObjPtr privdom, ret = 0; cleanup: - VIR_FREE(new_dom_name); virObjectEventStateQueue(driver->eventState, event_old); virObjectEventStateQueue(driver->eventState, event_new); return ret; @@ -5468,12 +5434,12 @@ testNodeDeviceMockCreateVport(testDriverPtr driver, const char *wwnn, const char *wwpn) { - char *xml = NULL; virNodeDeviceDefPtr def = NULL; virNodeDevCapsDefPtr caps; virNodeDeviceObjPtr obj = NULL, objcopy = NULL; virNodeDeviceDefPtr objdef; virObjectEventPtr event = NULL; + VIR_AUTOFREE(char *) xml = NULL; /* In the real code, we'd call virVHBAManageVport which would take the * wwnn/wwpn from the input XML in order to call the "vport_create" @@ -5537,7 +5503,6 @@ testNodeDeviceMockCreateVport(testDriverPtr driver, virObjectEventStateQueue(driver->eventState, event); cleanup: - VIR_FREE(xml); virNodeDeviceDefFree(def); return obj; } @@ -5550,10 +5515,11 @@ testNodeDeviceCreateXML(virConnectPtr conn, { testDriverPtr driver = conn->privateData; virNodeDeviceDefPtr def = NULL; - char *wwnn = NULL, *wwpn = NULL; virNodeDevicePtr dev = NULL, ret = NULL; virNodeDeviceObjPtr obj = NULL; virNodeDeviceDefPtr objdef; + VIR_AUTOFREE(char *) wwnn = NULL; + VIR_AUTOFREE(char *) wwpn = NULL; virCheckFlags(0, NULL); @@ -5594,8 +5560,6 @@ testNodeDeviceCreateXML(virConnectPtr conn, virNodeDeviceObjEndAPI(&obj); virNodeDeviceDefFree(def); virObjectUnref(dev); - VIR_FREE(wwnn); - VIR_FREE(wwpn); return ret; } @@ -5607,8 +5571,9 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) virNodeDeviceObjPtr obj = NULL; virNodeDeviceObjPtr parentobj = NULL; virNodeDeviceDefPtr def; - char *wwnn = NULL, *wwpn = NULL; virObjectEventPtr event = NULL; + VIR_AUTOFREE(char *) wwnn = NULL; + VIR_AUTOFREE(char *) wwpn = NULL; if (!(obj = testNodeDeviceObjFindByName(driver, dev->name))) return -1; @@ -5646,8 +5611,6 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) cleanup: virNodeDeviceObjEndAPI(&obj); virObjectEventStateQueue(driver->eventState, event); - VIR_FREE(wwnn); - VIR_FREE(wwpn); 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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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 c9ba9b3fde..288b81af1d 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -83,7 +83,6 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: - VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&pool); if (shouldFail) { virResetLastError(); @@ -101,27 +100,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 d4ac02b31d..20d986b524 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -42,11 +42,11 @@ testCompareXMLToArgvFiles(bool shouldFail, unsigned int flags, unsigned long parse_flags) { - char *actualCmdline = NULL; virStorageVolEncryptConvertStep convertStep = VIR_STORAGE_VOL_ENCRYPT_NONE; int ret = -1; virStoragePoolDefPtr def = NULL; virStoragePoolObjPtr obj = NULL; + VIR_AUTOFREE(char *) actualCmdline = NULL; VIR_AUTOPTR(virStorageVolDef) vol = NULL; VIR_AUTOPTR(virStorageVolDef) inputvol = NULL; VIR_AUTOPTR(virStoragePoolDef) inputpool = NULL; @@ -108,7 +108,7 @@ testCompareXMLToArgvFiles(bool shouldFail, goto cleanup; } else { char *createCmdline = actualCmdline; - char *cvtCmdline; + VIR_AUTOFREE(char *) cvtCmdline = NULL; int rc; if (!(cvtCmdline = virCommandToString(cmd, false))) @@ -118,7 +118,6 @@ testCompareXMLToArgvFiles(bool shouldFail, createCmdline, cvtCmdline); VIR_FREE(createCmdline); - VIR_FREE(cvtCmdline); if (rc < 0) goto cleanup; } @@ -138,7 +137,6 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: - VIR_FREE(actualCmdline); virStoragePoolObjEndAPI(&obj); return ret; } @@ -157,45 +155,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 7272df7e33..49b8f42e6d 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -128,9 +128,9 @@ static int testPrepImages(void) { int ret = EXIT_FAILURE; - char *buf = NULL; bool compat = false; VIR_AUTOPTR(virCommand) cmd = NULL; + VIR_AUTOFREE(char *) buf = NULL; qemuimg = virFindFileInPath("qemu-img"); if (!qemuimg) @@ -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,8 +647,8 @@ testBackingParse(const void *args) const struct testBackingParseData *data = args; virBuffer buf = VIR_BUFFER_INITIALIZER; virStorageSourcePtr src = NULL; - char *xml = NULL; int ret = -1; + VIR_AUTOFREE(char *) xml = NULL; if (!(src = virStorageSourceNewFromBackingAbsolute(data->backing))) { if (!data->expect) @@ -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..94d8b9b091 100644 --- a/tests/virstorageutiltest.c +++ b/tests/virstorageutiltest.c @@ -45,9 +45,9 @@ testGlusterExtractPoolSources(const void *opaque) .sources = NULL }; size_t i; - char *srcxmldata = NULL; - char *actual = NULL; int ret = -1; + VIR_AUTOFREE(char *) srcxmldata = NULL; + VIR_AUTOFREE(char *) actual = NULL; if (virTestLoadFile(data->srcxml, &srcxmldata) < 0) goto cleanup; @@ -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(a)redhat.com&gt; Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt; --- 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 c61d03519f..f153d23aec 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -911,10 +911,10 @@ static int virStorageBackendLogicalCreateVol(virStoragePoolObjPtr pool, virStorageVolDefPtr vol) { - int fd = -1; virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); virErrorPtr err; struct stat sb; + VIR_AUTOCLOSE fd = -1; vol->type = VIR_STORAGE_VOL_BLOCK; @@ -971,7 +971,6 @@ virStorageBackendLogicalCreateVol(virStoragePoolObjPtr pool, error: err = virSaveLastError(); - VIR_FORCE_CLOSE(fd); virStorageBackendLogicalDeleteVol(pool, vol, 0); virSetError(err); virFreeError(err); diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c index 7460349c81..1e643f466e 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..8817970f44 100644 --- a/src/storage/storage_file_fs.c +++ b/src/storage/storage_file_fs.c @@ -83,8 +83,8 @@ virStorageFileBackendFileInit(virStorageSourcePtr src) static int virStorageFileBackendFileCreate(virStorageSourcePtr src) { - int fd = -1; mode_t mode = S_IRUSR; + VIR_AUTOCLOSE fd = -1; if (!src->readonly) mode |= S_IWUSR; @@ -95,7 +95,6 @@ virStorageFileBackendFileCreate(virStorageSourcePtr src) return -1; } - VIR_FORCE_CLOSE(fd); return 0; } @@ -121,8 +120,8 @@ virStorageFileBackendFileRead(virStorageSourcePtr src, size_t len, char **buf) { - int fd = -1; ssize_t ret = -1; + VIR_AUTOCLOSE fd = -1; if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, src->drv->uid, src->drv->gid, 0)) < 0) { @@ -134,19 +133,15 @@ virStorageFileBackendFileRead(virStorageSourcePtr src, if (offset > 0) { if (lseek(fd, offset, SEEK_SET) == (off_t) -1) { virReportSystemError(errno, _("cannot seek into '%s'"), src->path); - goto cleanup; + return -1; } } if ((ret = virFileReadHeaderFD(fd, len, buf)) < 0) { - virReportSystemError(errno, - _("cannot read header '%s'"), src->path); - goto cleanup; + virReportSystemError(errno, _("cannot read header '%s'"), src->path); + return -1; } - cleanup: - VIR_FORCE_CLOSE(fd); - return ret; } diff --git a/src/storage/storage_util.c b/src/storage/storage_util.c index 1c290f1995..012c6b319b 100644 --- a/src/storage/storage_util.c +++ b/src/storage/storage_util.c @@ -130,7 +130,6 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, bool want_sparse, bool reflink_copy) { - int inputfd = -1; int amtread = -1; int ret = 0; size_t rbytes = READ_BLOCK_SIZE_DEFAULT; @@ -139,13 +138,14 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, struct stat st; VIR_AUTOFREE(char *) zerobuf = NULL; VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOCLOSE inputfd = -1; if ((inputfd = open(inputvol->target.path, O_RDONLY)) < 0) { ret = -errno; virReportSystemError(errno, _("could not open input path '%s'"), inputvol->target.path); - goto cleanup; + return ret; } #ifdef __linux__ @@ -157,15 +157,11 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, if (wbytes < WRITE_BLOCK_SIZE_DEFAULT) wbytes = WRITE_BLOCK_SIZE_DEFAULT; - if (VIR_ALLOC_N(zerobuf, wbytes) < 0) { - ret = -errno; - goto cleanup; - } + if (VIR_ALLOC_N(zerobuf, wbytes) < 0) + return -errno; - if (VIR_ALLOC_N(buf, rbytes) < 0) { - ret = -errno; - goto cleanup; - } + if (VIR_ALLOC_N(buf, rbytes) < 0) + return -errno; if (reflink_copy) { if (reflinkCloneFile(fd, inputfd) < 0) { @@ -173,10 +169,10 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, virReportSystemError(errno, _("failed to clone files from '%s'"), inputvol->target.path); - goto cleanup; + return ret; } else { VIR_DEBUG("btrfs clone finished."); - goto cleanup; + return 0; } } @@ -191,7 +187,7 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, virReportSystemError(errno, _("failed reading from file '%s'"), inputvol->target.path); - goto cleanup; + return ret; } *total -= amtread; @@ -208,14 +204,14 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, virReportSystemError(errno, _("cannot extend file '%s'"), vol->target.path); - goto cleanup; + return ret; } } else if (safewrite(fd, buf+offset, interval) < 0) { ret = -errno; virReportSystemError(errno, _("failed writing to file '%s'"), vol->target.path); - goto cleanup; + return ret; } } while ((amtleft -= interval) > 0); @@ -225,23 +221,18 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol, ret = -errno; virReportSystemError(errno, _("cannot sync data to file '%s'"), vol->target.path); - goto cleanup; + return ret; } - if (VIR_CLOSE(inputfd) < 0) { ret = -errno; virReportSystemError(errno, _("cannot close file '%s'"), inputvol->target.path); - goto cleanup; + return ret; } - inputfd = -1; - - cleanup: - VIR_FORCE_CLOSE(inputfd); - return ret; + return 0; } static int @@ -250,14 +241,13 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virStorageVolDefPtr inputvol, unsigned int flags) { - int fd = -1; - int ret = -1; unsigned long long remain; struct stat st; gid_t gid; uid_t uid; mode_t mode; bool reflink_copy = false; + VIR_AUTOCLOSE fd = -1; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA | VIR_STORAGE_VOL_CREATE_REFLINK, @@ -267,7 +257,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("metadata preallocation is not supported for block " "volumes")); - goto cleanup; + return -1; } if (flags & VIR_STORAGE_VOL_CREATE_REFLINK) @@ -277,7 +267,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virReportSystemError(errno, _("cannot create path '%s'"), vol->target.path); - goto cleanup; + return -1; } remain = vol->target.capacity; @@ -285,13 +275,13 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, if (inputvol) { if (virStorageBackendCopyToFD(vol, inputvol, fd, &remain, false, reflink_copy) < 0) - goto cleanup; + return -1; } if (fstat(fd, &st) == -1) { virReportSystemError(errno, _("stat of '%s' failed"), vol->target.path); - goto cleanup; + return -1; } uid = (vol->target.perms->uid != st.st_uid) ? vol->target.perms->uid : (uid_t)-1; @@ -303,7 +293,7 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, _("cannot chown '%s' to (%u, %u)"), vol->target.path, (unsigned int)uid, (unsigned int)gid); - goto cleanup; + return -1; } mode = (vol->target.perms->mode == (mode_t)-1 ? @@ -312,21 +302,16 @@ storageBackendCreateBlockFrom(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, virReportSystemError(errno, _("cannot set mode of '%s' to %04o"), vol->target.path, mode); - goto cleanup; + return -1; } if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("cannot close file '%s'"), vol->target.path); - goto cleanup; + return -1; } - fd = -1; - ret = 0; - cleanup: - VIR_FORCE_CLOSE(fd); - - return ret; + return 0; } static int @@ -419,11 +404,11 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool, { virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool); int ret = -1; - int fd = -1; int operation_flags; bool reflink_copy = false; mode_t open_mode = VIR_STORAGE_DEFAULT_VOL_PERM_MODE; bool created = false; + VIR_AUTOCLOSE fd = -1; virCheckFlags(VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA | VIR_STORAGE_VOL_CREATE_REFLINK, @@ -501,7 +486,6 @@ storageBackendCreateRaw(virStoragePoolObjPtr pool, ignore_value(virFileRemove(vol->target.path, vol->target.perms->uid, vol->target.perms->gid)); - VIR_FORCE_CLOSE(fd); return ret; } @@ -542,7 +526,7 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, * re-open the file and attempt to force the mode change. */ if (mode != (st.st_mode & S_IRWXUGO)) { - int fd = -1; + VIR_AUTOCLOSE fd = -1; int flags = VIR_FILE_OPEN_FORK | VIR_FILE_OPEN_FORCE_MODE; if ((fd = virFileOpenAs(vol->target.path, O_RDWR, mode, @@ -550,7 +534,6 @@ virStorageBackendCreateExecCommand(virStoragePoolObjPtr pool, vol->target.perms->gid, flags)) >= 0) { /* Success - means we're good */ - VIR_FORCE_CLOSE(fd); ret = 0; goto cleanup; } @@ -1227,10 +1210,10 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, { virStorageEncryptionPtr enc = vol->target.encryption; char *secretPath = NULL; - int fd = -1; uint8_t *secret = NULL; size_t secretlen = 0; virConnectPtr conn = NULL; + VIR_AUTOCLOSE fd = -1; if (!enc) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -1268,7 +1251,6 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, _("failed to write secret file")); goto error; } - VIR_FORCE_CLOSE(fd); if ((vol->target.perms->uid != (uid_t)-1) && (vol->target.perms->gid != (gid_t)-1)) { @@ -1283,7 +1265,6 @@ storageBackendCreateQemuImgSecretPath(virStoragePoolObjPtr pool, cleanup: virObjectUnref(conn); VIR_DISPOSE_N(secret, secretlen); - VIR_FORCE_CLOSE(fd); return secretPath; @@ -1751,17 +1732,17 @@ storageBackendUpdateVolTargetInfo(virStorageVolType voltype, unsigned int openflags, unsigned int readflags) { - int ret, fd = -1; + int ret; struct stat sb; ssize_t len = VIR_STORAGE_MAX_HEADER; VIR_AUTOFREE(char *) buf = NULL; + VIR_AUTOCLOSE fd = -1; - 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,11 +3353,11 @@ storageBackendProbeTarget(virStorageSourcePtr target, virStorageEncryptionPtr *encryption) { int backingStoreFormat; - int fd = -1; int ret = -1; int rc; virStorageSourcePtr meta = NULL; struct stat sb; + VIR_AUTOCLOSE fd = -1; if (encryption) *encryption = NULL; @@ -3499,7 +3460,6 @@ storageBackendProbeTarget(virStorageSourcePtr target, } cleanup: - VIR_FORCE_CLOSE(fd); virStorageSourceFree(meta); return ret; } @@ -3571,8 +3531,9 @@ virStorageBackendRefreshLocal(virStoragePoolObjPtr pool) struct stat statbuf; virStorageSourcePtr target = NULL; int direrr; - int fd = -1, ret = -1; + int ret = -1; VIR_AUTOPTR(virStorageVolDef) vol = NULL; + VIR_AUTOCLOSE fd = -1; if (virDirOpen(&dir, def->target.path) < 0) goto cleanup; @@ -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 fddfea65db..907358a8fe 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1080,10 +1080,9 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta, int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid) { - int fd; - int ret = -1; struct stat sb; ssize_t len = VIR_STORAGE_MAX_HEADER; + VIR_AUTOCLOSE fd = -1; VIR_AUTOFREE(char *) header = NULL; if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) { @@ -1093,31 +1092,24 @@ virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid) if (fstat(fd, &sb) < 0) { virReportSystemError(errno, _("cannot stat file '%s'"), path); - goto cleanup; + return -1; } /* No header to probe for directories */ - if (S_ISDIR(sb.st_mode)) { - ret = VIR_STORAGE_FILE_DIR; - goto cleanup; - } + if (S_ISDIR(sb.st_mode)) + return VIR_STORAGE_FILE_DIR; if (lseek(fd, 0, SEEK_SET) == (off_t)-1) { virReportSystemError(errno, _("cannot set to start of '%s'"), path); - goto cleanup; + return -1; } if ((len = virFileReadHeaderFD(fd, len, &header)) < 0) { virReportSystemError(errno, _("cannot read header '%s'"), path); - goto cleanup; + return -1; } - ret = virStorageFileProbeFormatFromBuf(path, header, len); - - cleanup: - VIR_FORCE_CLOSE(fd); - - return ret; + return virStorageFileProbeFormatFromBuf(path, header, len); } @@ -1312,13 +1304,12 @@ virStorageFileResize(const char *path, unsigned long long capacity, bool pre_allocate) { - int fd = -1; - int ret = -1; int rc; + VIR_AUTOCLOSE fd = -1; if ((fd = open(path, O_RDWR)) < 0) { virReportSystemError(errno, _("Unable to open '%s'"), path); - goto cleanup; + return -1; } if (pre_allocate) { @@ -1331,26 +1322,22 @@ virStorageFileResize(const char *path, _("Failed to pre-allocate space for " "file '%s'"), path); } - goto cleanup; + return -1; } } if (ftruncate(fd, capacity) < 0) { virReportSystemError(errno, _("Failed to truncate file '%s'"), path); - goto cleanup; + return -1; } if (VIR_CLOSE(fd) < 0) { virReportSystemError(errno, _("Unable to save '%s'"), path); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - VIR_FORCE_CLOSE(fd); - return ret; + return 0; } -- 2.20.1

Only one path will consume the @def; otherwise, we need to free it. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- tests/storagepoolxml2argvtest.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c index 288b81af1d..f2a8af12b0 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -23,6 +23,7 @@ testCompareXMLToArgvFiles(bool shouldFail, const char *cmdline) { int ret = -1; + bool consumeDef = false; virStoragePoolDefPtr def = NULL; virStoragePoolObjPtr pool = NULL; VIR_AUTOFREE(char *) actualCmdline = NULL; @@ -41,6 +42,7 @@ testCompareXMLToArgvFiles(bool shouldFail, goto cleanup; } virStoragePoolObjSetDef(pool, def); + consumeDef = true; if (!(src = virStorageBackendFileSystemGetPoolSource(pool))) { VIR_TEST_DEBUG("pool type %d has no pool source\n", def->type); @@ -83,6 +85,8 @@ testCompareXMLToArgvFiles(bool shouldFail, ret = 0; cleanup: + if (!consumeDef) + virStoragePoolDefFree(def); virStoragePoolObjEndAPI(&pool); if (shouldFail) { virResetLastError(); -- 2.20.1

On 2/11/19 7:24 AM, Erik Skultety wrote: For this test, perhaps a waste to introduce a virStoragePoolDefCopy type method to do a deep copy. Ironically, from the review of v1: The "correct" solution is to not reference @def after the virStoragePoolObjSetDef call since it "consumes" it; however, the alternate solution to fetch objDef from @pool wasn't accepted so this was essentially the way around that. I can restore defType here or I could add a : const char *defTypeStr = virStoragePoolTypeToString(def->type) and use that as '%s' instead of the %d def->type in the debug message. Then remove the consumeDef and use def = NULL before getting to cleanup after the virStoragePoolObjSetDef call. John

To prepare for subsequent change to use VIR_AUTOPTR logic rename the @ret to @def. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/util/virstoragefile.c | 96 +++++++++++++++++++-------------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index adce5f0fe3..45399f98ce 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2243,98 +2243,98 @@ virStorageSourcePtr virStorageSourceCopy(const virStorageSource *src, bool backingChain) { - virStorageSourcePtr ret = NULL; + virStorageSourcePtr def = 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) + 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) goto error; if (src->nhosts) { - if (!(ret->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts))) + if (!(def->hosts = virStorageNetHostDefCopy(src->nhosts, src->hosts))) goto error; - ret->nhosts = src->nhosts; + def->nhosts = src->nhosts; } if (src->srcpool && - !(ret->srcpool = virStorageSourcePoolDefCopy(src->srcpool))) + !(def->srcpool = virStorageSourcePoolDefCopy(src->srcpool))) goto error; if (src->features && - !(ret->features = virBitmapNewCopy(src->features))) + !(def->features = virBitmapNewCopy(src->features))) goto error; if (src->encryption && - !(ret->encryption = virStorageEncryptionCopy(src->encryption))) + !(def->encryption = virStorageEncryptionCopy(src->encryption))) goto error; if (src->perms && - !(ret->perms = virStoragePermsCopy(src->perms))) + !(def->perms = virStoragePermsCopy(src->perms))) goto error; if (src->timestamps && - !(ret->timestamps = virStorageTimestampsCopy(src->timestamps))) + !(def->timestamps = virStorageTimestampsCopy(src->timestamps))) goto error; - if (virStorageSourceSeclabelsCopy(ret, src) < 0) + if (virStorageSourceSeclabelsCopy(def, src) < 0) goto error; if (src->auth && - !(ret->auth = virStorageAuthDefCopy(src->auth))) + !(def->auth = virStorageAuthDefCopy(src->auth))) goto error; if (src->pr && - !(ret->pr = virStoragePRDefCopy(src->pr))) + !(def->pr = virStoragePRDefCopy(src->pr))) goto error; - if (virStorageSourceInitiatorCopy(&ret->initiator, &src->initiator)) + if (virStorageSourceInitiatorCopy(&def->initiator, &src->initiator)) goto error; if (backingChain && src->backingStore) { - if (!(ret->backingStore = virStorageSourceCopy(src->backingStore, + if (!(def->backingStore = virStorageSourceCopy(src->backingStore, true))) goto error; } - return ret; + return def; error: - virStorageSourceFree(ret); + virStorageSourceFree(def); return NULL; } -- 2.20.1

To prepare for subsequent change to use VIR_AUTOPTR logic rename the @ret to @def. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/util/virstoragefile.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 475e797e1b..d2636eec0a 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -3619,48 +3619,48 @@ virStorageSourcePtr virStorageSourceNewFromBackingAbsolute(const char *path) { const char *json; - virStorageSourcePtr ret; + virStorageSourcePtr def; 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) + if (VIR_STRDUP(def->path, path) < 0) goto error; } 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; - 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; } } - return ret; + return def; error: - virStorageSourceFree(ret); + virStorageSourceFree(def); return NULL; } -- 2.20.1

To prepare for subsequent change to use VIR_AUTOPTR logic rename the @ret to @def. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- tests/virstoragetest.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 49b8f42e6d..8db1d294b0 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -95,32 +95,32 @@ testStorageFileGetMetadata(const char *path, uid_t uid, gid_t gid) { struct stat st; - virStorageSourcePtr ret = NULL; + virStorageSourcePtr def = 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) + if (VIR_STRDUP(def->path, path) < 0) goto error; - if (virStorageFileGetMetadata(ret, uid, gid, false) < 0) + if (virStorageFileGetMetadata(def, uid, gid, false) < 0) goto error; - return ret; + return def; error: - virStorageSourceFree(ret); + virStorageSourceFree(def); return NULL; } -- 2.20.1

On Fri, Feb 08, 2019 at 01:37:26PM -0500, John Ferlan wrote: ... Not sure how much sense the comment makes without ^this hunk, I think we can drop it too MinGW is still stuborn about inlining in certain VIR_AUTO cases, I'm wondering why GCC and Clang are okay with that and whether we should drop -Winline or go without the cases that MinGW is sad about, I tried to lookup something related, but it doesn't seem to be a recent bug in MinGW. I'm okay with the changes, but I don't want to have a failing build for the next X weeks, so I'd say drop the violators: Reviewed-by: Erik Skultety <eskultet(a)redhat.com&gt;

On Mon, Feb 11, 2019 at 08:33:32AM -0500, John Ferlan wrote: You can either set up your own environment with Andrea's lcitool or you can make use of github's travis integration: https://travis-ci.org/eskultety/libvirt/jobs/491591878 Erik