[libvirt] [PATCH 00/11] A selection of refactors and improvements from the blockdev-add saga
Few patches require https://www.redhat.com/archives/libvir-list/2019-April/msg00324.html to be applied. This is a collection of cleanups and improvements from my blockdev branch which make sense even without the rest of the blockdev patches. Peter Krempa (11): qemu: block: Introduce and use AUTOPTR func for qemuBlockStorageSourceAttachDataPtr qemu: block: Use VIR_AUTOPTR for virJSONValue util: uri: Introduce VIR_AUTOPTR freeing function qemu: block: Use VIR_AUTOPTR for virURIPtr qemu: block: Use VIR_AUTOPTR for virHashTablePtr qemu: block: Use VIR_AUTOFREE for char * qemu: block: Add and use AUTOPTR func for qemuBlockNodeNameBackingChainData qemu: block: Remove unneeded cleanup jumps internal: Introduce VIR_RETURN_PTR qemu: block: Use VIR_RETURN_PTR util: json: Use VIR_APPEND_ELEMENT in virJSONValueObjectAppend src/internal.h | 15 ++ src/qemu/qemu_block.c | 388 +++++++++++++------------------------- src/qemu/qemu_block.h | 3 + src/qemu/qemu_command.c | 14 +- src/qemu/qemu_hotplug.c | 9 +- src/qemu/qemu_migration.c | 3 +- src/util/virjson.c | 19 +- src/util/viruri.h | 2 + 8 files changed, 172 insertions(+), 281 deletions(-) -- 2.20.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 3 +-- src/qemu/qemu_block.h | 3 +++ src/qemu/qemu_command.c | 14 +++++++------- src/qemu/qemu_hotplug.c | 9 +++------ src/qemu/qemu_migration.c | 3 +-- 5 files changed, 15 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index cbf0aa4189..7961d31978 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1543,7 +1543,7 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) qemuBlockStorageSourceAttachDataPtr qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src) { - qemuBlockStorageSourceAttachDataPtr data; + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL; qemuBlockStorageSourceAttachDataPtr ret = NULL; if (VIR_ALLOC(data) < 0) @@ -1559,7 +1559,6 @@ qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src) VIR_STEAL_PTR(ret, data); cleanup: - qemuBlockStorageSourceAttachDataFree(data); return ret; } diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 9401ab4e12..eab8da8e2c 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -102,6 +102,9 @@ struct qemuBlockStorageSourceAttachData { void qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data); +VIR_DEFINE_AUTOPTR_FUNC(qemuBlockStorageSourceAttachData, + qemuBlockStorageSourceAttachDataFree); + qemuBlockStorageSourceAttachDataPtr qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 029780fe86..7d439550f3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2554,7 +2554,7 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, { qemuBlockStorageSourceAttachDataPtr *data = NULL; size_t ndata = 0; - qemuBlockStorageSourceAttachDataPtr tmp = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) tmp = NULL; virJSONValuePtr copyOnReadProps = NULL; virStorageSourcePtr n; char *str = NULL; @@ -2613,7 +2613,6 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd, for (i = 0; i < ndata; i++) qemuBlockStorageSourceAttachDataFree(data[i]); VIR_FREE(data); - qemuBlockStorageSourceAttachDataFree(tmp); virJSONValueFree(copyOnReadProps); VIR_FREE(str); return ret; @@ -11160,18 +11159,19 @@ qemuBlockStorageSourceAttachDataPtr qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps) { - qemuBlockStorageSourceAttachDataPtr data = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL; + qemuBlockStorageSourceAttachDataPtr ret = NULL; if (VIR_ALLOC(data) < 0) return NULL; if (!(data->driveCmd = qemuBuildDriveStr(disk, qemuCaps)) || - !(data->driveAlias = qemuAliasDiskDriveFromDisk(disk))) { - qemuBlockStorageSourceAttachDataFree(data); + !(data->driveAlias = qemuAliasDiskDriveFromDisk(disk))) return NULL; - } - return data; + VIR_STEAL_PTR(ret, data); + + return ret; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 34249bd030..90c6a108dc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -530,7 +530,7 @@ qemuHotplugRemoveStorageSourcePrepareData(virStorageSourcePtr src, { qemuDomainStorageSourcePrivatePtr srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); - qemuBlockStorageSourceAttachDataPtr data; + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL; qemuBlockStorageSourceAttachDataPtr ret = NULL; if (VIR_ALLOC(data) < 0) @@ -570,7 +570,6 @@ qemuHotplugRemoveStorageSourcePrepareData(virStorageSourcePtr src, cleanup: VIR_FREE(driveAlias); - qemuBlockStorageSourceAttachDataFree(data); return ret; } @@ -581,7 +580,7 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps) { qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); - qemuBlockStorageSourceAttachDataPtr backend = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; qemuHotplugDiskSourceDataPtr data = NULL; qemuHotplugDiskSourceDataPtr ret = NULL; char *drivealias = NULL; @@ -616,7 +615,6 @@ qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk, VIR_STEAL_PTR(ret, data); cleanup: - qemuBlockStorageSourceAttachDataFree(backend); qemuHotplugDiskSourceDataFree(data); return ret; } @@ -636,7 +634,7 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk, virStorageSourcePtr src, virQEMUCapsPtr qemuCaps) { - qemuBlockStorageSourceAttachDataPtr backend = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) backend = NULL; qemuHotplugDiskSourceDataPtr data; qemuHotplugDiskSourceDataPtr ret = NULL; virStorageSourcePtr savesrc = NULL; @@ -682,7 +680,6 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr disk, if (savesrc) VIR_STEAL_PTR(disk->src, savesrc); - qemuBlockStorageSourceAttachDataFree(backend); qemuHotplugDiskSourceDataFree(data); return ret; } diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5db23b492f..a8d50d310d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -787,7 +787,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, unsigned int mirror_flags, const char *tlsAlias) { - qemuBlockStorageSourceAttachDataPtr data = NULL; + VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL; qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk); int mon_ret = 0; int ret = -1; @@ -849,7 +849,6 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuBlockStorageSourceAttachDataFree(data); return ret; } -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:13PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 3 +-- src/qemu/qemu_block.h | 3 +++ src/qemu/qemu_command.c | 14 +++++++------- src/qemu/qemu_hotplug.c | 9 +++------ src/qemu/qemu_migration.c | 3 +-- 5 files changed, 15 insertions(+), 17 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 80 ++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 51 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 7961d31978..769e07d3d8 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -345,8 +345,8 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, { qemuDomainObjPrivatePtr priv = vm->privateData; virHashTablePtr disktable = NULL; - virJSONValuePtr data = NULL; - virJSONValuePtr blockstats = NULL; + VIR_AUTOPTR(virJSONValue) data = NULL; + VIR_AUTOPTR(virJSONValue) blockstats = NULL; virDomainDiskDefPtr disk; size_t i; int ret = -1; @@ -376,8 +376,6 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, ret = 0; cleanup: - virJSONValueFree(data); - virJSONValueFree(blockstats); virHashFree(disktable); return ret; @@ -504,7 +502,7 @@ static virJSONValuePtr qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, bool legacy) { - virJSONValuePtr server = NULL; + VIR_AUTOPTR(virJSONValue) server = NULL; virJSONValuePtr ret = NULL; const char *transport; const char *field; @@ -553,7 +551,6 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, cleanup: VIR_FREE(port); - virJSONValueFree(server); return ret; } @@ -571,8 +568,8 @@ static virJSONValuePtr qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src, bool legacy) { - virJSONValuePtr servers = NULL; - virJSONValuePtr server = NULL; + VIR_AUTOPTR(virJSONValue) servers = NULL; + VIR_AUTOPTR(virJSONValue) server = NULL; virJSONValuePtr ret = NULL; virStorageNetHostDefPtr host; size_t i; @@ -595,8 +592,6 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src, VIR_STEAL_PTR(ret, servers); cleanup: - virJSONValueFree(servers); - virJSONValueFree(server); return ret; } @@ -646,8 +641,8 @@ qemuBlockStorageSourceBuildJSONInetSocketAddress(virStorageNetHostDefPtr host) static virJSONValuePtr qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src) { - virJSONValuePtr servers = NULL; - virJSONValuePtr server = NULL; + VIR_AUTOPTR(virJSONValue) servers = NULL; + VIR_AUTOPTR(virJSONValue) server = NULL; virJSONValuePtr ret = NULL; virStorageNetHostDefPtr host; size_t i; @@ -670,8 +665,6 @@ qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src) VIR_STEAL_PTR(ret, servers); cleanup: - virJSONValueFree(servers); - virJSONValueFree(server); return ret; } @@ -681,8 +674,8 @@ static virJSONValuePtr qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src, bool legacy) { - virJSONValuePtr servers = NULL; - virJSONValuePtr props = NULL; + VIR_AUTOPTR(virJSONValue) servers = NULL; + VIR_AUTOPTR(virJSONValue) props = NULL; virJSONValuePtr ret = NULL; if (!(servers = qemuBlockStorageSourceBuildHostsJSONSocketAddress(src, legacy))) @@ -708,8 +701,6 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src, VIR_STEAL_PTR(ret, props); cleanup: - virJSONValueFree(servers); - virJSONValueFree(props); return ret; } @@ -719,7 +710,7 @@ static virJSONValuePtr qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src) { const char *protocol = virStorageNetProtocolTypeToString(src->protocol); - virJSONValuePtr server = NULL; + VIR_AUTOPTR(virJSONValue) server = NULL; virJSONValuePtr ret = NULL; if (src->nhosts != 1) { @@ -737,12 +728,11 @@ qemuBlockStorageSourceGetVxHSProps(virStorageSourcePtr src) * vdisk-id:"eb90327c-8302-4725-4e85ed4dc251", * server:{type:"tcp", host:"1.2.3.4", port:9999}} */ - if (virJSONValueObjectCreate(&ret, - "s:driver", protocol, - "S:tls-creds", src->tlsAlias, - "s:vdisk-id", src->path, - "a:server", &server, NULL) < 0) - virJSONValueFree(server); + ignore_value(virJSONValueObjectCreate(&ret, + "s:driver", protocol, + "S:tls-creds", src->tlsAlias, + "s:vdisk-id", src->path, + "a:server", &server, NULL)); return ret; } @@ -875,7 +865,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) static virJSONValuePtr qemuBlockStorageSourceGetNBDProps(virStorageSourcePtr src) { - virJSONValuePtr serverprops; + VIR_AUTOPTR(virJSONValue) serverprops = NULL; virJSONValuePtr ret = NULL; if (src->nhosts != 1) { @@ -898,7 +888,6 @@ qemuBlockStorageSourceGetNBDProps(virStorageSourcePtr src) goto cleanup; cleanup: - virJSONValueFree(serverprops); return ret; } @@ -907,11 +896,11 @@ static virJSONValuePtr qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src) { qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); - virJSONValuePtr servers = NULL; + VIR_AUTOPTR(virJSONValue) servers = NULL; virJSONValuePtr ret = NULL; const char *username = NULL; - virJSONValuePtr authmodes = NULL; - virJSONValuePtr mode = NULL; + VIR_AUTOPTR(virJSONValue) authmodes = NULL; + VIR_AUTOPTR(virJSONValue) mode = NULL; const char *keysecret = NULL; if (src->nhosts > 0 && @@ -952,9 +941,6 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src) goto cleanup; cleanup: - virJSONValueFree(authmodes); - virJSONValueFree(mode); - virJSONValueFree(servers); return ret; } @@ -962,7 +948,7 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src) static virJSONValuePtr qemuBlockStorageSourceGetSheepdogProps(virStorageSourcePtr src) { - virJSONValuePtr serverprops; + VIR_AUTOPTR(virJSONValue) serverprops = NULL; virJSONValuePtr ret = NULL; if (src->nhosts != 1) { @@ -985,7 +971,6 @@ qemuBlockStorageSourceGetSheepdogProps(virStorageSourcePtr src) goto cleanup; cleanup: - virJSONValueFree(serverprops); return ret; } @@ -993,7 +978,7 @@ qemuBlockStorageSourceGetSheepdogProps(virStorageSourcePtr src) static virJSONValuePtr qemuBlockStorageSourceGetSshProps(virStorageSourcePtr src) { - virJSONValuePtr serverprops; + VIR_AUTOPTR(virJSONValue) serverprops = NULL; virJSONValuePtr ret = NULL; const char *username = NULL; @@ -1019,7 +1004,6 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr src) goto cleanup; cleanup: - virJSONValueFree(serverprops); return ret; } @@ -1078,7 +1062,7 @@ static int qemuBlockStorageSourceGetBlockdevGetCacheProps(virStorageSourcePtr src, virJSONValuePtr props) { - virJSONValuePtr cacheobj; + VIR_AUTOPTR(virJSONValue) cacheobj = NULL; bool direct = false; bool noflush = false; @@ -1094,10 +1078,9 @@ qemuBlockStorageSourceGetBlockdevGetCacheProps(virStorageSourcePtr src, NULL) < 0) return -1; - if (virJSONValueObjectAppend(props, "cache", cacheobj) < 0) { - virJSONValueFree(cacheobj); + if (virJSONValueObjectAppend(props, "cache", cacheobj) < 0) return -1; - } + cacheobj = NULL; return 0; } @@ -1116,7 +1099,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, bool legacy) { int actualType = virStorageSourceGetActualType(src); - virJSONValuePtr fileprops = NULL; + VIR_AUTOPTR(virJSONValue) fileprops = NULL; virJSONValuePtr ret = NULL; switch ((virStorageType)actualType) { @@ -1209,7 +1192,6 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, VIR_STEAL_PTR(ret, fileprops); cleanup: - virJSONValueFree(fileprops); return ret; } @@ -1292,7 +1274,7 @@ qemuBlockStorageSourceGetFormatQcowGenericProps(virStorageSourcePtr src, const char *format, virJSONValuePtr props) { - virJSONValuePtr encprops = NULL; + VIR_AUTOPTR(virJSONValue) encprops = NULL; int ret = -1; if (qemuBlockStorageSourceGetCryptoProps(src, &encprops) < 0) @@ -1306,7 +1288,6 @@ qemuBlockStorageSourceGetFormatQcowGenericProps(virStorageSourcePtr src, ret = 0; cleanup: - virJSONValueFree(encprops); return ret; } @@ -1342,7 +1323,7 @@ qemuBlockStorageSourceGetBlockdevFormatCommonProps(virStorageSourcePtr src) const char *discard = NULL; int detectZeroesMode = virDomainDiskGetDetectZeroesMode(src->discard, src->detect_zeroes); - virJSONValuePtr props = NULL; + VIR_AUTOPTR(virJSONValue) props = NULL; virJSONValuePtr ret = NULL; if (qemuBlockNodeNameValidate(src->nodeformat) < 0) @@ -1372,7 +1353,6 @@ qemuBlockStorageSourceGetBlockdevFormatCommonProps(virStorageSourcePtr src) VIR_STEAL_PTR(ret, props); cleanup: - virJSONValueFree(props); return ret; } @@ -1381,7 +1361,7 @@ static virJSONValuePtr qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) { const char *driver = NULL; - virJSONValuePtr props = NULL; + VIR_AUTOPTR(virJSONValue) props = NULL; virJSONValuePtr ret = NULL; if (!(props = qemuBlockStorageSourceGetBlockdevFormatCommonProps(src))) @@ -1449,7 +1429,6 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) VIR_STEAL_PTR(ret, props); cleanup: - virJSONValueFree(props); return ret; } @@ -1468,7 +1447,7 @@ virJSONValuePtr qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src) { bool backingSupported = src->format >= VIR_STORAGE_FILE_BACKING; - virJSONValuePtr props = NULL; + VIR_AUTOPTR(virJSONValue) props = NULL; virJSONValuePtr ret = NULL; if (virStorageSourceHasBacking(src) && !backingSupported) { @@ -1500,7 +1479,6 @@ qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src) VIR_STEAL_PTR(ret, props); cleanup: - virJSONValueFree(props); return ret; } -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:14PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 80 ++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 51 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viruri.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/util/viruri.h b/src/util/viruri.h index b6e97dafe6..f5b472860e 100644 --- a/src/util/viruri.h +++ b/src/util/viruri.h @@ -25,6 +25,7 @@ # include "internal.h" # include "virconf.h" +# include "virautoclean.h" typedef struct _virURI virURI; typedef virURI *virURIPtr; @@ -60,6 +61,7 @@ char *virURIFormat(virURIPtr uri) char *virURIFormatParams(virURIPtr uri); void virURIFree(virURIPtr uri); +VIR_DEFINE_AUTOPTR_FUNC(virURI, virURIFree); int virURIResolveAlias(virConfPtr conf, const char *alias, char **uri); # define VIR_URI_SERVER(uri) ((uri) && (uri)->server ? (uri)->server : "localhost") -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:15PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viruri.h | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 769e07d3d8..0504a79957 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -435,7 +435,7 @@ qemuBlockStorageSourceSupportsConcurrentAccess(virStorageSourcePtr src) virURIPtr qemuBlockStorageSourceGetURI(virStorageSourcePtr src) { - virURIPtr uri = NULL; + VIR_AUTOPTR(virURI) uri = NULL; virURIPtr ret = NULL; if (src->nhosts != 1) { @@ -480,7 +480,6 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src) VIR_STEAL_PTR(ret, uri); cleanup: - virURIFree(uri); return ret; } @@ -745,7 +744,7 @@ qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src) const char *passwordalias = NULL; const char *username = NULL; virJSONValuePtr ret = NULL; - virURIPtr uri = NULL; + VIR_AUTOPTR(virURI) uri = NULL; char *uristr = NULL; const char *driver; @@ -782,7 +781,6 @@ qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src) NULL)); cleanup: - virURIFree(uri); VIR_FREE(uristr); return ret; -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:16PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 0504a79957..b00f4c6f8e 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -240,8 +240,8 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr namednodes, virJSONValuePtr blockstats) { struct qemuBlockNodeNameGetBackingChainData data; - virHashTablePtr namednodestable = NULL; - virHashTablePtr disks = NULL; + VIR_AUTOPTR(virHashTable) namednodestable = NULL; + VIR_AUTOPTR(virHashTable) disks = NULL; virHashTablePtr ret = NULL; memset(&data, 0, sizeof(data)); @@ -268,8 +268,6 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr namednodes, VIR_STEAL_PTR(ret, disks); cleanup: - virHashFree(namednodestable); - virHashFree(disks); return ret; } @@ -344,7 +342,7 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, qemuDomainAsyncJob asyncJob) { qemuDomainObjPrivatePtr priv = vm->privateData; - virHashTablePtr disktable = NULL; + VIR_AUTOPTR(virHashTable) disktable = NULL; VIR_AUTOPTR(virJSONValue) data = NULL; VIR_AUTOPTR(virJSONValue) blockstats = NULL; virDomainDiskDefPtr disk; @@ -376,7 +374,6 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, ret = 0; cleanup: - virHashFree(disktable); return ret; } @@ -394,19 +391,20 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, virHashTablePtr qemuBlockGetNodeData(virJSONValuePtr data) { + VIR_AUTOPTR(virHashTable) nodedata = NULL; virHashTablePtr ret = NULL; - if (!(ret = virHashCreate(50, virJSONValueHashFree))) + if (!(nodedata = virHashCreate(50, virJSONValueHashFree))) return NULL; if (virJSONValueArrayForeachSteal(data, - qemuBlockNamedNodesArrayToHash, ret) < 0) + qemuBlockNamedNodesArrayToHash, nodedata) < 0) goto error; + VIR_STEAL_PTR(ret, nodedata); return ret; error: - virHashFree(ret); return NULL; } -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:17PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b00f4c6f8e..822ba44e18 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -293,7 +293,7 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk, { qemuBlockNodeNameBackingChainDataPtr entry = NULL; virStorageSourcePtr src = disk->src; - char *alias = NULL; + VIR_AUTOFREE(char *) alias = NULL; int ret = -1; /* don't attempt the detection if the top level already has node names */ @@ -328,7 +328,6 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk, ret = 0; cleanup: - VIR_FREE(alias); if (ret < 0) qemuBlockDiskClearDetectedNodes(disk); @@ -503,7 +502,7 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, virJSONValuePtr ret = NULL; const char *transport; const char *field; - char *port = NULL; + VIR_AUTOFREE(char *) port = NULL; switch ((virStorageNetHostTransport) host->transport) { case VIR_STORAGE_NET_HOST_TRANS_TCP: @@ -547,7 +546,6 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, VIR_STEAL_PTR(ret, server); cleanup: - VIR_FREE(port); return ret; } @@ -607,7 +605,7 @@ static virJSONValuePtr qemuBlockStorageSourceBuildJSONInetSocketAddress(virStorageNetHostDefPtr host) { virJSONValuePtr ret = NULL; - char *port = NULL; + VIR_AUTOFREE(char *) port = NULL; if (host->transport != VIR_STORAGE_NET_HOST_TRANS_TCP) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -623,7 +621,6 @@ qemuBlockStorageSourceBuildJSONInetSocketAddress(virStorageNetHostDefPtr host) "s:port", port, NULL)); - VIR_FREE(port); return ret; } @@ -743,7 +740,7 @@ qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src) const char *username = NULL; virJSONValuePtr ret = NULL; VIR_AUTOPTR(virURI) uri = NULL; - char *uristr = NULL; + VIR_AUTOFREE(char *) uristr = NULL; const char *driver; /** @@ -779,7 +776,6 @@ qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src) NULL)); cleanup: - VIR_FREE(uristr); return ret; } @@ -790,11 +786,11 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) { qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); const char *protocol = virStorageNetProtocolTypeToString(src->protocol); - char *target = NULL; + VIR_AUTOFREE(char *) target = NULL; char *lunStr = NULL; char *username = NULL; char *objalias = NULL; - char *portal = NULL; + VIR_AUTOFREE(char *) portal = NULL; unsigned int lun = 0; virJSONValuePtr ret = NULL; @@ -852,8 +848,6 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) goto cleanup; cleanup: - VIR_FREE(target); - VIR_FREE(portal); return ret; } @@ -1693,8 +1687,8 @@ qemuBlockSnapshotAddLegacy(virJSONValuePtr actions, bool reuse) { const char *format = virStorageFileFormatTypeToString(newsrc->format); - char *device = NULL; - char *source = NULL; + VIR_AUTOFREE(char *) device = NULL; + VIR_AUTOFREE(char *) source = NULL; int ret = -1; if (!(device = qemuAliasDiskDriveFromDisk(disk))) @@ -1714,8 +1708,6 @@ qemuBlockSnapshotAddLegacy(virJSONValuePtr actions, ret = 0; cleanup: - VIR_FREE(device); - VIR_FREE(source); return ret; } -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:18PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
This is a locally used helper struct but we can make use of automatic freeing for it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 822ba44e18..2017eca139 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -88,6 +88,9 @@ qemuBlockNodeNameBackingChainDataFree(qemuBlockNodeNameBackingChainDataPtr data) VIR_FREE(data); } +VIR_DEFINE_AUTOPTR_FUNC(qemuBlockNodeNameBackingChainData, + qemuBlockNodeNameBackingChainDataFree); + static void qemuBlockNodeNameBackingChainDataHashEntryFree(void *opaque, @@ -128,7 +131,7 @@ qemuBlockNodeNameGetBackingChainBacking(virJSONValuePtr next, virHashTablePtr nodenamestable, qemuBlockNodeNameBackingChainDataPtr *nodenamedata) { - qemuBlockNodeNameBackingChainDataPtr data = NULL; + VIR_AUTOPTR(qemuBlockNodeNameBackingChainData) data = NULL; qemuBlockNodeNameBackingChainDataPtr backingdata = NULL; virJSONValuePtr backing = virJSONValueObjectGetObject(next, "backing"); virJSONValuePtr parent = virJSONValueObjectGetObject(next, "parent"); @@ -189,7 +192,6 @@ qemuBlockNodeNameGetBackingChainBacking(virJSONValuePtr next, ret = 0; cleanup: - qemuBlockNodeNameBackingChainDataFree(data); return ret; } @@ -201,7 +203,7 @@ qemuBlockNodeNameGetBackingChainDisk(size_t pos ATTRIBUTE_UNUSED, { struct qemuBlockNodeNameGetBackingChainData *data = opaque; const char *device = virJSONValueObjectGetString(item, "device"); - qemuBlockNodeNameBackingChainDataPtr devicedata = NULL; + VIR_AUTOPTR(qemuBlockNodeNameBackingChainData) devicedata = NULL; int ret = -1; if (qemuBlockNodeNameGetBackingChainBacking(item, data->nodenamestable, @@ -216,7 +218,6 @@ qemuBlockNodeNameGetBackingChainDisk(size_t pos ATTRIBUTE_UNUSED, ret = 1; /* we don't really want to steal @item */ cleanup: - qemuBlockNodeNameBackingChainDataFree(devicedata); return ret; } -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:19PM +0200, Peter Krempa wrote:
This is a locally used helper struct but we can make use of automatic freeing for it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 206 +++++++++++++++--------------------------- 1 file changed, 72 insertions(+), 134 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 2017eca139..31800919a7 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -142,7 +142,6 @@ qemuBlockNodeNameGetBackingChainBacking(virJSONValuePtr next, const char *drvparent = NULL; const char *parentnodename = NULL; const char *filename = NULL; - int ret = -1; if (!nodename) return 0; @@ -172,27 +171,24 @@ qemuBlockNodeNameGetBackingChainBacking(virJSONValuePtr next, } if (VIR_ALLOC(data) < 0) - goto cleanup; + return -1; if (VIR_STRDUP(data->nodeformat, nodename) < 0 || VIR_STRDUP(data->nodestorage, parentnodename) < 0 || VIR_STRDUP(data->qemufilename, filename) < 0 || VIR_STRDUP(data->drvformat, drvname) < 0 || VIR_STRDUP(data->drvstorage, drvparent) < 0) - goto cleanup; + return -1; if (backing && qemuBlockNodeNameGetBackingChainBacking(backing, nodenamestable, &backingdata) < 0) - goto cleanup; + return -1; VIR_STEAL_PTR(data->backing, backingdata); VIR_STEAL_PTR(*nodenamedata, data); - ret = 0; - - cleanup: - return ret; + return 0; } @@ -204,22 +200,17 @@ qemuBlockNodeNameGetBackingChainDisk(size_t pos ATTRIBUTE_UNUSED, struct qemuBlockNodeNameGetBackingChainData *data = opaque; const char *device = virJSONValueObjectGetString(item, "device"); VIR_AUTOPTR(qemuBlockNodeNameBackingChainData) devicedata = NULL; - int ret = -1; if (qemuBlockNodeNameGetBackingChainBacking(item, data->nodenamestable, &devicedata) < 0) - goto cleanup; + return -1; if (devicedata && virHashAddEntry(data->disks, device, devicedata) < 0) - goto cleanup; + return -1; devicedata = NULL; - ret = 1; /* we don't really want to steal @item */ - - cleanup: - - return ret; + return 1; /* we don't really want to steal @item */ } @@ -248,15 +239,15 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr namednodes, memset(&data, 0, sizeof(data)); if (!(namednodestable = virHashCreate(50, virJSONValueHashFree))) - goto cleanup; + return NULL; if (virJSONValueArrayForeachSteal(namednodes, qemuBlockNamedNodesArrayToHash, namednodestable) < 0) - goto cleanup; + return NULL; if (!(disks = virHashCreate(50, qemuBlockNodeNameBackingChainDataHashEntryFree))) - goto cleanup; + return NULL; data.nodenamestable = namednodestable; data.disks = disks; @@ -264,13 +255,10 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr namednodes, if (virJSONValueArrayForeachSteal(blockstats, qemuBlockNodeNameGetBackingChainDisk, &data) < 0) - goto cleanup; + return NULL; VIR_STEAL_PTR(ret, disks); - - cleanup: - - return ret; + return ret; } @@ -347,7 +335,6 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, VIR_AUTOPTR(virJSONValue) blockstats = NULL; virDomainDiskDefPtr disk; size_t i; - int ret = -1; if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_NAMED_BLOCK_NODES)) return 0; @@ -359,23 +346,19 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver, blockstats = qemuMonitorQueryBlockstats(qemuDomainGetMonitor(vm)); if (qemuDomainObjExitMonitor(driver, vm) < 0 || !data || !blockstats) - goto cleanup; + return -1; if (!(disktable = qemuBlockNodeNameGetBackingChain(data, blockstats))) - goto cleanup; + return -1; for (i = 0; i < vm->def->ndisks; i++) { disk = vm->def->disks[i]; if (qemuBlockDiskDetectNodes(disk, disktable) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - - return ret; + return 0; } @@ -399,13 +382,10 @@ qemuBlockGetNodeData(virJSONValuePtr data) if (virJSONValueArrayForeachSteal(data, qemuBlockNamedNodesArrayToHash, nodedata) < 0) - goto error; + return NULL; VIR_STEAL_PTR(ret, nodedata); return ret; - - error: - return NULL; } @@ -440,44 +420,42 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src) virReportError(VIR_ERR_INTERNAL_ERROR, _("protocol '%s' accepts only one host"), virStorageNetProtocolTypeToString(src->protocol)); - goto cleanup; + return NULL; } if (VIR_ALLOC(uri) < 0) - goto cleanup; + return NULL; if (src->hosts->transport == VIR_STORAGE_NET_HOST_TRANS_TCP) { uri->port = src->hosts->port; if (VIR_STRDUP(uri->scheme, virStorageNetProtocolTypeToString(src->protocol)) < 0) - goto cleanup; + return NULL; } else { if (virAsprintf(&uri->scheme, "%s+%s", virStorageNetProtocolTypeToString(src->protocol), virStorageNetHostTransportTypeToString(src->hosts->transport)) < 0) - goto cleanup; + return NULL; } if (src->path) { if (src->volume) { if (virAsprintf(&uri->path, "/%s/%s", src->volume, src->path) < 0) - goto cleanup; + return NULL; } else { if (virAsprintf(&uri->path, "%s%s", src->path[0] == '/' ? "" : "/", src->path) < 0) - goto cleanup; + return NULL; } } if (VIR_STRDUP(uri->server, src->hosts->name) < 0) - goto cleanup; + return NULL; VIR_STEAL_PTR(ret, uri); - - cleanup: return ret; } @@ -513,14 +491,14 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, transport = "inet"; if (virAsprintf(&port, "%u", host->port) < 0) - goto cleanup; + return NULL; if (virJSONValueObjectCreate(&server, "s:type", transport, "s:host", host->name, "s:port", port, NULL) < 0) - goto cleanup; + return NULL; break; case VIR_STORAGE_NET_HOST_TRANS_UNIX: @@ -533,7 +511,7 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, "s:type", "unix", field, host->socket, NULL) < 0) - goto cleanup; + return NULL; break; case VIR_STORAGE_NET_HOST_TRANS_RDMA: @@ -541,13 +519,10 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, virReportError(VIR_ERR_INTERNAL_ERROR, _("transport protocol '%s' is not yet supported"), virStorageNetHostTransportTypeToString(host->transport)); - goto cleanup; + return NULL; } VIR_STEAL_PTR(ret, server); - - cleanup: - return ret; } @@ -571,24 +546,21 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src, size_t i; if (!(servers = virJSONValueNewArray())) - goto cleanup; + return NULL; for (i = 0; i < src->nhosts; i++) { host = src->hosts + i; if (!(server = qemuBlockStorageSourceBuildJSONSocketAddress(host, legacy))) - goto cleanup; + return NULL; if (virJSONValueArrayAppend(servers, server) < 0) - goto cleanup; + return NULL; server = NULL; } VIR_STEAL_PTR(ret, servers); - - cleanup: - return ret; } @@ -643,24 +615,21 @@ qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src) size_t i; if (!(servers = virJSONValueNewArray())) - goto cleanup; + return NULL; for (i = 0; i < src->nhosts; i++) { host = src->hosts + i; if (!(server = qemuBlockStorageSourceBuildJSONInetSocketAddress(host))) - goto cleanup; + return NULL; if (virJSONValueArrayAppend(servers, server) < 0) - goto cleanup; + return NULL; server = NULL; } VIR_STEAL_PTR(ret, servers); - - cleanup: - return ret; } @@ -687,16 +656,13 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src, "s:volume", src->volume, "s:path", src->path, "a:server", &servers, NULL) < 0) - goto cleanup; + return NULL; if (src->debug && virJSONValueObjectAdd(props, "u:debug", src->debugLevel, NULL) < 0) - goto cleanup; + return NULL; VIR_STEAL_PTR(ret, props); - - cleanup: - return ret; } @@ -759,10 +725,10 @@ qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src) driver = virStorageNetProtocolTypeToString(src->protocol); if (!(uri = qemuBlockStorageSourceGetURI(src))) - goto cleanup; + return NULL; if (!(uristr = virURIFormat(uri))) - goto cleanup; + return NULL; if (src->auth) { username = src->auth->username; @@ -776,8 +742,6 @@ qemuBlockStorageSourceGetCURLProps(virStorageSourcePtr src) "S:password-secret", passwordalias, NULL)); - cleanup: - return ret; } @@ -807,7 +771,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) */ if (VIR_STRDUP(target, src->path) < 0) - goto cleanup; + return NULL; /* Separate the target and lun */ if ((lunStr = strchr(target, '/'))) { @@ -816,7 +780,7 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot parse target for lunStr '%s'"), target); - goto cleanup; + return NULL; } } @@ -824,11 +788,11 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) if (virSocketAddrNumericFamily(src->hosts[0].name) == AF_INET6) { if (virAsprintf(&portal, "[%s]:%u", src->hosts[0].name, src->hosts[0].port) < 0) - goto cleanup; + return NULL; } else { if (virAsprintf(&portal, "%s:%u", src->hosts[0].name, src->hosts[0].port) < 0) - goto cleanup; + return NULL; } if (src->auth) { @@ -846,9 +810,6 @@ qemuBlockStorageSourceGetISCSIProps(virStorageSourcePtr src) "S:password-secret", objalias, "S:initiator-name", src->initiator.iqn, NULL)); - goto cleanup; - - cleanup: return ret; } @@ -876,9 +837,8 @@ qemuBlockStorageSourceGetNBDProps(virStorageSourcePtr src) "S:export", src->path, "S:tls-creds", src->tlsAlias, NULL) < 0) - goto cleanup; + return NULL; - cleanup: return ret; } @@ -903,17 +863,17 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src) keysecret = srcPriv->secinfo->s.aes.alias; /* the auth modes are modelled after our old command line generator */ if (!(authmodes = virJSONValueNewArray())) - goto cleanup; + return NULL; if (!(mode = virJSONValueNewString("cephx")) || virJSONValueArrayAppend(authmodes, mode) < 0) - goto cleanup; + return NULL; mode = NULL; if (!(mode = virJSONValueNewString("none")) || virJSONValueArrayAppend(authmodes, mode) < 0) - goto cleanup; + return NULL; mode = NULL; } @@ -929,9 +889,8 @@ qemuBlockStorageSourceGetRBDProps(virStorageSourcePtr src) "A:auth-client-required", &authmodes, "S:key-secret", keysecret, NULL) < 0) - goto cleanup; + return NULL; - cleanup: return ret; } @@ -959,9 +918,8 @@ qemuBlockStorageSourceGetSheepdogProps(virStorageSourcePtr src) "a:server", &serverprops, "s:vdi", src->path, NULL) < 0) - goto cleanup; + return NULL; - cleanup: return ret; } @@ -992,9 +950,8 @@ qemuBlockStorageSourceGetSshProps(virStorageSourcePtr src) "a:server", &serverprops, "S:user", username, NULL) < 0) - goto cleanup; + return NULL; - cleanup: return ret; } @@ -1167,22 +1124,20 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, if (qemuBlockNodeNameValidate(src->nodestorage) < 0 || virJSONValueObjectAdd(fileprops, "S:node-name", src->nodestorage, NULL) < 0) - goto cleanup; + return NULL; if (!legacy) { if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, fileprops) < 0) - goto cleanup; + return NULL; if (virJSONValueObjectAdd(fileprops, "b:read-only", src->readonly, "s:discard", "unmap", NULL) < 0) - goto cleanup; + return NULL; } VIR_STEAL_PTR(ret, fileprops); - - cleanup: return ret; } @@ -1266,7 +1221,6 @@ qemuBlockStorageSourceGetFormatQcowGenericProps(virStorageSourcePtr src, virJSONValuePtr props) { VIR_AUTOPTR(virJSONValue) encprops = NULL; - int ret = -1; if (qemuBlockStorageSourceGetCryptoProps(src, &encprops) < 0) return -1; @@ -1274,12 +1228,9 @@ qemuBlockStorageSourceGetFormatQcowGenericProps(virStorageSourcePtr src, if (virJSONValueObjectAdd(props, "s:driver", format, "A:encrypt", &encprops, NULL) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - return ret; + return 0; } @@ -1339,11 +1290,9 @@ qemuBlockStorageSourceGetBlockdevFormatCommonProps(virStorageSourcePtr src) return NULL; if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, props) < 0) - goto cleanup; + return NULL; VIR_STEAL_PTR(ret, props); - - cleanup: return ret; } @@ -1356,7 +1305,7 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) virJSONValuePtr ret = NULL; if (!(props = qemuBlockStorageSourceGetBlockdevFormatCommonProps(src))) - goto cleanup; + return NULL; switch ((virStorageFileFormat) src->format) { case VIR_STORAGE_FILE_FAT: @@ -1364,17 +1313,17 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) * put a raw layer on top */ case VIR_STORAGE_FILE_RAW: if (qemuBlockStorageSourceGetFormatRawProps(src, props) < 0) - goto cleanup; + return NULL; break; case VIR_STORAGE_FILE_QCOW2: if (qemuBlockStorageSourceGetFormatQcow2Props(src, props) < 0) - goto cleanup; + return NULL; break; case VIR_STORAGE_FILE_QCOW: if (qemuBlockStorageSourceGetFormatQcowGenericProps(src, "qcow", props) < 0) - goto cleanup; + return NULL; break; /* formats without any special parameters */ @@ -1405,22 +1354,19 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) virReportError(VIR_ERR_INTERNAL_ERROR, _("mishandled storage format '%s'"), virStorageFileFormatTypeToString(src->format)); - goto cleanup; + return NULL; case VIR_STORAGE_FILE_LAST: default: virReportEnumRangeError(virStorageFileFormat, src->format); - goto cleanup; + return NULL; } if (driver && virJSONValueObjectAdd(props, "s:driver", driver, NULL) < 0) - goto cleanup; + return NULL; VIR_STEAL_PTR(ret, props); - - cleanup: - return ret; } @@ -1445,31 +1391,29 @@ qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src) virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("storage format '%s' does not support backing store"), virStorageFileFormatTypeToString(src->format)); - goto cleanup; + return NULL; } if (!(props = qemuBlockStorageSourceGetBlockdevFormatProps(src))) - goto cleanup; + return NULL; if (virJSONValueObjectAppendString(props, "file", src->nodestorage) < 0) - goto cleanup; + return NULL; if (src->backingStore && backingSupported) { if (virStorageSourceHasBacking(src)) { if (virJSONValueObjectAppendString(props, "backing", src->backingStore->nodeformat) < 0) - goto cleanup; + return NULL; } else { /* chain is terminated, indicate that no detection should happen * in qemu */ if (virJSONValueObjectAppendNull(props, "backing") < 0) - goto cleanup; + return NULL; } } VIR_STEAL_PTR(ret, props); - - cleanup: return ret; } @@ -1520,14 +1464,12 @@ qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src) if (!(data->formatProps = qemuBlockStorageSourceGetBlockdevProps(src)) || !(data->storageProps = qemuBlockStorageSourceGetBackendProps(src, false))) - goto cleanup; + return NULL; data->storageNodeName = src->nodestorage; data->formatNodeName = src->nodeformat; VIR_STEAL_PTR(ret, data); - - cleanup: return ret; } @@ -1690,13 +1632,12 @@ qemuBlockSnapshotAddLegacy(virJSONValuePtr actions, const char *format = virStorageFileFormatTypeToString(newsrc->format); VIR_AUTOFREE(char *) device = NULL; VIR_AUTOFREE(char *) source = NULL; - int ret = -1; if (!(device = qemuAliasDiskDriveFromDisk(disk))) - goto cleanup; + return -1; if (qemuGetDriveSourceString(newsrc, NULL, &source) < 0) - goto cleanup; + return -1; if (qemuMonitorJSONTransactionAdd(actions, "blockdev-snapshot-sync", "s:device", device, @@ -1704,12 +1645,9 @@ qemuBlockSnapshotAddLegacy(virJSONValuePtr actions, "s:format", format, "S:mode", reuse ? "existing" : NULL, NULL) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - return ret; + return 0; } -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:20PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 206 +++++++++++++++--------------------------- 1 file changed, 72 insertions(+), 134 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
With the introduction of more and more internal data types which support VIR_AUTOPTR it's becoming common to see the following pattern: VIR_AUTOPTR(virSomething) some = NULL virSomethingPtr ret = NULL; ... (ret is not touched ) ... VIR_STEAL_PTR(ret, some); return ret; This patch introduces a macro named VIR_RETURN_PTR which returns the pointer directly without the need for an explicitly defined return variable and use of VIR_STEAL_PTR. Internally obviously a temporary pointer is created to allow setting the original pointer to NULL so that the VIR_AUTOPTR function does not free the memory which we want to actually return. The name of the temporary variable is deliberately long and complex to minimize the possibility of collision. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/internal.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/internal.h b/src/internal.h index cf03a82105..5a868bb00c 100644 --- a/src/internal.h +++ b/src/internal.h @@ -279,6 +279,21 @@ (b) = NULL; \ } while (0) +/** + * VIR_RETURN_PTR: + * @ret: pointer to return + * + * Returns value of @ret while clearing @ret. This ensures that pointers + * freed by using VIR_AUTOPTR can be easily passed back to the caller without + * any temporary variable. @ptr is evaluated more than once. + */ +# define VIR_RETURN_PTR(ptr) \ + do { \ + typeof(ptr) virTemporaryReturnPointer = (ptr); \ + (ptr) = NULL; \ + return virTemporaryReturnPointer; \ + } while (0) + /** * virCheckFlags: * @supported: an OR'ed set of supported flags -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:21PM +0200, Peter Krempa wrote:
With the introduction of more and more internal data types which support VIR_AUTOPTR it's becoming common to see the following pattern:
VIR_AUTOPTR(virSomething) some = NULL virSomethingPtr ret = NULL;
... (ret is not touched ) ...
VIR_STEAL_PTR(ret, some); return ret;
This patch introduces a macro named VIR_RETURN_PTR which returns the pointer directly without the need for an explicitly defined return variable and use of VIR_STEAL_PTR. Internally obviously a temporary pointer is created to allow setting the original pointer to NULL so that the VIR_AUTOPTR function does not free the memory which we want to actually return.
The name of the temporary variable is deliberately long and complex to minimize the possibility of collision.
Both my gcc and clang warn in that case: conf/domain_conf.c:2209:5: error: declaration shadows a local variable [-Werror,-Wshadow] VIR_RETURN_PTR(virTemporaryReturnPointer); ^ ./internal.h:292:21: note: expanded from macro 'VIR_RETURN_PTR' typeof(ptr) virTemporaryReturnPointer = (ptr); \ ^ conf/domain_conf.c:2199:26: note: previous declaration is here virDomainVsockDefPtr virTemporaryReturnPointer = NULL; ^
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/internal.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/src/internal.h b/src/internal.h index cf03a82105..5a868bb00c 100644 --- a/src/internal.h +++ b/src/internal.h @@ -279,6 +279,21 @@ (b) = NULL; \ } while (0)
+/** + * VIR_RETURN_PTR: + * @ret: pointer to return + * + * Returns value of @ret while clearing @ret. This ensures that pointers + * freed by using VIR_AUTOPTR can be easily passed back to the caller without + * any temporary variable. @ptr is evaluated more than once. + */ +# define VIR_RETURN_PTR(ptr) \ + do { \ + typeof(ptr) virTemporaryReturnPointer = (ptr); \ + (ptr) = NULL; \ + return virTemporaryReturnPointer; \ + } while (0) + /**
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Demonstrate how VIR_RETURN_PTR by refactoring qemu_block.c Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 48 +++++++++++-------------------------------- 1 file changed, 12 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 31800919a7..7d9f7ec3ab 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -234,7 +234,6 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr namednodes, struct qemuBlockNodeNameGetBackingChainData data; VIR_AUTOPTR(virHashTable) namednodestable = NULL; VIR_AUTOPTR(virHashTable) disks = NULL; - virHashTablePtr ret = NULL; memset(&data, 0, sizeof(data)); @@ -257,8 +256,7 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr namednodes, &data) < 0) return NULL; - VIR_STEAL_PTR(ret, disks); - return ret; + VIR_RETURN_PTR(disks); } @@ -375,7 +373,6 @@ virHashTablePtr qemuBlockGetNodeData(virJSONValuePtr data) { VIR_AUTOPTR(virHashTable) nodedata = NULL; - virHashTablePtr ret = NULL; if (!(nodedata = virHashCreate(50, virJSONValueHashFree))) return NULL; @@ -384,8 +381,7 @@ qemuBlockGetNodeData(virJSONValuePtr data) qemuBlockNamedNodesArrayToHash, nodedata) < 0) return NULL; - VIR_STEAL_PTR(ret, nodedata); - return ret; + VIR_RETURN_PTR(nodedata); } @@ -414,7 +410,6 @@ virURIPtr qemuBlockStorageSourceGetURI(virStorageSourcePtr src) { VIR_AUTOPTR(virURI) uri = NULL; - virURIPtr ret = NULL; if (src->nhosts != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -455,8 +450,7 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src) if (VIR_STRDUP(uri->server, src->hosts->name) < 0) return NULL; - VIR_STEAL_PTR(ret, uri); - return ret; + VIR_RETURN_PTR(uri); } @@ -478,7 +472,6 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, bool legacy) { VIR_AUTOPTR(virJSONValue) server = NULL; - virJSONValuePtr ret = NULL; const char *transport; const char *field; VIR_AUTOFREE(char *) port = NULL; @@ -522,8 +515,7 @@ qemuBlockStorageSourceBuildJSONSocketAddress(virStorageNetHostDefPtr host, return NULL; } - VIR_STEAL_PTR(ret, server); - return ret; + VIR_RETURN_PTR(server); } @@ -541,7 +533,6 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src, { VIR_AUTOPTR(virJSONValue) servers = NULL; VIR_AUTOPTR(virJSONValue) server = NULL; - virJSONValuePtr ret = NULL; virStorageNetHostDefPtr host; size_t i; @@ -560,8 +551,7 @@ qemuBlockStorageSourceBuildHostsJSONSocketAddress(virStorageSourcePtr src, server = NULL; } - VIR_STEAL_PTR(ret, servers); - return ret; + VIR_RETURN_PTR(servers); } @@ -610,7 +600,6 @@ qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src) { VIR_AUTOPTR(virJSONValue) servers = NULL; VIR_AUTOPTR(virJSONValue) server = NULL; - virJSONValuePtr ret = NULL; virStorageNetHostDefPtr host; size_t i; @@ -629,8 +618,7 @@ qemuBlockStorageSourceBuildHostsJSONInetSocketAddress(virStorageSourcePtr src) server = NULL; } - VIR_STEAL_PTR(ret, servers); - return ret; + VIR_RETURN_PTR(servers); } @@ -640,7 +628,6 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src, { VIR_AUTOPTR(virJSONValue) servers = NULL; VIR_AUTOPTR(virJSONValue) props = NULL; - virJSONValuePtr ret = NULL; if (!(servers = qemuBlockStorageSourceBuildHostsJSONSocketAddress(src, legacy))) return NULL; @@ -662,8 +649,7 @@ qemuBlockStorageSourceGetGlusterProps(virStorageSourcePtr src, virJSONValueObjectAdd(props, "u:debug", src->debugLevel, NULL) < 0) return NULL; - VIR_STEAL_PTR(ret, props); - return ret; + VIR_RETURN_PTR(props); } @@ -1048,7 +1034,6 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, { int actualType = virStorageSourceGetActualType(src); VIR_AUTOPTR(virJSONValue) fileprops = NULL; - virJSONValuePtr ret = NULL; switch ((virStorageType)actualType) { case VIR_STORAGE_TYPE_BLOCK: @@ -1137,8 +1122,7 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src, return NULL; } - VIR_STEAL_PTR(ret, fileprops); - return ret; + VIR_RETURN_PTR(fileprops); } @@ -1266,7 +1250,6 @@ qemuBlockStorageSourceGetBlockdevFormatCommonProps(virStorageSourcePtr src) int detectZeroesMode = virDomainDiskGetDetectZeroesMode(src->discard, src->detect_zeroes); VIR_AUTOPTR(virJSONValue) props = NULL; - virJSONValuePtr ret = NULL; if (qemuBlockNodeNameValidate(src->nodeformat) < 0) return NULL; @@ -1292,8 +1275,7 @@ qemuBlockStorageSourceGetBlockdevFormatCommonProps(virStorageSourcePtr src) if (qemuBlockStorageSourceGetBlockdevGetCacheProps(src, props) < 0) return NULL; - VIR_STEAL_PTR(ret, props); - return ret; + VIR_RETURN_PTR(props); } @@ -1302,7 +1284,6 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) { const char *driver = NULL; VIR_AUTOPTR(virJSONValue) props = NULL; - virJSONValuePtr ret = NULL; if (!(props = qemuBlockStorageSourceGetBlockdevFormatCommonProps(src))) return NULL; @@ -1366,8 +1347,7 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) virJSONValueObjectAdd(props, "s:driver", driver, NULL) < 0) return NULL; - VIR_STEAL_PTR(ret, props); - return ret; + VIR_RETURN_PTR(props); } @@ -1385,7 +1365,6 @@ qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src) { bool backingSupported = src->format >= VIR_STORAGE_FILE_BACKING; VIR_AUTOPTR(virJSONValue) props = NULL; - virJSONValuePtr ret = NULL; if (virStorageSourceHasBacking(src) && !backingSupported) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -1413,8 +1392,7 @@ qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src) } } - VIR_STEAL_PTR(ret, props); - return ret; + VIR_RETURN_PTR(props); } @@ -1457,7 +1435,6 @@ qemuBlockStorageSourceAttachDataPtr qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src) { VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL; - qemuBlockStorageSourceAttachDataPtr ret = NULL; if (VIR_ALLOC(data) < 0) return NULL; @@ -1469,8 +1446,7 @@ qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src) data->storageNodeName = src->nodestorage; data->formatNodeName = src->nodeformat; - VIR_STEAL_PTR(ret, data); - return ret; + VIR_RETURN_PTR(data); } -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:22PM +0200, Peter Krempa wrote:
Demonstrate how VIR_RETURN_PTR by refactoring qemu_block.c
verb
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 48 +++++++++++-------------------------------- 1 file changed, 12 insertions(+), 36 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
The function open-codes addition into an array. Use the helper instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virjson.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 1dceb746b9..7b874bf2ec 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -609,7 +609,8 @@ virJSONValueObjectAppend(virJSONValuePtr object, const char *key, virJSONValuePtr value) { - char *newkey; + virJSONObjectPair pair = { NULL, value }; + int ret = -1; if (object->type != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -622,20 +623,14 @@ virJSONValueObjectAppend(virJSONValuePtr object, return -1; } - if (VIR_STRDUP(newkey, key) < 0) - return -1; - - if (VIR_REALLOC_N(object->data.object.pairs, - object->data.object.npairs + 1) < 0) { - VIR_FREE(newkey); + if (VIR_STRDUP(pair.key, key) < 0) return -1; - } - object->data.object.pairs[object->data.object.npairs].key = newkey; - object->data.object.pairs[object->data.object.npairs].value = value; - object->data.object.npairs++; + ret = VIR_APPEND_ELEMENT(object->data.object.pairs, + object->data.object.npairs, pair); - return 0; + VIR_FREE(pair.key); + return ret; } -- 2.20.1
On Fri, Apr 05, 2019 at 06:04:23PM +0200, Peter Krempa wrote:
The function open-codes addition into an array. Use the helper instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virjson.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa