[libvirt] [PATCH 0/4] qemu_conf: use VIR_AUTOFREE in the recently created
Ján Tomko (4): virQEMUDriverConfigLoadProcessEntry: use VIR_AUTOFREE virQEMUDriverConfigLoadNVRAMEntry: use VIR_AUTOFREE virQEMUDriverConfigLoadSecurityEntry: use VIR_AUTOFREE virQEMUDriverConfigLoadSWTPMEntry: use VIR_AUTOFREE src/qemu/qemu_conf.c | 131 +++++++++++++++++++++------------------------------ 1 file changed, 55 insertions(+), 76 deletions(-) -- 2.16.4
Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros to get rid of the cleanup section. Requested-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_conf.c | 45 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 256aad2c0b..fc7101904e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -642,15 +642,14 @@ static int virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg, virConfPtr conf) { - char *stdioHandler = NULL; - char **hugetlbfs = NULL; - char *corestr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) stdioHandler = NULL; + VIR_AUTOPTR(virString) hugetlbfs = NULL; + VIR_AUTOFREE(char *) corestr = NULL; size_t i; if (virConfGetValueStringList(conf, "hugetlbfs_mount", true, &hugetlbfs) < 0) - goto cleanup; + return -1; if (hugetlbfs) { /* There already might be something autodetected. Avoid leaking it. */ while (cfg->nhugetlbfs) { @@ -662,49 +661,49 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg, cfg->nhugetlbfs = virStringListLength((const char *const *)hugetlbfs); if (hugetlbfs[0] && VIR_ALLOC_N(cfg->hugetlbfs, cfg->nhugetlbfs) < 0) - goto cleanup; + return -1; for (i = 0; hugetlbfs[i] != NULL; i++) { if (virQEMUDriverConfigHugeTLBFSInit(&cfg->hugetlbfs[i], hugetlbfs[i], i != 0) < 0) - goto cleanup; + return -1; } } if (virConfGetValueBool(conf, "clear_emulator_capabilities", &cfg->clearEmulatorCapabilities) < 0) - goto cleanup; + return -1; if (virConfGetValueString(conf, "bridge_helper", &cfg->bridgeHelperName) < 0) - goto cleanup; + return -1; if (virConfGetValueString(conf, "pr_helper", &cfg->prHelperName) < 0) - goto cleanup; + return -1; if (virConfGetValueBool(conf, "set_process_name", &cfg->setProcessName) < 0) - goto cleanup; + return -1; if (virConfGetValueUInt(conf, "max_processes", &cfg->maxProcesses) < 0) - goto cleanup; + return -1; if (virConfGetValueUInt(conf, "max_files", &cfg->maxFiles) < 0) - goto cleanup; + return -1; if (virConfGetValueType(conf, "max_core") == VIR_CONF_STRING) { if (virConfGetValueString(conf, "max_core", &corestr) < 0) - goto cleanup; + return -1; if (STREQ(corestr, "unlimited")) { cfg->maxCore = ULLONG_MAX; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown core size '%s'"), corestr); - goto cleanup; + return -1; } } else if (virConfGetValueULLong(conf, "max_core", &cfg->maxCore) < 0) { - goto cleanup; + return -1; } if (virConfGetValueBool(conf, "dump_guest_core", &cfg->dumpGuestCore) < 0) - goto cleanup; + return -1; if (virConfGetValueString(conf, "stdio_handler", &stdioHandler) < 0) - goto cleanup; + return -1; if (stdioHandler) { if (STREQ(stdioHandler, "logd")) { cfg->stdioLogD = true; @@ -714,17 +713,11 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unknown stdio handler %s"), stdioHandler); - VIR_FREE(stdioHandler); - goto cleanup; + return -1; } - VIR_FREE(stdioHandler); } - ret = 0; - cleanup: - virStringListFree(hugetlbfs); - VIR_FREE(corestr); - return ret; + return 0; } -- 2.16.4
On Mon, Jan 21, 2019 at 02:56:14PM +0100, Ján Tomko wrote:
Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros to get rid of the cleanup section.
Requested-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_conf.c | 45 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 256aad2c0b..fc7101904e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -642,15 +642,14 @@ static int virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg, virConfPtr conf) { - char *stdioHandler = NULL; - char **hugetlbfs = NULL; - char *corestr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) stdioHandler = NULL; + VIR_AUTOPTR(virString) hugetlbfs = NULL;
Minor nit pick, can ^this one be moved 1 up or 1 down? Reviewed-by: Erik Skultety <eskultet@redhat.com>
On Mon, Jan 21, 2019 at 03:05:30PM +0100, Erik Skultety wrote:
On Mon, Jan 21, 2019 at 02:56:14PM +0100, Ján Tomko wrote:
Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros to get rid of the cleanup section.
Requested-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_conf.c | 45 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 256aad2c0b..fc7101904e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -642,15 +642,14 @@ static int virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg, virConfPtr conf) { - char *stdioHandler = NULL; - char **hugetlbfs = NULL; - char *corestr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) stdioHandler = NULL; + VIR_AUTOPTR(virString) hugetlbfs = NULL;
Minor nit pick, can ^this one be moved 1 up or 1 down?
Why yes, it can! Care to provide a justification for such change? Jano
Reviewed-by: Erik Skultety <eskultet@redhat.com>
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Jan 21, 2019 at 03:12:28PM +0100, Ján Tomko wrote:
On Mon, Jan 21, 2019 at 03:05:30PM +0100, Erik Skultety wrote:
On Mon, Jan 21, 2019 at 02:56:14PM +0100, Ján Tomko wrote:
Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros to get rid of the cleanup section.
Requested-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_conf.c | 45 +++++++++++++++++++-------------------------- 1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 256aad2c0b..fc7101904e 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -642,15 +642,14 @@ static int virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfigPtr cfg, virConfPtr conf) { - char *stdioHandler = NULL; - char **hugetlbfs = NULL; - char *corestr = NULL; - int ret = -1; + VIR_AUTOFREE(char *) stdioHandler = NULL; + VIR_AUTOPTR(virString) hugetlbfs = NULL;
Minor nit pick, can ^this one be moved 1 up or 1 down?
Why yes, it can!
Care to provide a justification for such change?
Sure, optically it looks better to couple similar declarations together, if you look at patch 3 you made something similar, where you also changed the order compared to the original and it looks more pleasant. Erik
Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros to get rid of the cleanup section. Requested-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_conf.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index fc7101904e..04061b0135 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -829,31 +829,27 @@ static int virQEMUDriverConfigLoadNVRAMEntry(virQEMUDriverConfigPtr cfg, virConfPtr conf) { - char **nvram = NULL; - int ret = -1; + VIR_AUTOPTR(virString) nvram = NULL; size_t i; if (virConfGetValueStringList(conf, "nvram", false, &nvram) < 0) - goto cleanup; + return -1; if (nvram) { virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares); cfg->nfirmwares = virStringListLength((const char *const *)nvram); if (nvram[0] && VIR_ALLOC_N(cfg->firmwares, cfg->nfirmwares) < 0) - goto cleanup; + return -1; for (i = 0; nvram[i] != NULL; i++) { if (VIR_ALLOC(cfg->firmwares[i]) < 0) - goto cleanup; + return -1; if (virFirmwareParse(nvram[i], cfg->firmwares[i]) < 0) - goto cleanup; + return -1; } } - ret = 0; - cleanup: - virStringListFree(nvram); - return ret; + return 0; } -- 2.16.4
Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros to get rid of the cleanup section. Requested-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_conf.c | 52 +++++++++++++++++++++++----------------------------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 04061b0135..524e5d4d0f 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -866,14 +866,14 @@ virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg, virConfPtr conf, bool privileged) { - char *user = NULL, *group = NULL; - char **controllers = NULL; - char **namespaces = NULL; - int ret = -1; + VIR_AUTOPTR(virString) controllers = NULL; + VIR_AUTOPTR(virString) namespaces = NULL; + VIR_AUTOFREE(char *) user = NULL; + VIR_AUTOFREE(char *) group = NULL; size_t i, j; if (virConfGetValueStringList(conf, "security_driver", true, &cfg->securityDriverNames) < 0) - goto cleanup; + return -1; for (i = 0; cfg->securityDriverNames && cfg->securityDriverNames[i] != NULL; i++) { for (j = i + 1; cfg->securityDriverNames[j] != NULL; j++) { @@ -882,32 +882,32 @@ virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg, virReportError(VIR_ERR_CONF_SYNTAX, _("Duplicate security driver %s"), cfg->securityDriverNames[i]); - goto cleanup; + return -1; } } } if (virConfGetValueBool(conf, "security_default_confined", &cfg->securityDefaultConfined) < 0) - goto cleanup; + return -1; if (virConfGetValueBool(conf, "security_require_confined", &cfg->securityRequireConfined) < 0) - goto cleanup; + return -1; if (virConfGetValueString(conf, "user", &user) < 0) - goto cleanup; + return -1; if (user && virGetUserID(user, &cfg->user) < 0) - goto cleanup; + return -1; if (virConfGetValueString(conf, "group", &group) < 0) - goto cleanup; + return -1; if (group && virGetGroupID(group, &cfg->group) < 0) - goto cleanup; + return -1; if (virConfGetValueBool(conf, "dynamic_ownership", &cfg->dynamicOwnership) < 0) - goto cleanup; + return -1; if (virConfGetValueStringList(conf, "cgroup_controllers", false, &controllers) < 0) - goto cleanup; + return -1; if (controllers) { cfg->cgroupControllers = 0; @@ -917,7 +917,7 @@ virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg, virReportError(VIR_ERR_CONF_SYNTAX, _("Unknown cgroup controller '%s'"), controllers[i]); - goto cleanup; + return -1; } cfg->cgroupControllers |= (1 << ctl); } @@ -925,13 +925,13 @@ virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg, if (virConfGetValueStringList(conf, "cgroup_device_acl", false, &cfg->cgroupDeviceACL) < 0) - goto cleanup; + return -1; if (virConfGetValueInt(conf, "seccomp_sandbox", &cfg->seccompSandbox) < 0) - goto cleanup; + return -1; if (virConfGetValueStringList(conf, "namespaces", false, &namespaces) < 0) - goto cleanup; + return -1; if (namespaces) { virBitmapClearAll(cfg->namespaces); @@ -943,38 +943,32 @@ virQEMUDriverConfigLoadSecurityEntry(virQEMUDriverConfigPtr cfg, virReportError(VIR_ERR_CONF_SYNTAX, _("Unknown namespace: %s"), namespaces[i]); - goto cleanup; + return -1; } if (!privileged) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("cannot use namespaces in session mode")); - goto cleanup; + return -1; } if (!qemuDomainNamespaceAvailable(ns)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("%s namespace is not available"), namespaces[i]); - goto cleanup; + return -1; } if (virBitmapSetBit(cfg->namespaces, ns) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to enable namespace: %s"), namespaces[i]); - goto cleanup; + return -1; } } } - ret = 0; - cleanup: - virStringListFree(controllers); - virStringListFree(namespaces); - VIR_FREE(user); - VIR_FREE(group); - return ret; + return 0; } -- 2.16.4
Switch the function to use VIR_AUTOFREE and VIR_AUTOPTR macros to get rid of the cleanup section. Requested-by: John Ferlan <jferlan@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/qemu/qemu_conf.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 524e5d4d0f..ba491e448b 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -984,24 +984,20 @@ static int virQEMUDriverConfigLoadSWTPMEntry(virQEMUDriverConfigPtr cfg, virConfPtr conf) { - char *swtpm_user = NULL, *swtpm_group = NULL; - int ret = -1; + VIR_AUTOFREE(char *) swtpm_user = NULL; + VIR_AUTOFREE(char *) swtpm_group = NULL; if (virConfGetValueString(conf, "swtpm_user", &swtpm_user) < 0) - goto cleanup; + return -1; if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user) < 0) - goto cleanup; + return -1; if (virConfGetValueString(conf, "swtpm_group", &swtpm_group) < 0) - goto cleanup; + return -1; if (swtpm_group && virGetGroupID(swtpm_group, &cfg->swtpm_group) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - VIR_FREE(swtpm_user); - VIR_FREE(swtpm_group); - return ret; + return 0; } -- 2.16.4
On Mon, Jan 21, 2019 at 02:56:13PM +0100, Ján Tomko wrote:
Ján Tomko (4): virQEMUDriverConfigLoadProcessEntry: use VIR_AUTOFREE virQEMUDriverConfigLoadNVRAMEntry: use VIR_AUTOFREE virQEMUDriverConfigLoadSecurityEntry: use VIR_AUTOFREE virQEMUDriverConfigLoadSWTPMEntry: use VIR_AUTOFREE
src/qemu/qemu_conf.c | 131 +++++++++++++++++++++------------------------------ 1 file changed, 55 insertions(+), 76 deletions(-)
Reviewed-by: Erik Skultety <eskultet@redhat.com>
participants (2)
-
Erik Skultety -
Ján Tomko