[libvirt PATCH 00/10] virHashNew refactorings - part IV
"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): conf: Add AUTOPTR_CLEANUP_FUNC for virNWFilterBindingDef virNWFilterBindingDefCopy: `virHashNew` cannot return NULL virNWFilterBindingDefCopy: Use automatic memory management virNWFilterBindingDefCopy: Remove superfluous `goto`s virNWFilterBindingDefForNet: `virHashNew` cannot return NULL virNWFilterBindingDefForNet: Use automatic memory management virNWFilterBindingDefForNet: Remove superfluous `goto`s virNWFilterBindingObjListNew: `virHashNew` cannot return NULL virNWFilterBuildAll: `virHashNew` cannot return NULL virNWFilterDHCPSnoopInit: `virHashNew` cannot return NULL src/conf/domain_nwfilter.c | 15 ++++----------- src/conf/virnwfilterbindingdef.c | 15 ++++----------- src/conf/virnwfilterbindingdef.h | 2 ++ src/conf/virnwfilterbindingobjlist.c | 5 +---- src/nwfilter/nwfilter_dhcpsnoop.c | 17 ----------------- src/nwfilter/nwfilter_gentech_driver.c | 3 +-- 6 files changed, 12 insertions(+), 45 deletions(-) -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virnwfilterbindingdef.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/virnwfilterbindingdef.h b/src/conf/virnwfilterbindingdef.h index 0e52789332..68d531b75d 100644 --- a/src/conf/virnwfilterbindingdef.h +++ b/src/conf/virnwfilterbindingdef.h @@ -41,6 +41,8 @@ struct _virNWFilterBindingDef { void virNWFilterBindingDefFree(virNWFilterBindingDef *binding); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNWFilterBindingDef, virNWFilterBindingDefFree); + virNWFilterBindingDef * virNWFilterBindingDefCopy(virNWFilterBindingDef *src); -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virnwfilterbindingdef.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 98df1f750a..9704e1bebb 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -65,8 +65,7 @@ virNWFilterBindingDefCopy(virNWFilterBindingDef *src) ret->filter = g_strdup(src->filter); - if (!(ret->filterparams = virHashNew(virNWFilterVarValueHashFree))) - goto error; + ret->filterparams = virHashNew(virNWFilterVarValueHashFree); if (virNWFilterHashTablePutAll(src->filterparams, ret->filterparams) < 0) goto error; -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virnwfilterbindingdef.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 9704e1bebb..4ed3763efd 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -49,9 +49,7 @@ virNWFilterBindingDefFree(virNWFilterBindingDef *def) virNWFilterBindingDef * virNWFilterBindingDefCopy(virNWFilterBindingDef *src) { - virNWFilterBindingDef *ret; - - ret = g_new0(virNWFilterBindingDef, 1); + g_autoptr(virNWFilterBindingDef) ret = g_new0(virNWFilterBindingDef, 1); ret->ownername = g_strdup(src->ownername); @@ -70,10 +68,9 @@ virNWFilterBindingDefCopy(virNWFilterBindingDef *src) if (virNWFilterHashTablePutAll(src->filterparams, ret->filterparams) < 0) goto error; - return ret; + return g_steal_pointer(&ret); error: - virNWFilterBindingDefFree(ret); return NULL; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virnwfilterbindingdef.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 4ed3763efd..22ecf7b828 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -66,12 +66,9 @@ virNWFilterBindingDefCopy(virNWFilterBindingDef *src) ret->filterparams = virHashNew(virNWFilterVarValueHashFree); if (virNWFilterHashTablePutAll(src->filterparams, ret->filterparams) < 0) - goto error; + return NULL; return g_steal_pointer(&ret); - - error: - return NULL; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_nwfilter.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index 1c62353fa1..5024d5fa03 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -59,8 +59,7 @@ virNWFilterBindingDefForNet(const char *vmname, ret->filter = g_strdup(net->filter); - if (!(ret->filterparams = virHashNew(virNWFilterVarValueHashFree))) - goto error; + ret->filterparams = virHashNew(virNWFilterVarValueHashFree); if (net->filterparams && virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_nwfilter.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index 5024d5fa03..cb35221a59 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -42,9 +42,7 @@ virNWFilterBindingDefForNet(const char *vmname, const unsigned char *vmuuid, virDomainNetDef *net) { - virNWFilterBindingDef *ret; - - ret = g_new0(virNWFilterBindingDef, 1); + g_autoptr(virNWFilterBindingDef) ret = g_new0(virNWFilterBindingDef, 1); ret->ownername = g_strdup(vmname); @@ -65,10 +63,9 @@ virNWFilterBindingDefForNet(const char *vmname, virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) goto error; - return ret; + return g_steal_pointer(&ret); error: - virNWFilterBindingDefFree(ret); return NULL; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_nwfilter.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c index cb35221a59..0a67b6765e 100644 --- a/src/conf/domain_nwfilter.c +++ b/src/conf/domain_nwfilter.c @@ -61,12 +61,9 @@ virNWFilterBindingDefForNet(const char *vmname, if (net->filterparams && virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0) - goto error; + return NULL; return g_steal_pointer(&ret); - - error: - return NULL; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virnwfilterbindingobjlist.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c index 1f19e27eb0..470a30ca90 100644 --- a/src/conf/virnwfilterbindingobjlist.c +++ b/src/conf/virnwfilterbindingobjlist.c @@ -66,10 +66,7 @@ virNWFilterBindingObjListNew(void) if (!(bindings = virObjectRWLockableNew(virNWFilterBindingObjListClass))) return NULL; - if (!(bindings->objs = virHashNew(virObjectFreeHashData))) { - virObjectUnref(bindings); - return NULL; - } + bindings->objs = virHashNew(virObjectFreeHashData); return bindings; } -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 8aa1db23d3..da4f71daf1 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -1007,8 +1007,7 @@ virNWFilterBuildAll(virNWFilterDriverState *driver, VIR_DEBUG("Build all filters newFilters=%d", newFilters); if (newFilters) { - if (!(data.skipInterfaces = virHashNew(NULL))) - return -1; + data.skipInterfaces = virHashNew(NULL); data.step = STEP_APPLY_NEW; if (virNWFilterBindingObjListForEach(driver->bindings, -- 2.31.1
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/nwfilter/nwfilter_dhcpsnoop.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c index 2481ea5371..4b62a7b661 100644 --- a/src/nwfilter/nwfilter_dhcpsnoop.c +++ b/src/nwfilter/nwfilter_dhcpsnoop.c @@ -1998,27 +1998,10 @@ virNWFilterDHCPSnoopInit(void) virNWFilterSnoopState.snoopReqs = virHashNew(virNWFilterSnoopReqRelease); - if (!virNWFilterSnoopState.ifnameToKey || - !virNWFilterSnoopState.snoopReqs || - !virNWFilterSnoopState.active) - goto error; - virNWFilterSnoopLeaseFileLoad(); virNWFilterSnoopLeaseFileOpen(); return 0; - - error: - virHashFree(virNWFilterSnoopState.ifnameToKey); - virNWFilterSnoopState.ifnameToKey = NULL; - - virHashFree(virNWFilterSnoopState.snoopReqs); - virNWFilterSnoopState.snoopReqs = NULL; - - virHashFree(virNWFilterSnoopState.active); - virNWFilterSnoopState.active = NULL; - - return -1; } /** -- 2.31.1
On Tue, Jul 13, 2021 at 16:44:31 +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): conf: Add AUTOPTR_CLEANUP_FUNC for virNWFilterBindingDef virNWFilterBindingDefCopy: `virHashNew` cannot return NULL virNWFilterBindingDefCopy: Use automatic memory management virNWFilterBindingDefCopy: Remove superfluous `goto`s virNWFilterBindingDefForNet: `virHashNew` cannot return NULL virNWFilterBindingDefForNet: Use automatic memory management virNWFilterBindingDefForNet: Remove superfluous `goto`s virNWFilterBindingObjListNew: `virHashNew` cannot return NULL virNWFilterBuildAll: `virHashNew` cannot return NULL virNWFilterDHCPSnoopInit: `virHashNew` cannot return NULL
Series: Reviewed-by: Peter Krempa <pkrempa@redhat.com>
participants (2)
-
Peter Krempa -
Tim Wiederhake