[libvirt PATCH 00/10] virHashNew refactorings - part II
"virHashNew" cannot return NULL, yet we check for NULL in various places. See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html. 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 | 9 +++------ src/util/viriptables.c | 20 +++++--------------- tests/nwfilterxml2firewalltest.c | 18 ++++++------------ 3 files changed, 14 insertions(+), 33 deletions(-) -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@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> --- 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> --- 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> --- 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> --- 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> --- 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> --- 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 | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8f35b4240f..8646efe9c4 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4238,6 +4238,7 @@ int qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, GHashTable **info) { + GHashTable *hash; int ret; VIR_DEBUG("info=%p", info); @@ -4246,14 +4247,14 @@ 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) { + virHashFree(hash); } + *info = hash; return ret; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_monitor.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8646efe9c4..3a56ed8ef9 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4238,7 +4238,7 @@ int qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, GHashTable **info) { - GHashTable *hash; + g_autoptr(GHashTable) hash = NULL; int ret; VIR_DEBUG("info=%p", info); @@ -4250,11 +4250,10 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, if (!(hash = virHashNew(g_free))) return -1; - if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) < 0) { - virHashFree(hash); + if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) >= 0) { + *info = g_steal_pointer(&hash); } - *info = hash; return ret; } -- 2.31.1
On Fri, Jul 09, 2021 at 10:27:38 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_monitor.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8646efe9c4..3a56ed8ef9 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4238,7 +4238,7 @@ int qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, GHashTable **info) { - GHashTable *hash; + g_autoptr(GHashTable) hash = NULL; int ret;
VIR_DEBUG("info=%p", info); @@ -4250,11 +4250,10 @@ qemuMonitorGetMemoryDeviceInfo(qemuMonitor *mon, if (!(hash = virHashNew(g_free))) return -1;
- if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) < 0) { - virHashFree(hash); + if ((ret = qemuMonitorJSONGetMemoryDeviceInfo(mon, hash)) >= 0) { + *info = g_steal_pointer(&hash); }
- *info = hash; return ret
This would probably look better when squashed into the previous commit.
Signed-off-by: Tim Wiederhake <twiederh@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 3a56ed8ef9..4506f962d1 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4238,7 +4238,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); @@ -4247,9 +4247,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
On Fri, Jul 09, 2021 at 10:27:29 +0200, Tim Wiederhake wrote:
"virHashNew" cannot return NULL, yet we check for NULL in various places.
See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html.
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
Series: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Peter Krempa -
Tim Wiederhake