[PATCH 0/7] Misc g_auto() rewrites
I've been looking at our tests lately and noticed an opportunity to rewrite pieces of code to g_auto() magic. Michal Prívozník (7): qemuagenttest: Don't leak virTypedParameter on failure Prefer g_auto(GStrv) over g_strfreev() qemu: Use g_autoptr(qemuMonitorCPUModelInfo) qemuConnectStealCPUModelFromInfo: Drop needless 'cleanup' label tests: Use g_autoptr(qemuMonitorTest) test: Use g_autofree more tests: Drop cleanup/error labels src/bhyve/bhyve_command.c | 3 +- src/bhyve/bhyve_parse_command.c | 22 +-- src/libxl/libxl_conf.c | 9 +- src/libxl/xen_common.c | 18 +- src/libxl/xen_xl.c | 17 +- src/lxc/lxc_container.c | 4 +- src/lxc/lxc_native.c | 24 +-- src/qemu/qemu_driver.c | 17 +- src/remote/remote_daemon_dispatch.c | 3 +- src/remote/remote_driver.c | 4 +- src/storage/storage_backend_rbd.c | 3 +- src/util/vircgroup.c | 3 +- src/util/vircgroupv2.c | 4 +- src/util/virfirmware.c | 6 +- src/util/viruri.c | 3 +- src/vbox/vbox_common.c | 12 +- src/vbox/vbox_snapshot_conf.c | 40 ++-- src/vbox/vbox_tmpl.c | 3 +- src/vz/vz_sdk.c | 3 +- tests/qemuagenttest.c | 286 ++++++++++++---------------- tests/qemucapabilitiestest.c | 22 +-- tests/qemuhotplugtest.c | 3 +- tests/qemumigparamstest.c | 40 ++-- tests/qemumonitorjsontest.c | 95 ++++----- tests/qemumonitortestutils.c | 63 +++--- tests/vboxsnapshotxmltest.c | 3 +- tests/virconftest.c | 3 +- tests/virfiletest.c | 3 +- tests/virstringtest.c | 3 +- tools/virsh-host.c | 13 +- tools/virt-login-shell-helper.c | 7 +- tools/vsh.c | 4 +- 32 files changed, 279 insertions(+), 464 deletions(-) -- 2.32.0
There are two functions (testQemuAgentOSInfo() and testQemuAgentTimezone()) which call virTypedParamsFree() only in successful paths. If an error is met then those parameters would be leaked. Fix this by placing the virTypedParamsFree() calls on better place. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuagenttest.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index bef6dfd152..cebeb5733e 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -1371,12 +1371,12 @@ testQemuAgentOSInfo(const void *data) VALIDATE_PARAM("os.kernel-release", "7601"); VALIDATE_PARAM("os.kernel-version", "6.1"); VALIDATE_PARAM("os.machine", "x86_64"); - virTypedParamsFree(params, nparams); ret = 0; cleanup: qemuMonitorTestFree(test); + virTypedParamsFree(params, nparams); return ret; } @@ -1394,6 +1394,8 @@ testQemuAgentTimezone(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + virTypedParameterPtr params = NULL; + int nparams = 0; int ret = -1; if (!test) @@ -1401,8 +1403,6 @@ testQemuAgentTimezone(const void *data) #define VALIDATE_TIMEZONE(response_, expected_name_, expected_offset_) \ do { \ - virTypedParameterPtr params_ = NULL; \ - int nparams_ = 0; \ int maxparams_ = 0; \ const char *name_ = NULL; \ int offset_; \ @@ -1411,15 +1411,18 @@ testQemuAgentTimezone(const void *data) if (qemuMonitorTestAddItem(test, "guest-get-timezone", \ response_) < 0) \ goto cleanup; \ + virTypedParamsFree(params, nparams); \ + params = NULL; \ + nparams = 0; \ if (qemuAgentGetTimezone(qemuMonitorTestGetAgent(test), \ - ¶ms_, &nparams_, &maxparams_, true) < 0) \ + ¶ms, &nparams, &maxparams_, true) < 0) \ goto cleanup; \ - if (nparams_ != 2) { \ + if (nparams != 2) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ - "Expected 2 params, got %d", nparams_); \ + "Expected 2 params, got %d", nparams); \ goto cleanup; \ } \ - if (virTypedParamsGetString(params_, nparams_, \ + if (virTypedParamsGetString(params, nparams, \ "timezone.name", &name_) < 0) { \ virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", \ "tiemzone.name"); \ @@ -1430,7 +1433,7 @@ testQemuAgentTimezone(const void *data) "Expected name '%s', got '%s'", expected_name_, name_); \ goto cleanup; \ } \ - if (virTypedParamsGetInt(params_, nparams_, \ + if (virTypedParamsGetInt(params, nparams, \ "timezone.offset", &offset_) < 0) { \ virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", \ "tiemzone.offset"); \ @@ -1442,7 +1445,6 @@ testQemuAgentTimezone(const void *data) expected_offset_); \ goto cleanup; \ } \ - virTypedParamsFree(params_, nparams_); \ } while (0) VALIDATE_TIMEZONE(testQemuAgentTimezoneResponse1, "IST", 19800); @@ -1454,6 +1456,7 @@ testQemuAgentTimezone(const void *data) cleanup: qemuMonitorTestFree(test); + virTypedParamsFree(params, nparams); return ret; } static int -- 2.32.0
There are a few cases where a string list is freed by an explicit call of g_strfreev(), but the same result can be achieved by g_atuo(GStrv). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/bhyve/bhyve_command.c | 3 +-- src/bhyve/bhyve_parse_command.c | 22 +++++----------- src/libxl/libxl_conf.c | 9 +++---- src/libxl/xen_common.c | 18 +++++-------- src/libxl/xen_xl.c | 17 +++--------- src/lxc/lxc_container.c | 4 +-- src/lxc/lxc_native.c | 24 +++++------------ src/qemu/qemu_driver.c | 6 ++--- src/remote/remote_daemon_dispatch.c | 3 +-- src/remote/remote_driver.c | 4 +-- src/storage/storage_backend_rbd.c | 3 +-- src/util/vircgroup.c | 3 +-- src/util/vircgroupv2.c | 4 +-- src/util/virfirmware.c | 6 ++--- src/util/viruri.c | 3 +-- src/vbox/vbox_common.c | 12 +++------ src/vbox/vbox_snapshot_conf.c | 40 ++++++++++------------------- src/vbox/vbox_tmpl.c | 3 +-- src/vz/vz_sdk.c | 3 +-- tests/qemumonitorjsontest.c | 3 +-- tests/vboxsnapshotxmltest.c | 3 +-- tests/virconftest.c | 3 +-- tests/virfiletest.c | 3 +-- tests/virstringtest.c | 3 +-- tools/virsh-host.c | 13 +++------- tools/virt-login-shell-helper.c | 7 +++-- tools/vsh.c | 4 +-- 27 files changed, 71 insertions(+), 155 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index ab9d3026cc..cf858dfcd6 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -833,12 +833,11 @@ virBhyveProcessBuildDestroyCmd(struct _bhyveConn *driver G_GNUC_UNUSED, static void virAppendBootloaderArgs(virCommand *cmd, virDomainDef *def) { - char **blargs; + g_auto(GStrv) blargs = NULL; /* XXX: Handle quoted? */ blargs = g_strsplit(def->os.bootloaderArgs, " ", 0); virCommandAddArgSet(cmd, (const char * const *)blargs); - g_strfreev(blargs); } static virCommand * diff --git a/src/bhyve/bhyve_parse_command.c b/src/bhyve/bhyve_parse_command.c index 76dcea6f21..f2c15f3aa5 100644 --- a/src/bhyve/bhyve_parse_command.c +++ b/src/bhyve/bhyve_parse_command.c @@ -127,7 +127,7 @@ bhyveCommandLineToArgv(const char *nativeConfig, const char *start; const char *next; char *line; - char **lines = NULL; + g_auto(GStrv) lines = NULL; size_t i; size_t line_count = 0; size_t lines_alloc = 0; @@ -168,7 +168,7 @@ bhyveCommandLineToArgv(const char *nativeConfig, for (i = 0; i < line_count; i++) { size_t j; - char **arglist = NULL; + g_auto(GStrv) arglist = NULL; size_t args_count = 0; size_t args_alloc = 0; @@ -223,23 +223,17 @@ bhyveCommandLineToArgv(const char *nativeConfig, if (!bhyve_argc) goto error; for (j = 0; j < args_count; j++) - _bhyve_argv[j] = arglist[j]; + _bhyve_argv[j] = g_steal_pointer(&arglist[j]); _bhyve_argv[j] = NULL; *bhyve_argc = args_count-1; - VIR_FREE(arglist); } else if (!_loader_argv) { VIR_REALLOC_N(_loader_argv, args_count + 1); if (!loader_argc) goto error; for (j = 0; j < args_count; j++) - _loader_argv[j] = arglist[j]; + _loader_argv[j] = g_steal_pointer(&arglist[j]); _loader_argv[j] = NULL; *loader_argc = args_count-1; - VIR_FREE(arglist); - } else { - /* To prevent a use-after-free here, only free the argument list - * when it is definitely not going to be used */ - g_strfreev(arglist); } } @@ -247,13 +241,11 @@ bhyveCommandLineToArgv(const char *nativeConfig, if (!(*bhyve_argv = _bhyve_argv)) goto error; - g_strfreev(lines); return 0; error: VIR_FREE(_loader_argv); VIR_FREE(_bhyve_argv); - g_strfreev(lines); return -1; } @@ -944,9 +936,9 @@ bhyveParseCommandLineString(const char* nativeConfig, { virDomainDef *def = NULL; int bhyve_argc = 0; - char **bhyve_argv = NULL; + g_auto(GStrv) bhyve_argv = NULL; int loader_argc = 0; - char **loader_argv = NULL; + g_auto(GStrv) loader_argv = NULL; if (!(def = virDomainDefNew(xmlopt))) goto cleanup; @@ -982,8 +974,6 @@ bhyveParseCommandLineString(const char* nativeConfig, } cleanup: - g_strfreev(loader_argv); - g_strfreev(bhyve_argv); return def; error: virDomainDefFree(def); diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 9f0739e1fa..f37c228139 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1887,8 +1887,7 @@ int libxlDriverGetDom0MaxmemConf(libxlDriverConfig *cfg, unsigned long long *maxmem) { - char **cmd_tokens = NULL; - char **mem_tokens = NULL; + g_auto(GStrv) cmd_tokens = NULL; size_t i; size_t j; libxl_physinfo physinfo; @@ -1899,6 +1898,8 @@ libxlDriverGetDom0MaxmemConf(libxlDriverConfig *cfg, goto physmem; for (i = 0; cmd_tokens[i] != NULL; i++) { + g_auto(GStrv) mem_tokens = NULL; + if (!STRPREFIX(cmd_tokens[i], "dom0_mem=")) continue; @@ -1934,8 +1935,6 @@ libxlDriverGetDom0MaxmemConf(libxlDriverConfig *cfg, goto cleanup; } } - g_strfreev(mem_tokens); - mem_tokens = NULL; } physmem: @@ -1950,8 +1949,6 @@ libxlDriverGetDom0MaxmemConf(libxlDriverConfig *cfg, ret = 0; cleanup: - g_strfreev(cmd_tokens); - g_strfreev(mem_tokens); return ret; } diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c index 87f090f979..32c31d240e 100644 --- a/src/libxl/xen_common.c +++ b/src/libxl/xen_common.c @@ -1045,7 +1045,7 @@ xenParseVifBridge(virDomainNetDef *net, const char *bridge) /* 'bridge' string contains a bridge name and one or more vlan trunks */ size_t i; size_t nvlans = 0; - char **vlanstr_list = g_strsplit(bridge, ":", 0); + g_auto(GStrv) vlanstr_list = g_strsplit(bridge, ":", 0); if (!vlanstr_list) return -1; @@ -1058,15 +1058,13 @@ xenParseVifBridge(virDomainNetDef *net, const char *bridge) net->vlan.tag = g_new0(unsigned int, nvlans); for (i = 1; i <= nvlans; i++) { - if (virStrToLong_ui(vlanstr_list[i], NULL, 10, &tag) < 0) { - g_strfreev(vlanstr_list); + if (virStrToLong_ui(vlanstr_list[i], NULL, 10, &tag) < 0) return -1; - } + net->vlan.tag[i - 1] = tag; } net->vlan.nTags = nvlans; net->vlan.trunk = true; - g_strfreev(vlanstr_list); net->virtPortProfile = g_new0(virNetDevVPortProfile, 1); net->virtPortProfile->virtPortType = VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH; @@ -1201,19 +1199,16 @@ xenParseVif(char *entry, const char *vif_typename) goto cleanup; } if (ip) { - char **ip_list = g_strsplit(ip, " ", 0); + g_auto(GStrv) ip_list = g_strsplit(ip, " ", 0); size_t i; if (!ip_list) goto cleanup; for (i = 0; ip_list[i]; i++) { - if (virDomainNetAppendIPAddress(net, ip_list[i], 0, 0) < 0) { - g_strfreev(ip_list); + if (virDomainNetAppendIPAddress(net, ip_list[i], 0, 0) < 0) goto cleanup; - } } - g_strfreev(ip_list); } if (script && script[0]) @@ -1579,7 +1574,7 @@ char * xenMakeIPList(virNetDevIPInfo *guestIP) { size_t i; - char **address_array; + g_auto(GStrv) address_array = NULL; char *ret = NULL; address_array = g_new0(char *, guestIP->nips + 1); @@ -1592,7 +1587,6 @@ xenMakeIPList(virNetDevIPInfo *guestIP) ret = g_strjoinv(" ", address_array); cleanup: - g_strfreev(address_array); return ret; } diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c index 850786e5c9..05d4abbe81 100644 --- a/src/libxl/xen_xl.c +++ b/src/libxl/xen_xl.c @@ -246,8 +246,7 @@ static int xenParseXLCPUID(virConf *conf, virDomainDef *def) { g_autofree char *cpuid_str = NULL; - char **cpuid_pairs = NULL; - char **name_and_value = NULL; + g_auto(GStrv) cpuid_pairs = NULL; size_t i; int ret = -1; int policy; @@ -283,7 +282,7 @@ xenParseXLCPUID(virConf *conf, virDomainDef *def) } for (i = 1; cpuid_pairs[i]; i++) { - name_and_value = g_strsplit(cpuid_pairs[i], "=", 2); + g_auto(GStrv) name_and_value = g_strsplit(cpuid_pairs[i], "=", 2); if (!name_and_value) goto cleanup; if (!name_and_value[0] || !name_and_value[1]) { @@ -313,16 +312,11 @@ xenParseXLCPUID(virConf *conf, virDomainDef *def) xenTranslateCPUFeature(name_and_value[0], true), policy) < 0) goto cleanup; - - g_strfreev(name_and_value); - name_and_value = NULL; } ret = 0; cleanup: - g_strfreev(name_and_value); - g_strfreev(cpuid_pairs); return ret; } @@ -406,7 +400,6 @@ xenParseXLVnuma(virConf *conf, { int ret = -1; char *tmp = NULL; - char **token = NULL; size_t vcpus = 0; size_t nr_nodes = 0; size_t vnodeCnt = 0; @@ -506,6 +499,7 @@ xenParseXLVnuma(virConf *conf, vcpus += virBitmapCountBits(cpumask); } else if (STRPREFIX(str, "vdistances")) { + g_auto(GStrv) token = NULL; size_t i, ndistances; unsigned int value; @@ -519,7 +513,6 @@ xenParseXLVnuma(virConf *conf, VIR_FREE(tmp); tmp = g_strdup(vtoken); - g_strfreev(token); if (!(token = g_strsplit(tmp, ",", 0))) goto cleanup; @@ -583,7 +576,6 @@ xenParseXLVnuma(virConf *conf, cleanup: if (ret) VIR_FREE(cpu); - g_strfreev(token); VIR_FREE(tmp); return ret; @@ -1301,7 +1293,7 @@ xenFormatXLOS(virConf *conf, virDomainDef *def) static int xenFormatXLCPUID(virConf *conf, virDomainDef *def) { - char **cpuid_pairs = NULL; + g_auto(GStrv) cpuid_pairs = NULL; g_autofree char *cpuid_string = NULL; size_t i, j; int ret = -1; @@ -1359,7 +1351,6 @@ xenFormatXLCPUID(virConf *conf, virDomainDef *def) ret = 0; cleanup: - g_strfreev(cpuid_pairs); return ret; } diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index d788e77196..13f2fd4c29 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -510,7 +510,7 @@ extern int pivot_root(const char * new_root, const char * put_old); static int lxcContainerUnmountSubtree(const char *prefix, bool isOldRootFS) { - char **mounts = NULL; + g_auto(GStrv) mounts = NULL; size_t nmounts = 0; size_t i; int saveErrno; @@ -555,8 +555,6 @@ static int lxcContainerUnmountSubtree(const char *prefix, ret = 0; cleanup: - g_strfreev(mounts); - return ret; } diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c index 3840652912..f3b8e85143 100644 --- a/src/lxc/lxc_native.c +++ b/src/lxc/lxc_native.c @@ -107,8 +107,8 @@ static char ** lxcStringSplit(const char *string) g_autofree char *tmp = NULL; size_t i; size_t ntokens = 0; - char **parts; - char **result = NULL; + g_auto(GStrv) parts = NULL; + g_auto(GStrv) result = NULL; tmp = g_strdup(string); @@ -132,12 +132,9 @@ static char ** lxcStringSplit(const char *string) result[ntokens - 2] = g_strdup(parts[i]); } - g_strfreev(parts); - return result; + return g_steal_pointer(&result); error: - g_strfreev(parts); - g_strfreev(result); return NULL; } @@ -145,7 +142,7 @@ static lxcFstab * lxcParseFstabLine(char *fstabLine) { lxcFstab *fstab = NULL; - char **parts; + g_auto(GStrv) parts = NULL; if (!fstabLine) return NULL; @@ -162,13 +159,10 @@ lxcParseFstabLine(char *fstabLine) fstab->type = g_strdup(parts[2]); fstab->options = g_strdup(parts[3]); - g_strfreev(parts); - return fstab; error: lxcFstabFree(fstab); - g_strfreev(parts); return NULL; } @@ -252,7 +246,7 @@ lxcAddFstabLine(virDomainDef *def, lxcFstab *fstab) { const char *src = NULL; g_autofree char *dst = NULL; - char **options = g_strsplit(fstab->options, ",", 0); + g_auto(GStrv) options = g_strsplit(fstab->options, ",", 0); bool readonly; int type = VIR_DOMAIN_FS_TYPE_MOUNT; unsigned long long usage = 0; @@ -307,7 +301,6 @@ lxcAddFstabLine(virDomainDef *def, lxcFstab *fstab) ret = 1; cleanup: - g_strfreev(options); return ret; } @@ -967,7 +960,7 @@ lxcSetCpusetTune(virDomainDef *def, virConf *properties) static int lxcBlkioDeviceWalkCallback(const char *name, virConfValue *value, void *data) { - char **parts = NULL; + g_auto(GStrv) parts = NULL; virBlkioDevice *device = NULL; virDomainDef *def = data; size_t i = 0; @@ -1044,7 +1037,6 @@ lxcBlkioDeviceWalkCallback(const char *name, virConfValue *value, void *data) ret = 0; cleanup: - g_strfreev(parts); return ret; } @@ -1072,7 +1064,7 @@ static void lxcSetCapDrop(virDomainDef *def, virConf *properties) { g_autofree char *value = NULL; - char **toDrop = NULL; + g_auto(GStrv) toDrop = NULL; const char *capString; size_t i; @@ -1087,8 +1079,6 @@ lxcSetCapDrop(virDomainDef *def, virConf *properties) } def->features[VIR_DOMAIN_FEATURE_CAPABILITIES] = VIR_DOMAIN_CAPABILITIES_POLICY_ALLOW; - - g_strfreev(toDrop); } virDomainDef * diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 70b5f37e6b..a12ef2227e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1329,7 +1329,7 @@ qemuGetSchedInfo(unsigned long long *cpuWait, { g_autofree char *proc = NULL; g_autofree char *data = NULL; - char **lines = NULL; + g_auto(GStrv) lines = NULL; size_t i; int ret = -1; double val; @@ -1392,7 +1392,6 @@ qemuGetSchedInfo(unsigned long long *cpuWait, ret = 0; cleanup: - g_strfreev(lines); return ret; } @@ -12517,7 +12516,7 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, bool migratable; virCPUDef *cpu = NULL; char *cpustr = NULL; - char **features = NULL; + g_auto(GStrv) features = NULL; virCheckFlags(VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES | VIR_CONNECT_BASELINE_CPU_MIGRATABLE, NULL); @@ -12591,7 +12590,6 @@ qemuConnectBaselineHypervisorCPU(virConnectPtr conn, cleanup: virCPUDefListFree(cpus); virCPUDefFree(cpu); - g_strfreev(features); return cpustr; } diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index bcfeadc2ae..d8fb02a89d 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -5824,7 +5824,7 @@ remoteDispatchConnectGetCPUModelNames(virNetServer *server G_GNUC_UNUSED, remote_connect_get_cpu_model_names_ret *ret) { int len, rv = -1; - char **models = NULL; + g_auto(GStrv) models = NULL; virConnectPtr conn = remoteGetHypervisorConn(client); if (!conn) @@ -5858,7 +5858,6 @@ remoteDispatchConnectGetCPUModelNames(virNetServer *server G_GNUC_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - g_strfreev(models); return rv; } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 719fcf4297..8f094db68c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6266,7 +6266,7 @@ remoteConnectGetCPUModelNames(virConnectPtr conn, { int rv = -1; size_t i; - char **retmodels = NULL; + g_auto(GStrv) retmodels = NULL; remote_connect_get_cpu_model_names_args args; remote_connect_get_cpu_model_names_ret ret; @@ -6307,8 +6307,6 @@ remoteConnectGetCPUModelNames(virConnectPtr conn, rv = ret.ret; cleanup: - g_strfreev(retmodels); - xdr_free((xdrproc_t) xdr_remote_connect_get_cpu_model_names_ret, (char *) &ret); done: diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index 28b4b7fae6..8276ce20ab 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -659,7 +659,7 @@ virStorageBackendRBDRefreshPool(virStoragePoolObj *pool) virStorageBackendRBDState *ptr = NULL; struct rados_cluster_stat_t clusterstat; struct rados_pool_stat_t poolstat; - char **names = NULL; + g_auto(GStrv) names = NULL; size_t i; if (!(ptr = virStorageBackendRBDNewState(pool))) @@ -724,7 +724,6 @@ virStorageBackendRBDRefreshPool(virStoragePoolObj *pool) ret = 0; cleanup: - g_strfreev(names); virStorageBackendRBDFreeState(&ptr); return ret; } diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 37b63a2e2d..6234095827 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -851,7 +851,7 @@ virCgroupAddThread(virCgroup *group, static int virCgroupSetPartitionSuffix(const char *path, char **res) { - char **tokens; + g_auto(GStrv) tokens = NULL; size_t i; int ret = -1; @@ -887,7 +887,6 @@ virCgroupSetPartitionSuffix(const char *path, char **res) ret = 0; cleanup: - g_strfreev(tokens); return ret; } diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c index 8881d3a88a..4c110940cf 100644 --- a/src/util/vircgroupv2.c +++ b/src/util/vircgroupv2.c @@ -264,7 +264,7 @@ virCgroupV2ParseControllersFile(virCgroup *group, int rc; g_autofree char *contStr = NULL; g_autofree char *contFile = NULL; - char **contList = NULL; + g_auto(GStrv) contList = NULL; char **tmp; if (parent) { @@ -300,8 +300,6 @@ virCgroupV2ParseControllersFile(virCgroup *group, tmp++; } - g_strfreev(contList); - return 0; } diff --git a/src/util/virfirmware.c b/src/util/virfirmware.c index 258d13bb73..cc5e315b7a 100644 --- a/src/util/virfirmware.c +++ b/src/util/virfirmware.c @@ -59,7 +59,7 @@ int virFirmwareParse(const char *str, virFirmware *firmware) { int ret = -1; - char **token; + g_auto(GStrv) token = NULL; if (!(token = g_strsplit(str, ":", 0))) goto cleanup; @@ -84,7 +84,6 @@ virFirmwareParse(const char *str, virFirmware *firmware) ret = 0; cleanup: - g_strfreev(token); return ret; } @@ -95,7 +94,7 @@ virFirmwareParseList(const char *list, size_t *nfirmwares) { int ret = -1; - char **token; + g_auto(GStrv) token = NULL; size_t i, j; if (!(token = g_strsplit(list, ":", 0))) @@ -126,6 +125,5 @@ virFirmwareParseList(const char *list, ret = 0; cleanup: - g_strfreev(token); return ret; } diff --git a/src/util/viruri.c b/src/util/viruri.c index 252e4f598e..3c73188a55 100644 --- a/src/util/viruri.c +++ b/src/util/viruri.c @@ -349,7 +349,7 @@ int virURIResolveAlias(virConf *conf, const char *alias, char **uri) { int ret = -1; - char **aliases = NULL; + g_auto(GStrv) aliases = NULL; *uri = NULL; @@ -358,7 +358,6 @@ virURIResolveAlias(virConf *conf, const char *alias, char **uri) if (aliases && *aliases) { ret = virURIFindAliasMatch(aliases, alias, uri); - g_strfreev(aliases); } else { ret = 0; } diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 45e7225ae1..cc90e634a3 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -4559,16 +4559,16 @@ vboxSnapshotRedefine(virDomainPtr dom, char *currentSnapshotXmlFilePath = NULL; PRUnichar *machineNameUtf16 = NULL; char *machineName = NULL; - char **realReadWriteDisksPath = NULL; + g_auto(GStrv) realReadWriteDisksPath = NULL; int realReadWriteDisksPathSize = 0; - char **realReadOnlyDisksPath = NULL; + g_auto(GStrv) realReadOnlyDisksPath = NULL; int realReadOnlyDisksPathSize = 0; virVBoxSnapshotConfSnapshot *newSnapshotPtr = NULL; unsigned char snapshotUuid[VIR_UUID_BUFLEN]; virVBoxSnapshotConfHardDisk **hardDiskToOpen = NULL; size_t hardDiskToOpenSize = 0; virVBoxSnapshotConfHardDisk *newHardDisk = NULL; - char **searchResultTab = NULL; + g_auto(GStrv) searchResultTab = NULL; ssize_t resultSize = 0; int it = 0; int jt = 0; @@ -5371,9 +5371,6 @@ vboxSnapshotRedefine(virDomainPtr dom, VIR_FREE(currentSnapshotXmlFilePath); VBOX_UTF16_FREE(machineNameUtf16); VBOX_UTF8_FREE(machineName); - g_strfreev(realReadOnlyDisksPath); - g_strfreev(realReadWriteDisksPath); - g_strfreev(searchResultTab); virVboxSnapshotConfHardDiskFree(newHardDisk); VIR_FREE(hardDiskToOpen); VIR_FREE(newSnapshotPtr); @@ -6792,7 +6789,7 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) char *settingsFilepath = NULL; virVBoxSnapshotConfMachine *snapshotMachineDesc = NULL; int isCurrent = -1; - char **searchResultTab = NULL; + g_auto(GStrv) searchResultTab = NULL; ssize_t resultSize = 0; int it = 0; PRUnichar *machineNameUtf16 = NULL; @@ -7195,7 +7192,6 @@ vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot) VBOX_RELEASE(machine); VBOX_UTF16_FREE(settingsFilePathUtf16); VBOX_UTF8_FREE(settingsFilepath); - g_strfreev(searchResultTab); VIR_FREE(snapshotMachineDesc); VBOX_UTF16_FREE(machineNameUtf16); VBOX_UTF8_FREE(machineName); diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index 65546b785e..5894ee1cac 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -38,7 +38,7 @@ virVBoxSnapshotConfCreateVBoxSnapshotConfHardDiskPtr(xmlNodePtr diskNode, virVBoxSnapshotConfHardDisk *hardDisk = NULL; xmlNodePtr *nodes = NULL; char *uuid = NULL; - char **searchTabResult = NULL; + g_auto(GStrv) searchTabResult = NULL; int resultSize = 0; size_t i = 0; int result = -1; @@ -103,7 +103,6 @@ virVBoxSnapshotConfCreateVBoxSnapshotConfHardDiskPtr(xmlNodePtr diskNode, VIR_FREE(nodes); VIR_FREE(location); VIR_FREE(tmp); - g_strfreev(searchTabResult); if (result < 0) { virVboxSnapshotConfHardDiskFree(hardDisk); hardDisk = NULL; @@ -184,7 +183,7 @@ virVBoxSnapshotConfRetrieveSnapshot(xmlNodePtr snapshotNode, xmlNodePtr snapshotsNode = NULL; xmlNodePtr *nodes = NULL; char *uuid = NULL; - char **searchTabResult = NULL; + g_auto(GStrv) searchTabResult = NULL; int resultSize = 0; size_t i = 0; int result = -1; @@ -270,7 +269,6 @@ virVBoxSnapshotConfRetrieveSnapshot(xmlNodePtr snapshotNode, } VIR_FREE(nodes); VIR_FREE(uuid); - g_strfreev(searchTabResult); return snapshot; } @@ -371,9 +369,9 @@ virVBoxSnapshotConfSerializeSnapshot(xmlNodePtr node, char *uuid = NULL; char *timeStamp = NULL; - char **firstRegex = NULL; + g_auto(GStrv) firstRegex = NULL; int firstRegexResult = 0; - char **secondRegex = NULL; + g_auto(GStrv) secondRegex = NULL; int secondRegexResult = 0; uuid = g_strdup_printf("{%s}", snapshot->uuid); @@ -450,8 +448,6 @@ virVBoxSnapshotConfSerializeSnapshot(xmlNodePtr node, xmlUnlinkNode(snapshotsNode); xmlFreeNode(snapshotsNode); } - g_strfreev(firstRegex); - g_strfreev(secondRegex); VIR_FREE(uuid); VIR_FREE(timeStamp); return result; @@ -581,7 +577,7 @@ virVBoxSnapshotConfLoadVboxFile(const char *filePath, g_autoptr(xmlXPathContext) xPathContext = NULL; char *currentStateModifiedString = NULL; - char **searchResultTab = NULL; + g_auto(GStrv) searchResultTab = NULL; ssize_t searchResultSize = 0; char *currentSnapshotAttribute = NULL; @@ -719,7 +715,6 @@ virVBoxSnapshotConfLoadVboxFile(const char *filePath, VIR_FREE(currentStateModifiedString); VIR_FREE(currentSnapshotAttribute); - g_strfreev(searchResultTab); if (ret < 0) { virVBoxSnapshotConfMachineFree(machineDescription); machineDescription = NULL; @@ -963,9 +958,9 @@ virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachine *machine, char *currentSnapshot = NULL; char *timeStamp = NULL; - char **firstRegex = NULL; + g_auto(GStrv) firstRegex = NULL; int firstRegexResult = 0; - char **secondRegex = NULL; + g_auto(GStrv) secondRegex = NULL; int secondRegexResult = 0; if (machine == NULL) { @@ -1171,9 +1166,6 @@ virVBoxSnapshotConfSaveVboxFile(virVBoxSnapshotConfMachine *machine, xmlUnlinkNode(machineNode); xmlFreeNode(machineNode); - - g_strfreev(firstRegex); - g_strfreev(secondRegex); return ret; } @@ -1216,7 +1208,7 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, { int result = -1; size_t i = 0; - char **ret = NULL; + g_auto(GStrv) ret = NULL; g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) xPathContext = NULL; xmlNodePtr *nodes = NULL; @@ -1253,15 +1245,12 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, if (sourceNode) ret[i] = virXMLPropString(sourceNode, "file"); } + *rwDisksPath = g_steal_pointer(&ret); result = 0; cleanup: - if (result < 0) { - g_strfreev(ret); + if (result < 0) nodeSize = -1; - } else { - *rwDisksPath = ret; - } VIR_FREE(nodes); return nodeSize; } @@ -1277,7 +1266,7 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const char *filePath, { int result = -1; size_t i = 0; - char **ret = NULL; + g_auto(GStrv) ret = NULL; g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) xPathContext = NULL; xmlNodePtr *nodes = NULL; @@ -1313,15 +1302,12 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const char *filePath, if (sourceNode) ret[i] = virXMLPropString(sourceNode, "file"); } + *roDisksPath = g_steal_pointer(&ret); result = 0; cleanup: - if (result < 0) { - g_strfreev(ret); + if (result < 0) nodeSize = -1; - } else { - *roDisksPath = ret; - } VIR_FREE(nodes); return nodeSize; } diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 1772a9b70e..7344882bbd 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1548,7 +1548,7 @@ _vrdeServerGetPorts(struct _vboxDriver *data, IVRDEServer *VRDEServer, PRUnichar *VRDEPortsValue = NULL; PRInt32 port = -1; ssize_t nmatches = 0; - char **matches = NULL; + g_auto(GStrv) matches = NULL; char *portUtf8 = NULL; /* get active (effective) port - available only when VM is running and has @@ -1596,7 +1596,6 @@ _vrdeServerGetPorts(struct _vboxDriver *data, IVRDEServer *VRDEServer, } cleanup: - g_strfreev(matches); VBOX_UTF8_FREE(portUtf8); VBOX_UTF16_FREE(VRDEPortsValue); VBOX_UTF16_FREE(VRDEPortsKey); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 2ba48134b0..1772f75c3e 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -705,7 +705,7 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk, { char *buf = NULL; int ret = -1; - char **matches = NULL; + g_auto(GStrv) matches = NULL; virURI *uri = NULL; fs->type = VIR_DOMAIN_FS_TYPE_FILE; @@ -767,7 +767,6 @@ prlsdkGetFSInfo(PRL_HANDLE prldisk, cleanup: VIR_FREE(buf); - g_strfreev(matches); return ret; } diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index e5ba39cd2f..25a91cdeb3 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1003,7 +1003,7 @@ testQemuMonitorJSONGetDeviceAliases(const void *opaque) const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; int ret = -1; - char **aliases = NULL; + g_auto(GStrv) aliases = NULL; const char **alias; const char *expected[] = { "virtio-disk25", "video0", "serial0", "ide0-0-0", "usb", NULL }; @@ -1053,7 +1053,6 @@ testQemuMonitorJSONGetDeviceAliases(const void *opaque) } cleanup: - g_strfreev(aliases); return ret; } diff --git a/tests/vboxsnapshotxmltest.c b/tests/vboxsnapshotxmltest.c index da39561456..d69eb3fc24 100644 --- a/tests/vboxsnapshotxmltest.c +++ b/tests/vboxsnapshotxmltest.c @@ -19,7 +19,7 @@ static char * testFilterXML(char *xml) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - char **xmlLines = NULL; + g_auto(GStrv) xmlLines = NULL; char **xmlLine; char *ret = NULL; @@ -39,7 +39,6 @@ testFilterXML(char *xml) ret = virBufferContentAndReset(&buf); cleanup: - g_strfreev(xmlLines); return ret; } diff --git a/tests/virconftest.c b/tests/virconftest.c index 5d1ad8a612..32d3d8849b 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -372,7 +372,7 @@ static int testConfParseStringList(const void *opaque G_GNUC_UNUSED) int ret = -1; g_autoptr(virConf) conf = virConfReadString(srcdata, 0); - char **str = NULL; + g_auto(GStrv) str = NULL; if (!conf) return -1; @@ -423,7 +423,6 @@ static int testConfParseStringList(const void *opaque G_GNUC_UNUSED) ret = 0; cleanup: - g_strfreev(str); return ret; } diff --git a/tests/virfiletest.c b/tests/virfiletest.c index 1434d6b7ba..2fbece8f63 100644 --- a/tests/virfiletest.c +++ b/tests/virfiletest.c @@ -65,7 +65,7 @@ struct testFileGetMountSubtreeData { static int testFileGetMountSubtree(const void *opaque) { int ret = -1; - char **gotmounts = NULL; + g_auto(GStrv) gotmounts = NULL; size_t gotnmounts = 0; const struct testFileGetMountSubtreeData *data = opaque; @@ -88,7 +88,6 @@ static int testFileGetMountSubtree(const void *opaque) data->mounts, data->nmounts); cleanup: - g_strfreev(gotmounts); return ret; } #endif /* ! defined WITH_MNTENT_H && defined WITH_GETMNTENT_R */ diff --git a/tests/virstringtest.c b/tests/virstringtest.c index 277deca934..77fcec5613 100644 --- a/tests/virstringtest.c +++ b/tests/virstringtest.c @@ -127,7 +127,7 @@ static int testStringSearch(const void *opaque) { const struct stringSearchData *data = opaque; - char **matches = NULL; + g_auto(GStrv) matches = NULL; ssize_t nmatches; int ret = -1; @@ -174,7 +174,6 @@ testStringSearch(const void *opaque) ret = 0; cleanup: - g_strfreev(matches); return ret; } diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 2a84c58be7..f6aa532b40 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -1059,7 +1059,7 @@ static char ** vshExtractCPUDefXMLs(vshControl *ctl, const char *xmlFile) { - char **cpus = NULL; + g_auto(GStrv) cpus = NULL; g_autofree char *buffer = NULL; g_autofree char *xmlStr = NULL; g_autoptr(xmlDoc) xml = NULL; @@ -1126,7 +1126,6 @@ vshExtractCPUDefXMLs(vshControl *ctl, return cpus; error: - g_strfreev(cpus); goto cleanup; } @@ -1163,7 +1162,7 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; bool ret = false; int result; - char **cpus = NULL; + g_auto(GStrv) cpus = NULL; unsigned int flags = 0; virshControl *priv = ctl->privData; @@ -1207,8 +1206,6 @@ cmdCPUCompare(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: - g_strfreev(cpus); - return ret; } @@ -1585,7 +1582,7 @@ cmdHypervisorCPUCompare(vshControl *ctl, const char *machine = NULL; bool ret = false; int result; - char **cpus = NULL; + g_auto(GStrv) cpus = NULL; unsigned int flags = 0; virshControl *priv = ctl->privData; @@ -1640,7 +1637,6 @@ cmdHypervisorCPUCompare(vshControl *ctl, ret = true; cleanup: - g_strfreev(cpus); return ret; } @@ -1699,7 +1695,7 @@ cmdHypervisorCPUBaseline(vshControl *ctl, const char *machine = NULL; bool ret = false; g_autofree char *result = NULL; - char **list = NULL; + g_auto(GStrv) list = NULL; unsigned int flags = 0; virshControl *priv = ctl->privData; @@ -1729,7 +1725,6 @@ cmdHypervisorCPUBaseline(vshControl *ctl, ret = true; } - g_strfreev(list); return ret; } diff --git a/tools/virt-login-shell-helper.c b/tools/virt-login-shell-helper.c index 5c6e007b09..0fb03da697 100644 --- a/tools/virt-login-shell-helper.c +++ b/tools/virt-login-shell-helper.c @@ -47,7 +47,8 @@ static int virLoginShellAllowedUser(virConf *conf, int ret = -1; size_t i; char *gname = NULL; - char **users = NULL, **entries; + g_auto(GStrv) users = NULL; + char **entries; if (virConfGetValueStringList(conf, "allowed_users", false, &users) < 0) goto cleanup; @@ -84,7 +85,6 @@ static int virLoginShellAllowedUser(virConf *conf, name, conf_file); cleanup: VIR_FREE(gname); - g_strfreev(users); return ret; } @@ -157,7 +157,7 @@ main(int argc, char **argv) uid_t uid; gid_t gid; char *name = NULL; - char **shargv = NULL; + g_auto(GStrv) shargv = NULL; size_t shargvlen = 0; char *shcmd = NULL; virSecurityModelPtr secmodel = NULL; @@ -403,7 +403,6 @@ main(int argc, char **argv) virDomainFree(dom); if (conn) virConnectClose(conn); - g_strfreev(shargv); VIR_FREE(shcmd); VIR_FREE(term); VIR_FREE(name); diff --git a/tools/vsh.c b/tools/vsh.c index ecb591fb63..e3e27a0ba6 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3365,7 +3365,8 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd) int stdin_fileno = STDIN_FILENO; const char *arg = ""; const vshCmdOpt *opt = NULL; - char **matches = NULL, **iter; + g_auto(GStrv) matches = NULL; + char **iter; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0) @@ -3406,7 +3407,6 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: - g_strfreev(matches); return ret; } -- 2.32.0
There are two instances of an explicit call to qemuMonitorCPUModelInfoFree() which in fact can be turned into g_auto(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 3 +-- tests/qemumonitorjsontest.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a12ef2227e..eac64bce70 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12395,7 +12395,7 @@ static int qemuConnectStealCPUModelFromInfo(virCPUDef *dst, qemuMonitorCPUModelInfo **src) { - qemuMonitorCPUModelInfo *info; + g_autoptr(qemuMonitorCPUModelInfo) info = NULL; size_t i; int ret = -1; @@ -12418,7 +12418,6 @@ qemuConnectStealCPUModelFromInfo(virCPUDef *dst, ret = 0; cleanup: - qemuMonitorCPUModelInfoFree(info); return ret; } diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 25a91cdeb3..bdf557656b 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2839,7 +2839,7 @@ testQemuMonitorJSONqemuMonitorJSONGetCPUModelBaseline(const void *opaque) g_autoptr(qemuMonitorTest) test = NULL; g_autoptr(virCPUDef) cpu_a = virCPUDefNew(); g_autoptr(virCPUDef) cpu_b = virCPUDefNew(); - qemuMonitorCPUModelInfo *baseline = NULL; + g_autoptr(qemuMonitorCPUModelInfo) baseline = NULL; int ret = -1; if (!(test = qemuMonitorTestNewSchema(data->xmlopt, data->schema))) @@ -2898,7 +2898,6 @@ testQemuMonitorJSONqemuMonitorJSONGetCPUModelBaseline(const void *opaque) ret = 0; cleanup: - qemuMonitorCPUModelInfoFree(baseline); return ret; } -- 2.32.0
Previous commit rendered 'cleanup' label and @ret variable redundant. The same result can be achieved by returning 0/-1 directly. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index eac64bce70..4c32d969c7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12397,7 +12397,6 @@ qemuConnectStealCPUModelFromInfo(virCPUDef *dst, { g_autoptr(qemuMonitorCPUModelInfo) info = NULL; size_t i; - int ret = -1; virCPUDefFreeModel(dst); @@ -12412,13 +12411,10 @@ qemuConnectStealCPUModelFromInfo(virCPUDef *dst, continue; if (virCPUDefAddFeature(dst, name, VIR_CPU_FEATURE_REQUIRE) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } -- 2.32.0
Instead of calling qemuMonitorTestFree() explicitly, we can use g_autoptr() and let it be called automagically. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuagenttest.c | 46 ++++++++++++------------------------ tests/qemucapabilitiestest.c | 3 +-- tests/qemuhotplugtest.c | 3 +-- tests/qemumigparamstest.c | 6 ++--- tests/qemumonitorjsontest.c | 23 +++++++++--------- tests/qemumonitortestutils.c | 25 ++++++++------------ 6 files changed, 41 insertions(+), 65 deletions(-) diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index cebeb5733e..402c290056 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -109,7 +109,7 @@ static int testQemuAgentFSFreeze(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); const char *mountpoints[] = {"/fs1", "/fs2", "/fs3", "/fs4", "/fs5"}; int ret = -1; @@ -152,7 +152,6 @@ testQemuAgentFSFreeze(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); return ret; } @@ -161,7 +160,7 @@ static int testQemuAgentFSThaw(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); int ret = -1; if (!test) @@ -202,7 +201,6 @@ testQemuAgentFSThaw(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); return ret; } @@ -211,7 +209,7 @@ static int testQemuAgentFSTrim(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); int ret = -1; if (!test) @@ -232,7 +230,6 @@ testQemuAgentFSTrim(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); return ret; } @@ -244,7 +241,7 @@ testQemuAgentGetFSInfoCommon(virDomainXMLOption *xmlopt, { int ret = -1; g_autofree char *domain_filename = NULL; - qemuMonitorTest *ret_test = NULL; + g_autoptr(qemuMonitorTest) ret_test = NULL; g_autoptr(virDomainDef) ret_def = NULL; if (!test || !def) @@ -303,8 +300,6 @@ testQemuAgentGetFSInfoCommon(virDomainXMLOption *xmlopt, ret = 0; cleanup: - if (ret_test) - qemuMonitorTestFree(ret_test); return ret; } @@ -312,7 +307,7 @@ static int testQemuAgentGetFSInfo(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = NULL; + g_autoptr(qemuMonitorTest) test = NULL; g_autoptr(virDomainDef) def = NULL; qemuAgentFSInfo **info = NULL; int ret = -1, ninfo = 0, i; @@ -405,7 +400,6 @@ testQemuAgentGetFSInfo(const void *data) for (i = 0; i < ninfo; i++) qemuAgentFSInfoFree(info[i]); VIR_FREE(info); - qemuMonitorTestFree(test); return ret; } @@ -413,7 +407,7 @@ static int testQemuAgentSuspend(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); int ret = -1; size_t i; @@ -450,7 +444,6 @@ testQemuAgentSuspend(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); return ret; } @@ -511,7 +504,7 @@ static int testQemuAgentShutdown(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); struct qemuAgentShutdownTestData priv; int ret = -1; @@ -590,7 +583,6 @@ testQemuAgentShutdown(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); return ret; } @@ -631,7 +623,7 @@ static int testQemuAgentCPU(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); qemuAgentCPUInfo *cpuinfo = NULL; int nvcpus; int ret = -1; @@ -703,7 +695,6 @@ testQemuAgentCPU(const void *data) cleanup: VIR_FREE(cpuinfo); - qemuMonitorTestFree(test); return ret; } @@ -715,7 +706,7 @@ static int testQemuAgentArbitraryCommand(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); int ret = -1; g_autofree char *reply = NULL; @@ -746,7 +737,6 @@ testQemuAgentArbitraryCommand(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); return ret; } @@ -764,7 +754,7 @@ static int testQemuAgentTimeout(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); g_autofree char *reply = NULL; int ret = -1; @@ -809,7 +799,6 @@ testQemuAgentTimeout(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); return ret; } @@ -890,7 +879,7 @@ static int testQemuAgentGetInterfaces(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); size_t i; int ret = -1; int ifaces_count = 0; @@ -991,7 +980,6 @@ testQemuAgentGetInterfaces(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); if (ifaces) { for (i = 0; i < ifaces_count; i++) virDomainInterfaceFree(ifaces[i]); @@ -1039,7 +1027,7 @@ static int testQemuAgentGetDisks(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); size_t i; int ret = -1; int disks_count = 0; @@ -1100,7 +1088,6 @@ testQemuAgentGetDisks(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); if (disks) { for (i = 0; i < disks_count; i++) qemuAgentDiskInfoFree(disks[i]); @@ -1189,7 +1176,7 @@ static int testQemuAgentUsers(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); virTypedParameterPtr params = NULL; int nparams = 0; int maxparams = 0; @@ -1255,7 +1242,6 @@ testQemuAgentUsers(const void *data) cleanup: virTypedParamsFree(params, nparams); - qemuMonitorTestFree(test); return ret; } @@ -1289,7 +1275,7 @@ static int testQemuAgentOSInfo(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); virTypedParameterPtr params = NULL; int nparams = 0; int maxparams = 0; @@ -1375,7 +1361,6 @@ testQemuAgentOSInfo(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); virTypedParamsFree(params, nparams); return ret; } @@ -1393,7 +1378,7 @@ static int testQemuAgentTimezone(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; - qemuMonitorTest *test = qemuMonitorTestNewAgent(xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); virTypedParameterPtr params = NULL; int nparams = 0; int ret = -1; @@ -1455,7 +1440,6 @@ testQemuAgentTimezone(const void *data) ret = 0; cleanup: - qemuMonitorTestFree(test); virTypedParamsFree(params, nparams); return ret; } diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index b495bfc07d..b866818e0a 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -72,7 +72,7 @@ testQemuCaps(const void *opaque) testQemuData *data = (void *) opaque; g_autofree char *repliesFile = NULL; g_autofree char *capsFile = NULL; - qemuMonitorTest *mon = NULL; + g_autoptr(qemuMonitorTest) mon = NULL; g_autoptr(virQEMUCaps) capsActual = NULL; g_autofree char *binary = NULL; g_autofree char *actual = NULL; @@ -132,7 +132,6 @@ testQemuCaps(const void *opaque) ret = 0; cleanup: - qemuMonitorTestFree(mon); return ret; } diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index ec448da09e..ac2cf9bb65 100644 --- a/tests/qemuhotplugtest.c +++ b/tests/qemuhotplugtest.c @@ -255,7 +255,7 @@ testQemuHotplug(const void *data) virDomainObj *vm = NULL; virDomainDeviceDef *dev = NULL; g_autoptr(virCaps) caps = NULL; - qemuMonitorTest *test_mon = NULL; + g_autoptr(qemuMonitorTest) test_mon = NULL; qemuDomainObjPrivate *priv = NULL; domain_filename = g_strdup_printf("%s/qemuhotplugtestdomains/qemuhotplug-%s.xml", @@ -359,7 +359,6 @@ testQemuHotplug(const void *data) test->vm = NULL; } virDomainDeviceDefFree(dev); - qemuMonitorTestFree(test_mon); return ((ret < 0 && fail) || (!ret && !fail)) ? 0 : -1; } diff --git a/tests/qemumigparamstest.c b/tests/qemumigparamstest.c index 4ab40d9d2e..7d677e2b3a 100644 --- a/tests/qemumigparamstest.c +++ b/tests/qemumigparamstest.c @@ -96,7 +96,7 @@ qemuMigParamsTestXML(const void *opaque) g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_autofree char *replyFile = NULL; g_autofree char *xmlFile = NULL; - qemuMonitorTest *mon = NULL; + g_autoptr(qemuMonitorTest) mon = NULL; g_autoptr(virJSONValue) params = NULL; g_autoptr(qemuMigrationParams) migParams = NULL; g_autofree char *actualXML = NULL; @@ -128,7 +128,6 @@ qemuMigParamsTestXML(const void *opaque) ret = 0; cleanup: - qemuMonitorTestFree(mon); return ret; } @@ -139,7 +138,7 @@ qemuMigParamsTestJSON(const void *opaque) const qemuMigParamsData *data = opaque; g_autofree char *replyFile = NULL; g_autofree char *jsonFile = NULL; - qemuMonitorTest *mon = NULL; + g_autoptr(qemuMonitorTest) mon = NULL; g_autoptr(virJSONValue) paramsIn = NULL; g_autoptr(virJSONValue) paramsOut = NULL; g_autoptr(qemuMigrationParams) migParams = NULL; @@ -184,7 +183,6 @@ qemuMigParamsTestJSON(const void *opaque) ret = 0; cleanup: - qemuMonitorTestFree(mon); return ret; } diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index bdf557656b..e51d6768d5 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -650,10 +650,14 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOption *xmlopt, { struct qemuMonitorJSONTestAttachChardevData data = {0}; + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewSchema(xmlopt, schema); g_autofree char *jsonreply = NULL; g_autofree char *fulllabel = NULL; int ret = -1; + if (!test) + goto cleanup; + if (!reply) reply = ""; @@ -661,17 +665,16 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOption *xmlopt, fulllabel = g_strdup_printf("qemuMonitorJSONTestAttachChardev(%s)", label); + qemuMonitorTestAllowUnusedCommands(test); + + if (qemuMonitorTestAddItemExpect(test, "chardev-add", + expectargs, true, jsonreply) < 0) + goto cleanup; + data.chr = chr; data.fail = fail; data.expectPty = expectPty; - if (!(data.test = qemuMonitorTestNewSchema(xmlopt, schema))) - goto cleanup; - - qemuMonitorTestAllowUnusedCommands(data.test); - - if (qemuMonitorTestAddItemExpect(data.test, "chardev-add", - expectargs, true, jsonreply) < 0) - goto cleanup; + data.test = test; if (virTestRun(fulllabel, &testQemuMonitorJSONAttachChardev, &data) < 0) goto cleanup; @@ -679,7 +682,6 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOption *xmlopt, ret = 0; cleanup: - qemuMonitorTestFree(data.test); return ret; } @@ -2656,7 +2658,7 @@ static int testQueryJobs(const void *opaque) { const struct testQueryJobsData *data = opaque; - qemuMonitorTest *test = qemuMonitorTestNewSimple(data->xmlopt); + g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewSimple(data->xmlopt); g_autofree char *filenameJSON = NULL; g_autofree char *fileJSON = NULL; g_autofree char *filenameResult = NULL; @@ -2701,7 +2703,6 @@ testQueryJobs(const void *opaque) for (i = 0; i < njobs; i++) qemuMonitorJobInfoFree(jobs[i]); VIR_FREE(jobs); - qemuMonitorTestFree(test); return ret; } diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 75a6a76b92..2c63e95bda 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -996,7 +996,7 @@ qemuMonitorCommonTestNew(virDomainXMLOption *xmlopt, virDomainObj *vm, virDomainChrSourceDef *src) { - qemuMonitorTest *test = NULL; + g_autoptr(qemuMonitorTest) test = NULL; char *path = NULL; char *tmpdir_template = NULL; @@ -1044,12 +1044,11 @@ qemuMonitorCommonTestNew(virDomainXMLOption *xmlopt, if (virNetSocketListen(test->server, 1) < 0) goto error; - return test; + return g_steal_pointer(&test); error: VIR_FREE(path); VIR_FREE(tmpdir_template); - qemuMonitorTestFree(test); return NULL; } @@ -1116,7 +1115,7 @@ qemuMonitorTestNew(virDomainXMLOption *xmlopt, const char *greeting, GHashTable *schema) { - qemuMonitorTest *test = NULL; + g_autoptr(qemuMonitorTest) test = NULL; virDomainChrSourceDef src; memset(&src, 0, sizeof(src)); @@ -1150,11 +1149,10 @@ qemuMonitorTestNew(virDomainXMLOption *xmlopt, virDomainChrSourceDefClear(&src); - return test; + return g_steal_pointer(&test); error: virDomainChrSourceDefClear(&src); - qemuMonitorTestFree(test); return NULL; } @@ -1177,7 +1175,7 @@ qemuMonitorTestNewFromFile(const char *fileName, virDomainXMLOption *xmlopt, bool simple) { - qemuMonitorTest *test = NULL; + g_autoptr(qemuMonitorTest) test = NULL; g_autofree char *json = NULL; char *tmp; char *singleReply; @@ -1226,10 +1224,9 @@ qemuMonitorTestNewFromFile(const char *fileName, if (test && qemuMonitorTestAddItem(test, NULL, singleReply) < 0) goto error; - return test; + return g_steal_pointer(&test); error: - qemuMonitorTestFree(test); return NULL; } @@ -1311,7 +1308,7 @@ qemuMonitorTestNewFromFileFull(const char *fileName, virDomainObj *vm, GHashTable *qmpschema) { - qemuMonitorTest *ret = NULL; + g_autoptr(qemuMonitorTest) ret = NULL; g_autofree char *jsonstr = NULL; char *tmp; size_t line = 0; @@ -1377,10 +1374,9 @@ qemuMonitorTestNewFromFileFull(const char *fileName, goto error; } - return ret; + return g_steal_pointer(&ret); error: - qemuMonitorTestFree(ret); return NULL; } @@ -1388,7 +1384,7 @@ qemuMonitorTestNewFromFileFull(const char *fileName, qemuMonitorTest * qemuMonitorTestNewAgent(virDomainXMLOption *xmlopt) { - qemuMonitorTest *test = NULL; + g_autoptr(qemuMonitorTest) test = NULL; virDomainChrSourceDef src; memset(&src, 0, sizeof(src)); @@ -1413,11 +1409,10 @@ qemuMonitorTestNewAgent(virDomainXMLOption *xmlopt) virDomainChrSourceDefClear(&src); - return test; + return g_steal_pointer(&test); error: virDomainChrSourceDefClear(&src); - qemuMonitorTestFree(test); return NULL; } -- 2.32.0
This commit doesn't aim to extinguish every VIR_FREE() call, but only those which were touched by the previous commit. The aim is to drop cleanup/error labels. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuagenttest.c | 3 +-- tests/qemumonitorjsontest.c | 3 +-- tests/qemumonitortestutils.c | 6 ++---- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 402c290056..479f4c0503 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -624,7 +624,7 @@ testQemuAgentCPU(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); - qemuAgentCPUInfo *cpuinfo = NULL; + g_autofree qemuAgentCPUInfo *cpuinfo = NULL; int nvcpus; int ret = -1; @@ -694,7 +694,6 @@ testQemuAgentCPU(const void *data) ret = 0; cleanup: - VIR_FREE(cpuinfo); return ret; } diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index e51d6768d5..e4bc656308 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -247,7 +247,7 @@ testQemuMonitorJSONGetVersion(const void *opaque) int major; int minor; int micro; - char *package = NULL; + g_autofree char *package = NULL; g_autoptr(qemuMonitorTest) test = NULL; if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) @@ -337,7 +337,6 @@ testQemuMonitorJSONGetVersion(const void *opaque) ret = 0; cleanup: - VIR_FREE(package); return ret; } diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 2c63e95bda..2ca17f7f48 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -997,8 +997,8 @@ qemuMonitorCommonTestNew(virDomainXMLOption *xmlopt, virDomainChrSourceDef *src) { g_autoptr(qemuMonitorTest) test = NULL; - char *path = NULL; - char *tmpdir_template = NULL; + g_autofree char *path = NULL; + g_autofree char *tmpdir_template = NULL; test = g_new0(qemuMonitorTest, 1); @@ -1047,8 +1047,6 @@ qemuMonitorCommonTestNew(virDomainXMLOption *xmlopt, return g_steal_pointer(&test); error: - VIR_FREE(path); - VIR_FREE(tmpdir_template); return NULL; } -- 2.32.0
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuagenttest.c | 216 +++++++++++++++-------------------- tests/qemucapabilitiestest.c | 19 ++- tests/qemumigparamstest.c | 34 +++--- tests/qemumonitorjsontest.c | 67 +++++------ tests/qemumonitortestutils.c | 32 ++---- 5 files changed, 149 insertions(+), 219 deletions(-) diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 479f4c0503..6a5aa3edd2 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -111,48 +111,45 @@ testQemuAgentFSFreeze(const void *data) virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); const char *mountpoints[] = {"/fs1", "/fs2", "/fs3", "/fs4", "/fs5"}; - int ret = -1; + int rc; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-fsfreeze-freeze-list", "{ \"return\" : 5 }") < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-fsfreeze-freeze", "{ \"return\" : 7 }") < 0) - goto cleanup; + return -1; - if ((ret = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test), - mountpoints, 5)) < 0) - goto cleanup; + if ((rc = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test), + mountpoints, 5)) < 0) + return -1; - if (ret != 5) { + if (rc != 5) { virReportError(VIR_ERR_INTERNAL_ERROR, - "expected 5 frozen filesystems, got %d", ret); - goto cleanup; + "expected 5 frozen filesystems, got %d", rc); + return -1; } - if ((ret = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test), NULL, 0)) < 0) - goto cleanup; + if ((rc = qemuAgentFSFreeze(qemuMonitorTestGetAgent(test), NULL, 0)) < 0) + return -1; - if (ret != 7) { + if (rc != 7) { virReportError(VIR_ERR_INTERNAL_ERROR, - "expected 7 frozen filesystems, got %d", ret); - goto cleanup; + "expected 7 frozen filesystems, got %d", rc); + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -161,47 +158,44 @@ testQemuAgentFSThaw(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); - int ret = -1; + int rc; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-fsfreeze-thaw", "{ \"return\" : 5 }") < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-fsfreeze-thaw", "{ \"return\" : 7 }") < 0) - goto cleanup; + return -1; - if ((ret = qemuAgentFSThaw(qemuMonitorTestGetAgent(test))) < 0) - goto cleanup; + if ((rc = qemuAgentFSThaw(qemuMonitorTestGetAgent(test))) < 0) + return -1; - if (ret != 5) { + if (rc != 5) { virReportError(VIR_ERR_INTERNAL_ERROR, - "expected 5 thawed filesystems, got %d", ret); - goto cleanup; + "expected 5 thawed filesystems, got %d", rc); + return -1; } - if ((ret = qemuAgentFSThaw(qemuMonitorTestGetAgent(test))) < 0) - goto cleanup; + if ((rc = qemuAgentFSThaw(qemuMonitorTestGetAgent(test))) < 0) + return -1; - if (ret != 7) { + if (rc != 7) { virReportError(VIR_ERR_INTERNAL_ERROR, - "expected 7 thawed filesystems, got %d", ret); - goto cleanup; + "expected 7 thawed filesystems, got %d", rc); + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -210,27 +204,23 @@ testQemuAgentFSTrim(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); - int ret = -1; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItemParams(test, "guest-fstrim", "{ \"return\" : {} }", "minimum", "1337", NULL) < 0) - goto cleanup; + return -1; if (qemuAgentFSTrim(qemuMonitorTestGetAgent(test), 1337) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - return ret; + return 0; } @@ -239,7 +229,6 @@ testQemuAgentGetFSInfoCommon(virDomainXMLOption *xmlopt, qemuMonitorTest **test, virDomainDef **def) { - int ret = -1; g_autofree char *domain_filename = NULL; g_autoptr(qemuMonitorTest) ret_test = NULL; g_autoptr(virDomainDef) ret_def = NULL; @@ -254,10 +243,10 @@ testQemuAgentGetFSInfoCommon(virDomainXMLOption *xmlopt, if (!(ret_def = virDomainDefParseFile(domain_filename, xmlopt, NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE))) - goto cleanup; + return -1; if (qemuMonitorTestAddAgentSyncResponse(ret_test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(ret_test, "guest-get-fsinfo", "{\"return\": [" @@ -293,14 +282,11 @@ testQemuAgentGetFSInfoCommon(virDomainXMLOption *xmlopt, " {\"name\": \"sdb1\"," " \"mountpoint\": \"/mnt/disk\"," " \"disk\": [], \"type\": \"xfs\"}]}") < 0) - goto cleanup; + return -1; *test = g_steal_pointer(&ret_test); *def = g_steal_pointer(&ret_def); - ret = 0; - - cleanup: - return ret; + return 0; } static int @@ -408,43 +394,39 @@ testQemuAgentSuspend(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); - int ret = -1; size_t i; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-suspend-ram", "{ \"return\" : {} }") < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-suspend-disk", "{ \"return\" : {} }") < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-suspend-hybrid", "{ \"return\" : {} }") < 0) - goto cleanup; + return -1; /* try the commands - fail if ordering changes */ for (i = 0; i < VIR_NODE_SUSPEND_TARGET_LAST; i++) { if (qemuAgentSuspend(qemuMonitorTestGetAgent(test), i) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -506,13 +488,12 @@ testQemuAgentShutdown(const void *data) virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); struct qemuAgentShutdownTestData priv; - int ret = -1; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; priv.event = QEMU_AGENT_EVENT_SHUTDOWN; priv.mode = "halt"; @@ -520,14 +501,14 @@ testQemuAgentShutdown(const void *data) if (qemuMonitorTestAddHandler(test, "guest-shutdown", qemuAgentShutdownTestMonitorHandler, &priv, NULL) < 0) - goto cleanup; + return -1; if (qemuAgentShutdown(qemuMonitorTestGetAgent(test), QEMU_AGENT_SHUTDOWN_HALT) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; priv.event = QEMU_AGENT_EVENT_SHUTDOWN; priv.mode = "powerdown"; @@ -535,14 +516,14 @@ testQemuAgentShutdown(const void *data) if (qemuMonitorTestAddHandler(test, "guest-shutdown", qemuAgentShutdownTestMonitorHandler, &priv, NULL) < 0) - goto cleanup; + return -1; if (qemuAgentShutdown(qemuMonitorTestGetAgent(test), QEMU_AGENT_SHUTDOWN_POWERDOWN) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; priv.event = QEMU_AGENT_EVENT_RESET; priv.mode = "reboot"; @@ -551,17 +532,17 @@ testQemuAgentShutdown(const void *data) "guest-shutdown", qemuAgentShutdownTestMonitorHandler, &priv, NULL) < 0) - goto cleanup; + return -1; if (qemuAgentShutdown(qemuMonitorTestGetAgent(test), QEMU_AGENT_SHUTDOWN_REBOOT) < 0) - goto cleanup; + return -1; /* check negative response, so that we can verify that the agent breaks * out from sleep */ if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-shutdown", "{\"error\":" @@ -571,19 +552,16 @@ testQemuAgentShutdown(const void *data) " \"data\":{\"name\":\"guest-shutdown\"}" " }" "}") < 0) - goto cleanup; + return -1; if (qemuAgentShutdown(qemuMonitorTestGetAgent(test), QEMU_AGENT_SHUTDOWN_REBOOT) != -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "agent shutdown command should have failed"); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -626,75 +604,71 @@ testQemuAgentCPU(const void *data) g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); g_autofree qemuAgentCPUInfo *cpuinfo = NULL; int nvcpus; - int ret = -1; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "guest-get-vcpus", testQemuAgentCPUResponse) < 0) - goto cleanup; + return -1; /* get cpus */ if ((nvcpus = qemuAgentGetVCPUs(qemuMonitorTestGetAgent(test), &cpuinfo)) < 0) - goto cleanup; + return -1; if (nvcpus != 4) { virReportError(VIR_ERR_INTERNAL_ERROR, "Expected '4' cpus, got '%d'", nvcpus); - goto cleanup; + return -1; } /* try to unplug one */ if (qemuAgentUpdateCPUInfo(2, cpuinfo, nvcpus) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItemParams(test, "guest-set-vcpus", "{ \"return\" : 1 }", "vcpus", testQemuAgentCPUArguments1, NULL) < 0) - goto cleanup; + return -1; if (qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), cpuinfo, nvcpus) < 0) - goto cleanup; + return -1; /* try to hotplug two, second one will fail */ if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItemParams(test, "guest-set-vcpus", "{ \"return\" : 1 }", "vcpus", testQemuAgentCPUArguments2, NULL) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItemParams(test, "guest-set-vcpus", "{ \"error\" : \"random error\" }", "vcpus", testQemuAgentCPUArguments3, NULL) < 0) - goto cleanup; + return -1; if (qemuAgentUpdateCPUInfo(4, cpuinfo, nvcpus) < 0) - goto cleanup; + return -1; /* this should fail */ if (qemuAgentSetVCPUs(qemuMonitorTestGetAgent(test), cpuinfo, nvcpus) != -1) - goto cleanup; + return -1; - ret = 0; - - cleanup: - return ret; + return 0; } @@ -706,37 +680,33 @@ testQemuAgentArbitraryCommand(const void *data) { virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); - int ret = -1; g_autofree char *reply = NULL; if (!test) return -1; if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "ble", testQemuAgentArbitraryCommandResponse) < 0) - goto cleanup; + return -1; if (qemuAgentArbitraryCommand(qemuMonitorTestGetAgent(test), "{\"execute\":\"ble\"}", &reply, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) - goto cleanup; + return -1; if (STRNEQ(reply, testQemuAgentArbitraryCommandResponse)) { virReportError(VIR_ERR_INTERNAL_ERROR, "invalid processing of guest agent reply: " "got '%s' expected '%s'", reply, testQemuAgentArbitraryCommandResponse); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } @@ -755,36 +725,33 @@ testQemuAgentTimeout(const void *data) virDomainXMLOption *xmlopt = (virDomainXMLOption *)data; g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewAgent(xmlopt); g_autofree char *reply = NULL; - int ret = -1; if (!test) return -1; - if (virTestGetExpensive() == 0) { - ret = EXIT_AM_SKIP; - goto cleanup; - } + if (virTestGetExpensive() == 0) + return EXIT_AM_SKIP; if (qemuMonitorTestAddHandler(test, NULL, qemuAgentTimeoutTestMonitorHandler, NULL, NULL) < 0) - goto cleanup; + return -1; if (qemuAgentFSFreeze(qemuMonitorTestGetAgent(test), NULL, 0) != -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "agent command should have failed"); - goto cleanup; + return -1; } /* test timeout */ if (qemuMonitorTestAddAgentSyncResponse(test) < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddHandler(test, NULL, qemuAgentTimeoutTestMonitorHandler, NULL, NULL) < 0) - goto cleanup; + return -1; if (qemuAgentArbitraryCommand(qemuMonitorTestGetAgent(test), "{\"execute\":\"ble\"}", @@ -792,13 +759,10 @@ testQemuAgentTimeout(const void *data) 1) != -2) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "agent command didn't time out"); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } static const char testQemuAgentGetInterfacesResponse[] = diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index b866818e0a..99534ab9a1 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -68,7 +68,6 @@ testQemuDataReset(testQemuData *data) static int testQemuCaps(const void *opaque) { - int ret = -1; testQemuData *data = (void *) opaque; g_autofree char *repliesFile = NULL; g_autofree char *capsFile = NULL; @@ -88,10 +87,10 @@ testQemuCaps(const void *opaque) if (!(mon = qemuMonitorTestNewFromFileFull(repliesFile, &data->driver, NULL, NULL))) - goto cleanup; + return -1; if (qemuProcessQMPInitMonitor(qemuMonitorTestGetMonitor(mon)) < 0) - goto cleanup; + return -1; binary = g_strdup_printf("/usr/bin/qemu-system-%s", data->archName); @@ -99,17 +98,17 @@ testQemuCaps(const void *opaque) if (!(capsActual = virQEMUCapsNewBinary(binary)) || virQEMUCapsInitQMPMonitor(capsActual, qemuMonitorTestGetMonitor(mon)) < 0) - goto cleanup; + return -1; if (virQEMUCapsGet(capsActual, QEMU_CAPS_KVM)) { qemuMonitorResetCommandID(qemuMonitorTestGetMonitor(mon)); if (qemuProcessQMPInitMonitor(qemuMonitorTestGetMonitor(mon)) < 0) - goto cleanup; + return -1; if (virQEMUCapsInitQMPMonitorTCG(capsActual, qemuMonitorTestGetMonitor(mon)) < 0) - goto cleanup; + return -1; /* calculate fake microcode version based on filename for a reproducible * number for testing which does not change with the contents */ @@ -125,14 +124,12 @@ testQemuCaps(const void *opaque) } if (!(actual = virQEMUCapsFormatCache(capsActual))) - goto cleanup; + return -1; if (virTestCompareToFile(actual, capsFile) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - return ret; + return 0; } diff --git a/tests/qemumigparamstest.c b/tests/qemumigparamstest.c index 7d677e2b3a..ae59ff6757 100644 --- a/tests/qemumigparamstest.c +++ b/tests/qemumigparamstest.c @@ -100,7 +100,6 @@ qemuMigParamsTestXML(const void *opaque) g_autoptr(virJSONValue) params = NULL; g_autoptr(qemuMigrationParams) migParams = NULL; g_autofree char *actualXML = NULL; - int ret = -1; replyFile = g_strdup_printf("%s/qemumigparamsdata/%s.reply", abs_srcdir, data->name); @@ -108,27 +107,24 @@ qemuMigParamsTestXML(const void *opaque) abs_srcdir, data->name); if (!(mon = qemuMonitorTestNewFromFile(replyFile, data->xmlopt, true))) - goto cleanup; + return -1; if (qemuMonitorGetMigrationParams(qemuMonitorTestGetMonitor(mon), ¶ms) < 0) - goto cleanup; + return -1; if (!(migParams = qemuMigrationParamsFromJSON(params))) - goto cleanup; + return -1; qemuMigParamsTestFormatXML(&buf, migParams); if (!(actualXML = virBufferContentAndReset(&buf))) - goto cleanup; + return -1; if (virTestCompareToFile(actualXML, xmlFile) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - return ret; + return 0; } @@ -144,7 +140,6 @@ qemuMigParamsTestJSON(const void *opaque) g_autoptr(qemuMigrationParams) migParams = NULL; g_autofree char *actualJSON = NULL; g_auto(virBuffer) debug = VIR_BUFFER_INITIALIZER; - int ret = -1; replyFile = g_strdup_printf("%s/qemumigparamsdata/%s.reply", abs_srcdir, data->name); @@ -152,18 +147,18 @@ qemuMigParamsTestJSON(const void *opaque) abs_srcdir, data->name); if (!(mon = qemuMonitorTestNewFromFile(replyFile, data->xmlopt, true))) - goto cleanup; + return -1; if (qemuMonitorGetMigrationParams(qemuMonitorTestGetMonitor(mon), ¶msIn) < 0) - goto cleanup; + return -1; if (!(migParams = qemuMigrationParamsFromJSON(paramsIn))) - goto cleanup; + return -1; if (!(paramsOut = qemuMigrationParamsToJSON(migParams)) || !(actualJSON = virJSONValueToString(paramsOut, true))) - goto cleanup; + return -1; if (testQEMUSchemaValidateCommand("migrate-set-parameters", paramsOut, @@ -174,16 +169,13 @@ qemuMigParamsTestJSON(const void *opaque) &debug) < 0) { VIR_TEST_VERBOSE("failed to validate migration params '%s' against QMP schema: %s", actualJSON, virBufferCurrentContent(&debug)); - goto cleanup; + return -1; } if (virTestCompareToFile(actualJSON, jsonFile) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - return ret; + return 0; } diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index e4bc656308..8e552a4b20 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -243,7 +243,6 @@ testQemuMonitorJSONGetVersion(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - int ret = -1; int major; int minor; int micro; @@ -264,7 +263,7 @@ testQemuMonitorJSONGetVersion(const void *opaque) " \"package\":\"\"" " }" "}") < 0) - goto cleanup; + return -1; if (qemuMonitorTestAddItem(test, "query-version", "{ " @@ -277,67 +276,64 @@ testQemuMonitorJSONGetVersion(const void *opaque) " \"package\":\"2.283.el6\"" " }" "}") < 0) - goto cleanup; + return -1; if (qemuMonitorGetVersion(qemuMonitorTestGetMonitor(test), &major, &minor, µ, &package) < 0) - goto cleanup; + return -1; if (major != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "Major %d was not 1", major); - goto cleanup; + return -1; } if (minor != 2) { virReportError(VIR_ERR_INTERNAL_ERROR, "Minor %d was not 2", major); - goto cleanup; + return -1; } if (micro != 3) { virReportError(VIR_ERR_INTERNAL_ERROR, "Micro %d was not 3", major); - goto cleanup; + return -1; } if (STRNEQ(package, "")) { virReportError(VIR_ERR_INTERNAL_ERROR, "Package %s was not ''", package); - goto cleanup; + return -1; } VIR_FREE(package); if (qemuMonitorGetVersion(qemuMonitorTestGetMonitor(test), &major, &minor, µ, &package) < 0) - goto cleanup; + return -1; if (major != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "Major %d was not 0", major); - goto cleanup; + return -1; } if (minor != 11) { virReportError(VIR_ERR_INTERNAL_ERROR, "Minor %d was not 11", major); - goto cleanup; + return -1; } if (micro != 6) { virReportError(VIR_ERR_INTERNAL_ERROR, "Micro %d was not 6", major); - goto cleanup; + return -1; } if (STRNEQ(package, "2.283.el6")) { virReportError(VIR_ERR_INTERNAL_ERROR, "Package %s was not '2.283.el6'", package); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } static int @@ -652,10 +648,9 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOption *xmlopt, g_autoptr(qemuMonitorTest) test = qemuMonitorTestNewSchema(xmlopt, schema); g_autofree char *jsonreply = NULL; g_autofree char *fulllabel = NULL; - int ret = -1; if (!test) - goto cleanup; + return -1; if (!reply) reply = ""; @@ -668,7 +663,7 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOption *xmlopt, if (qemuMonitorTestAddItemExpect(test, "chardev-add", expectargs, true, jsonreply) < 0) - goto cleanup; + return -1; data.chr = chr; data.fail = fail; @@ -676,12 +671,9 @@ qemuMonitorJSONTestAttachOneChardev(virDomainXMLOption *xmlopt, data.test = test; if (virTestRun(fulllabel, &testQemuMonitorJSONAttachChardev, &data) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - return ret; + return 0; } static int @@ -1028,15 +1020,15 @@ testQemuMonitorJSONGetDeviceAliases(const void *opaque) " \"type\": \"child<piix3-usb-uhci>\"}," " {\"name\": \"type\", \"type\": \"string\"}" "]}") < 0) - goto cleanup; + return -1; if (qemuMonitorJSONGetDeviceAliases(qemuMonitorTestGetMonitor(test), &aliases) < 0) - goto cleanup; + return -1; if (!aliases) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "no aliases returned"); - goto cleanup; + return -1; } ret = 0; @@ -1053,7 +1045,6 @@ testQemuMonitorJSONGetDeviceAliases(const void *opaque) } } - cleanup: return ret; } @@ -2840,7 +2831,6 @@ testQemuMonitorJSONqemuMonitorJSONGetCPUModelBaseline(const void *opaque) g_autoptr(virCPUDef) cpu_a = virCPUDefNew(); g_autoptr(virCPUDef) cpu_b = virCPUDefNew(); g_autoptr(qemuMonitorCPUModelInfo) baseline = NULL; - int ret = -1; if (!(test = qemuMonitorTestNewSchema(data->xmlopt, data->schema))) return -1; @@ -2865,26 +2855,26 @@ testQemuMonitorJSONqemuMonitorJSONGetCPUModelBaseline(const void *opaque) if (virCPUDefAddFeature(cpu_a, "feat_a", VIR_CPU_FEATURE_REQUIRE) < 0 || virCPUDefAddFeature(cpu_a, "feat_b", VIR_CPU_FEATURE_REQUIRE) < 0 || virCPUDefAddFeature(cpu_a, "feat_c", VIR_CPU_FEATURE_REQUIRE) < 0) - goto cleanup; + return -1; if (qemuMonitorJSONGetCPUModelBaseline(qemuMonitorTestGetMonitor(test), cpu_a, cpu_b, &baseline) < 0) - goto cleanup; + return -1; if (!baseline) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Baseline missing result"); - goto cleanup; + return -1; } if (!baseline->name) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Baseline missing model name"); - goto cleanup; + return -1; } if (baseline->nprops != 2) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Baseline missing properties"); - goto cleanup; + return -1; } if (STRNEQ(baseline->props[0].name, "feat_a") || !baseline->props[0].value.boolean || @@ -2892,13 +2882,10 @@ testQemuMonitorJSONqemuMonitorJSONGetCPUModelBaseline(const void *opaque) baseline->props[1].value.boolean) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Baseline property error"); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - return ret; + return 0; } diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index 2ca17f7f48..0d99b45909 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -1014,7 +1014,7 @@ qemuMonitorCommonTestNew(virDomainXMLOption *xmlopt, if (!(test->tmpdir = g_mkdtemp(tmpdir_template))) { virReportSystemError(errno, "%s", "Failed to create temporary directory"); - goto error; + return NULL; } tmpdir_template = NULL; @@ -1026,14 +1026,14 @@ qemuMonitorCommonTestNew(virDomainXMLOption *xmlopt, } else { test->vm = virDomainObjNew(xmlopt); if (!test->vm) - goto error; + return NULL; if (!(test->vm->def = virDomainDefNew(xmlopt))) - goto error; + return NULL; } if (virNetSocketNewListenUNIX(path, 0700, geteuid(), getegid(), &test->server) < 0) - goto error; + return NULL; memset(src, 0, sizeof(*src)); src->type = VIR_DOMAIN_CHR_TYPE_UNIX; @@ -1042,13 +1042,9 @@ qemuMonitorCommonTestNew(virDomainXMLOption *xmlopt, path = NULL; if (virNetSocketListen(test->server, 1) < 0) - goto error; + return NULL; return g_steal_pointer(&test); - - error: - return NULL; - } @@ -1200,12 +1196,12 @@ qemuMonitorTestNewFromFile(const char *fileName, if (test) { if (qemuMonitorTestAddItem(test, NULL, singleReply) < 0) - goto error; + return NULL; } else { /* Create new mocked monitor with our greeting */ if (!(test = qemuMonitorTestNew(xmlopt, NULL, NULL, singleReply, NULL))) - goto error; + return NULL; } if (!eof) { @@ -1220,12 +1216,9 @@ qemuMonitorTestNewFromFile(const char *fileName, } if (test && qemuMonitorTestAddItem(test, NULL, singleReply) < 0) - goto error; + return NULL; return g_steal_pointer(&test); - - error: - return NULL; } @@ -1344,7 +1337,7 @@ qemuMonitorTestNewFromFileFull(const char *fileName, if (response) { if (qemuMonitorTestFullAddItem(ret, fileName, command, response, commandln) < 0) - goto error; + return NULL; command = NULL; response = NULL; } @@ -1364,18 +1357,15 @@ qemuMonitorTestNewFromFileFull(const char *fileName, if (!response) { virReportError(VIR_ERR_INTERNAL_ERROR, "missing response for command " "on line '%zu' in '%s'", commandln, fileName); - goto error; + return NULL; } if (qemuMonitorTestFullAddItem(ret, fileName, command, response, commandln) < 0) - goto error; + return NULL; } return g_steal_pointer(&ret); - - error: - return NULL; } -- 2.32.0
On Mon, 2021-11-01 at 15:16 +0100, Michal Privoznik wrote:
I've been looking at our tests lately and noticed an opportunity to rewrite pieces of code to g_auto() magic.
Michal Prívozník (7): qemuagenttest: Don't leak virTypedParameter on failure Prefer g_auto(GStrv) over g_strfreev() qemu: Use g_autoptr(qemuMonitorCPUModelInfo) qemuConnectStealCPUModelFromInfo: Drop needless 'cleanup' label tests: Use g_autoptr(qemuMonitorTest) test: Use g_autofree more tests: Drop cleanup/error labels
src/bhyve/bhyve_command.c | 3 +- src/bhyve/bhyve_parse_command.c | 22 +-- src/libxl/libxl_conf.c | 9 +- src/libxl/xen_common.c | 18 +- src/libxl/xen_xl.c | 17 +- src/lxc/lxc_container.c | 4 +- src/lxc/lxc_native.c | 24 +-- src/qemu/qemu_driver.c | 17 +- src/remote/remote_daemon_dispatch.c | 3 +- src/remote/remote_driver.c | 4 +- src/storage/storage_backend_rbd.c | 3 +- src/util/vircgroup.c | 3 +- src/util/vircgroupv2.c | 4 +- src/util/virfirmware.c | 6 +- src/util/viruri.c | 3 +- src/vbox/vbox_common.c | 12 +- src/vbox/vbox_snapshot_conf.c | 40 ++-- src/vbox/vbox_tmpl.c | 3 +- src/vz/vz_sdk.c | 3 +- tests/qemuagenttest.c | 286 ++++++++++++---------------- tests/qemucapabilitiestest.c | 22 +-- tests/qemuhotplugtest.c | 3 +- tests/qemumigparamstest.c | 40 ++-- tests/qemumonitorjsontest.c | 95 ++++----- tests/qemumonitortestutils.c | 63 +++--- tests/vboxsnapshotxmltest.c | 3 +- tests/virconftest.c | 3 +- tests/virfiletest.c | 3 +- tests/virstringtest.c | 3 +- tools/virsh-host.c | 13 +- tools/virt-login-shell-helper.c | 7 +- tools/vsh.c | 4 +- 32 files changed, 279 insertions(+), 464 deletions(-)
When applying this series, compiling with ASAN enabled, and running "virsh hypervisor-cpu-compare empty.xml" with "empty.xml" == "<cpu/>", I see the following error message: ================================================================= ==45506==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000009b70 at pc 0x5588d1c81aa8 bp 0x7fffc8510af0 sp 0x7fffc8510ae8 READ of size 8 at 0x602000009b70 thread T0 #0 0x5588d1c81aa7 in cmdHypervisorCPUCompare ../../git/libvirt/tools/virsh-host.c:1605 #1 0x5588d1cead5d in vshCommandRun ../../git/libvirt/tools/vsh.c:1309 #2 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 #3 0x7fc8c4f32b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) #4 0x5588d1bcef3d in _start (/home/twiederh/build/libvirt/tools/virsh+0x16bf3d) 0x602000009b70 is located 0 bytes inside of 16-byte region [0x602000009b70,0x602000009b80) freed by thread T0 here: #0 0x7fc8c9020647 in free (/lib64/libasan.so.6+0xae647) #1 0x7fc8c5b3a24c in g_free (/lib64/libglib-2.0.so.0+0x5a24c) #2 0x5588d1c7ebcb in vshExtractCPUDefXMLs ../../git/libvirt/tools/virsh-host.c:1062 #3 0x5588d1c819fe in cmdHypervisorCPUCompare ../../git/libvirt/tools/virsh-host.c:1602 #4 0x5588d1cead5d in vshCommandRun ../../git/libvirt/tools/vsh.c:1309 #5 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 #6 0x7fc8c4f32b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) previously allocated by thread T0 here: #0 0x7fc8c9020af7 in calloc (/lib64/libasan.so.6+0xaeaf7) #1 0x7fc8c5b3de60 in g_malloc0 (/lib64/libglib-2.0.so.0+0x5de60) #2 0x5588d1c819fe in cmdHypervisorCPUCompare ../../git/libvirt/tools/virsh-host.c:1602 #3 0x5588d1cead5d in vshCommandRun ../../git/libvirt/tools/vsh.c:1309 #4 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 #5 0x7fc8c4f32b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) SUMMARY: AddressSanitizer: heap-use-after-free ../../git/libvirt/tools/virsh-host.c:1605 in cmdHypervisorCPUCompare Shadow bytes around the buggy address: 0x0c047fff9310: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fa 0x0c047fff9320: fa fa fd fa fa fa fd fd fa fa fd fd fa fa fd fd 0x0c047fff9330: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fd 0x0c047fff9340: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd 0x0c047fff9350: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd =>0x0c047fff9360: fa fa fd fd fa fa fd fd fa fa fd fa fa fa[fd]fd 0x0c047fff9370: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff9390: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff93a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c047fff93b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==45506==ABORTING
On 11/9/21 3:16 PM, Tim Wiederhake wrote:
On Mon, 2021-11-01 at 15:16 +0100, Michal Privoznik wrote:
I've been looking at our tests lately and noticed an opportunity to rewrite pieces of code to g_auto() magic.
Michal Prívozník (7): qemuagenttest: Don't leak virTypedParameter on failure Prefer g_auto(GStrv) over g_strfreev() qemu: Use g_autoptr(qemuMonitorCPUModelInfo) qemuConnectStealCPUModelFromInfo: Drop needless 'cleanup' label tests: Use g_autoptr(qemuMonitorTest) test: Use g_autofree more tests: Drop cleanup/error labels
src/bhyve/bhyve_command.c | 3 +- src/bhyve/bhyve_parse_command.c | 22 +-- src/libxl/libxl_conf.c | 9 +- src/libxl/xen_common.c | 18 +- src/libxl/xen_xl.c | 17 +- src/lxc/lxc_container.c | 4 +- src/lxc/lxc_native.c | 24 +-- src/qemu/qemu_driver.c | 17 +- src/remote/remote_daemon_dispatch.c | 3 +- src/remote/remote_driver.c | 4 +- src/storage/storage_backend_rbd.c | 3 +- src/util/vircgroup.c | 3 +- src/util/vircgroupv2.c | 4 +- src/util/virfirmware.c | 6 +- src/util/viruri.c | 3 +- src/vbox/vbox_common.c | 12 +- src/vbox/vbox_snapshot_conf.c | 40 ++-- src/vbox/vbox_tmpl.c | 3 +- src/vz/vz_sdk.c | 3 +- tests/qemuagenttest.c | 286 ++++++++++++---------------- tests/qemucapabilitiestest.c | 22 +-- tests/qemuhotplugtest.c | 3 +- tests/qemumigparamstest.c | 40 ++-- tests/qemumonitorjsontest.c | 95 ++++----- tests/qemumonitortestutils.c | 63 +++--- tests/vboxsnapshotxmltest.c | 3 +- tests/virconftest.c | 3 +- tests/virfiletest.c | 3 +- tests/virstringtest.c | 3 +- tools/virsh-host.c | 13 +- tools/virt-login-shell-helper.c | 7 +- tools/vsh.c | 4 +- 32 files changed, 279 insertions(+), 464 deletions(-)
When applying this series, compiling with ASAN enabled, and running "virsh hypervisor-cpu-compare empty.xml" with "empty.xml" == "<cpu/>", I see the following error message:
================================================================= ==45506==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000009b70 at pc 0x5588d1c81aa8 bp 0x7fffc8510af0 sp 0x7fffc8510ae8 READ of size 8 at 0x602000009b70 thread T0 #0 0x5588d1c81aa7 in cmdHypervisorCPUCompare ../../git/libvirt/tools/virsh-host.c:1605 #1 0x5588d1cead5d in vshCommandRun ../../git/libvirt/tools/vsh.c:1309 #2 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 #3 0x7fc8c4f32b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) #4 0x5588d1bcef3d in _start (/home/twiederh/build/libvirt/tools/virsh+0x16bf3d)
Ah thanks, it's a problem in 2/7 where this needs to be squashed in: diff --git i/tools/virsh-host.c w/tools/virsh-host.c index f6aa532b40..fc84415e7b 100644 --- i/tools/virsh-host.c +++ w/tools/virsh-host.c @@ -1123,7 +1123,7 @@ vshExtractCPUDefXMLs(vshControl *ctl, } cleanup: - return cpus; + return g_steal_pointer(&cpus); error: goto cleanup; Michal
On Tue, 2021-11-09 at 16:24 +0100, Michal Prívozník wrote:
On 11/9/21 3:16 PM, Tim Wiederhake wrote:
On Mon, 2021-11-01 at 15:16 +0100, Michal Privoznik wrote:
I've been looking at our tests lately and noticed an opportunity to rewrite pieces of code to g_auto() magic.
Michal Prívozník (7): qemuagenttest: Don't leak virTypedParameter on failure Prefer g_auto(GStrv) over g_strfreev() qemu: Use g_autoptr(qemuMonitorCPUModelInfo) qemuConnectStealCPUModelFromInfo: Drop needless 'cleanup' label tests: Use g_autoptr(qemuMonitorTest) test: Use g_autofree more tests: Drop cleanup/error labels
src/bhyve/bhyve_command.c | 3 +- src/bhyve/bhyve_parse_command.c | 22 +-- src/libxl/libxl_conf.c | 9 +- src/libxl/xen_common.c | 18 +- src/libxl/xen_xl.c | 17 +- src/lxc/lxc_container.c | 4 +- src/lxc/lxc_native.c | 24 +-- src/qemu/qemu_driver.c | 17 +- src/remote/remote_daemon_dispatch.c | 3 +- src/remote/remote_driver.c | 4 +- src/storage/storage_backend_rbd.c | 3 +- src/util/vircgroup.c | 3 +- src/util/vircgroupv2.c | 4 +- src/util/virfirmware.c | 6 +- src/util/viruri.c | 3 +- src/vbox/vbox_common.c | 12 +- src/vbox/vbox_snapshot_conf.c | 40 ++-- src/vbox/vbox_tmpl.c | 3 +- src/vz/vz_sdk.c | 3 +- tests/qemuagenttest.c | 286 ++++++++++++---------- ------ tests/qemucapabilitiestest.c | 22 +-- tests/qemuhotplugtest.c | 3 +- tests/qemumigparamstest.c | 40 ++-- tests/qemumonitorjsontest.c | 95 ++++----- tests/qemumonitortestutils.c | 63 +++--- tests/vboxsnapshotxmltest.c | 3 +- tests/virconftest.c | 3 +- tests/virfiletest.c | 3 +- tests/virstringtest.c | 3 +- tools/virsh-host.c | 13 +- tools/virt-login-shell-helper.c | 7 +- tools/vsh.c | 4 +- 32 files changed, 279 insertions(+), 464 deletions(-)
When applying this series, compiling with ASAN enabled, and running "virsh hypervisor-cpu-compare empty.xml" with "empty.xml" == "<cpu/>", I see the following error message:
================================================================= ==45506==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000009b70 at pc 0x5588d1c81aa8 bp 0x7fffc8510af0 sp 0x7fffc8510ae8 READ of size 8 at 0x602000009b70 thread T0 #0 0x5588d1c81aa7 in cmdHypervisorCPUCompare ../../git/libvirt/tools/virsh-host.c:1605 #1 0x5588d1cead5d in vshCommandRun ../../git/libvirt/tools/vsh.c:1309 #2 0x5588d1bd5331 in main ../../git/libvirt/tools/virsh.c:899 #3 0x7fc8c4f32b74 in __libc_start_main (/lib64/libc.so.6+0x27b74) #4 0x5588d1bcef3d in _start (/home/twiederh/build/libvirt/tools/virsh+0x16bf3d)
Ah thanks, it's a problem in 2/7 where this needs to be squashed in:
diff --git i/tools/virsh-host.c w/tools/virsh-host.c index f6aa532b40..fc84415e7b 100644 --- i/tools/virsh-host.c +++ w/tools/virsh-host.c @@ -1123,7 +1123,7 @@ vshExtractCPUDefXMLs(vshControl *ctl, }
cleanup: - return cpus; + return g_steal_pointer(&cpus);
error: goto cleanup;
Michal
With that applied: Reviewed-by: Tim Wiederhake <twiederh@redhat.com>
participants (3)
-
Michal Privoznik -
Michal Prívozník
-
Tim Wiederhake