[libvirt PATCH v2 00/10] virHashNew refactorings - part II
"virHashNew" cannot return NULL, yet we check for NULL in various places. V1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00188.html Changes since V1: * Moved the inversion of the `if` condition in patch #9 to patch #8 * Only patches #8 and #9 are missing review Tim Wiederhake (10): virNWFilterCreateVarsFrom: `virHashNew` cannot return NULL virNWFilterCreateVarsFrom: Use automatic memory management virNWFilterCreateVarsFrom: Remove superfluous `goto`s virNWFilterRuleDefToRuleInst: `virHashNew` cannot return NULL iptablesPrivateChainCreate: `virHashNew` cannot return NULL iptablesPrivateChainCreate: Use automatic memory management iptablesPrivateChainCreate: Remove superfluous `goto`s qemuMonitorGetMemoryDeviceInfo: Assign hash table only on success qemuMonitorGetMemoryDeviceInfo: Use automatic memory management qemuMonitorGetMemoryDeviceInfo: `virHashNew` cannot return NULL src/qemu/qemu_monitor.c | 10 +++------- src/util/viriptables.c | 20 +++++--------------- tests/nwfilterxml2firewalltest.c | 18 ++++++------------ 3 files changed, 14 insertions(+), 34 deletions(-) -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- tests/nwfilterxml2firewalltest.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 6709cc15fd..3b7190b5cd 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -149,8 +149,6 @@ virNWFilterCreateVarsFrom(GHashTable *vars1, GHashTable *vars2) { GHashTable *res = virHashNew(virNWFilterVarValueHashFree); - if (!res) - return NULL; if (virNWFilterHashTablePutAll(vars1, res) < 0) goto err_exit; -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- tests/nwfilterxml2firewalltest.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 3b7190b5cd..26d4a936ad 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -148,7 +148,7 @@ static GHashTable * virNWFilterCreateVarsFrom(GHashTable *vars1, GHashTable *vars2) { - GHashTable *res = virHashNew(virNWFilterVarValueHashFree); + g_autoptr(GHashTable) res = virHashNew(virNWFilterVarValueHashFree); if (virNWFilterHashTablePutAll(vars1, res) < 0) goto err_exit; @@ -156,10 +156,9 @@ virNWFilterCreateVarsFrom(GHashTable *vars1, if (virNWFilterHashTablePutAll(vars2, res) < 0) goto err_exit; - return res; + return g_steal_pointer(&res); err_exit: - virHashFree(res); return NULL; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- tests/nwfilterxml2firewalltest.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 26d4a936ad..bdfe858185 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -151,15 +151,12 @@ virNWFilterCreateVarsFrom(GHashTable *vars1, g_autoptr(GHashTable) res = virHashNew(virNWFilterVarValueHashFree); if (virNWFilterHashTablePutAll(vars1, res) < 0) - goto err_exit; + return NULL; if (virNWFilterHashTablePutAll(vars2, res) < 0) - goto err_exit; + return NULL; return g_steal_pointer(&res); - - err_exit: - return NULL; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- tests/nwfilterxml2firewalltest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index bdfe858185..75a70e9972 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -208,8 +208,8 @@ virNWFilterRuleDefToRuleInst(virNWFilterDef *def, ruleinst->chainPriority = def->chainPriority; ruleinst->def = rule; ruleinst->priority = rule->priority; - if (!(ruleinst->vars = virHashNew(virNWFilterVarValueHashFree))) - goto cleanup; + ruleinst->vars = virHashNew(virNWFilterVarValueHashFree); + if (virNWFilterHashTablePutAll(vars, ruleinst->vars) < 0) goto cleanup; -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viriptables.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/util/viriptables.c b/src/util/viriptables.c index 4189578245..198ece3d71 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -70,17 +70,12 @@ iptablesPrivateChainCreate(virFirewall *fw, void *opaque) { iptablesGlobalChainData *data = opaque; - GHashTable *chains = NULL; - GHashTable *links = NULL; + GHashTable *chains = virHashNew(NULL); + GHashTable *links = virHashNew(NULL); const char *const *tmp; int ret = -1; size_t i; - if (!(chains = virHashNew(NULL))) - goto cleanup; - if (!(links = virHashNew(NULL))) - goto cleanup; - tmp = lines; while (tmp && *tmp) { if (STRPREFIX(*tmp, "-N ")) { /* eg "-N LIBVIRT_INP" */ -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viriptables.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/viriptables.c b/src/util/viriptables.c index 198ece3d71..847af9b9d7 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -70,8 +70,8 @@ iptablesPrivateChainCreate(virFirewall *fw, void *opaque) { iptablesGlobalChainData *data = opaque; - GHashTable *chains = virHashNew(NULL); - GHashTable *links = virHashNew(NULL); + g_autoptr(GHashTable) chains = virHashNew(NULL); + g_autoptr(GHashTable) links = virHashNew(NULL); const char *const *tmp; int ret = -1; size_t i; @@ -114,8 +114,6 @@ iptablesPrivateChainCreate(virFirewall *fw, ret = 0; cleanup: - virHashFree(chains); - virHashFree(links); return ret; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/util/viriptables.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/util/viriptables.c b/src/util/viriptables.c index 847af9b9d7..721e1eeae7 100644 --- a/src/util/viriptables.c +++ b/src/util/viriptables.c @@ -73,14 +73,13 @@ iptablesPrivateChainCreate(virFirewall *fw, g_autoptr(GHashTable) chains = virHashNew(NULL); g_autoptr(GHashTable) links = virHashNew(NULL); const char *const *tmp; - int ret = -1; size_t i; tmp = lines; while (tmp && *tmp) { if (STRPREFIX(*tmp, "-N ")) { /* eg "-N LIBVIRT_INP" */ if (virHashUpdateEntry(chains, *tmp + 3, (void *)0x1) < 0) - goto cleanup; + return -1; } else if (STRPREFIX(*tmp, "-A ")) { /* eg "-A INPUT -j LIBVIRT_INP" */ char *sep = strchr(*tmp + 3, ' '); if (sep) { @@ -88,7 +87,7 @@ iptablesPrivateChainCreate(virFirewall *fw, if (STRPREFIX(sep + 1, "-j ")) { if (virHashUpdateEntry(links, sep + 4, (char *)*tmp + 3) < 0) - goto cleanup; + return -1; } } } @@ -112,9 +111,7 @@ iptablesPrivateChainCreate(virFirewall *fw, "--jump", data->chains[i].child, NULL); } - ret = 0; - cleanup: - return ret; + return 0; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_monitor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a2df1a6ec3..9b1a3ec3eb 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4214,6 +4214,7 @@ int qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, GHashTable **info) { + GHashTable *hash; int ret; VIR_DEBUG("info=%p", info); @@ -4222,14 +4223,13 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, QEMU_CHECK_MONITOR(mon); - if (!(*info = virHashNew(g_free))) + if (!(hash = virHashNew(g_free))) return -1; - if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, *info)) < 0) { - virHashFree(*info); - *info = NULL; - } + if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) >= 0) + *info = g_steal_pointer(&hash); + virHashFree(hash); return ret; } -- 2.31.1
On Tue, Jul 13, 2021 at 17:12:10 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_monitor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_monitor.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 9b1a3ec3eb..0b5da8b71f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4214,7 +4214,7 @@ int qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, GHashTable **info) { - GHashTable *hash; + g_autoptr(GHashTable) hash = NULL; int ret; VIR_DEBUG("info=%p", info); @@ -4229,7 +4229,6 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) >= 0) *info = g_steal_pointer(&hash); - virHashFree(hash); return ret; } -- 2.31.1
On Tue, Jul 13, 2021 at 17:12:11 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_monitor.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 0b5da8b71f..f66a3457c1 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4214,7 +4214,7 @@ int qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, GHashTable **info) { - g_autoptr(GHashTable) hash = NULL; + g_autoptr(GHashTable) hash = virHashNew(g_free); int ret; VIR_DEBUG("info=%p", info); @@ -4223,9 +4223,6 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, QEMU_CHECK_MONITOR(mon); - if (!(hash = virHashNew(g_free))) - return -1; - if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) >= 0) *info = g_steal_pointer(&hash); -- 2.31.1
participants (2)
-
Peter Krempa -
Tim Wiederhake