[libvirt PATCH 00/38] virHashNew refactorings - part VII

"virHashNew" cannot return NULL, yet we check for NULL in various places. See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html. Not split up further as per https://listman.redhat.com/archives/libvir-list/2021-July/msg00575.html Tim Wiederhake (38): virSystemdActivationNew: `virHashNew` cannot return NULL virSystemdActivationNew: Use automatic memory management virSystemdActivationNew: Remove superfluous `goto`s virNWFilterCreateVarsFrom: `virHashNew` cannot return NULL virNWFilterCreateVarsFrom: Use automatic memory management virNWFilterCreateVarsFrom: Remove superfluous `goto`s virNWFilterRuleDefToRuleInst: `virHashNew` cannot return NULL ebiptablesApplyNewRules: `virHashNew` cannot return NULL hypervCreateEmbeddedParam: `virHashNew` cannot return NULL qemusecuritymock: init_hash: `virHashNew` cannot return NULL libxlLoggerNew: `virHashNew` cannot return NULL qemuBlockNodeNamesDetect: `virHashNew` cannot return NULL qemuBlockNodeNameGetBackingChain: `virHashNew` cannot return NULL qemuDomainObjPrivateAlloc: `virHashNew` cannot return NULL qemuInteropFetchConfigs: `virHashNew` cannot return NULL virChrdevAlloc: `virHashNew` cannot return NULL virCloseCallbacksNew: `virHashNew` cannot return NULL virDomainCCWAddressSetCreate: `virHashNew` cannot return NULL virDomainDefBootOrderPostParse: `virHashNew` cannot return NULL virDomainDefValidateAliases: `virHashNew` cannot return NULL virDomainMomentObjListNew: `virHashNew` cannot return NULL virDomainObjListNew: `virHashNew` cannot return NULL virFileCacheNew: `virHashNew` cannot return NULL virHashAtomicNew: `virHashNew` cannot return NULL virInterfaceObjListNew: `virHashNew` cannot return NULL virLockDaemonNew: `virHashNew` cannot return NULL virLockDaemonNewPostExecRestart: `virHashNew` cannot return NULL virLockSpaceNew: `virHashNew` cannot return NULL virLockSpaceNewPostExecRestart: `virHashNew` cannot return NULL virNetDaemonNew: `virHashNew` cannot return NULL virNetworkObjListNew: `virHashNew` cannot return NULL virNetworkObjNew: `virHashNew` cannot return NULL virNodeDeviceObjListNew: `virHashNew` cannot return NULL virQEMUCapsProbeQMPHostCPU: `virHashNew` cannot return NULL virSecuritySELinuxLXCInitialize: `virHashNew` cannot return NULL virSecuritySELinuxQEMUInitialize: `virHashNew` cannot return NULL virStoragePoolObjListNew: `virHashNew` cannot return NULL virStorageVolObjListNew: `virHashNew` cannot return NULL src/conf/domain_addr.c | 7 +------ src/conf/domain_conf.c | 5 +---- src/conf/domain_validate.c | 3 +-- src/conf/virchrdev.c | 6 +----- src/conf/virdomainmomentobjlist.c | 4 ---- src/conf/virdomainobjlist.c | 8 ++------ src/conf/virinterfaceobj.c | 5 +---- src/conf/virnetworkobj.c | 8 ++------ src/conf/virnodedeviceobj.c | 5 +---- src/conf/virstorageobj.c | 17 +++++------------ src/hyperv/hyperv_wmi.c | 6 +----- src/hypervisor/virclosecallbacks.c | 4 ---- src/libxl/libxl_logger.c | 4 +--- src/locking/lock_daemon.c | 6 ++---- src/nwfilter/nwfilter_ebiptables_driver.c | 3 --- src/nwfilter/nwfilter_gentech_driver.c | 18 ++++++------------ src/qemu/qemu_block.c | 15 +++------------ src/qemu/qemu_capabilities.c | 3 +-- src/qemu/qemu_domain.c | 3 +-- src/qemu/qemu_interop_config.c | 5 +---- src/rpc/virnetdaemon.c | 3 +-- src/security/security_selinux.c | 6 ++---- src/util/virfilecache.c | 7 +------ src/util/virhash.c | 5 +---- src/util/virlockspace.c | 6 ++---- src/util/virsystemd.c | 16 +++++----------- tests/qemusecuritymock.c | 17 +++-------------- 27 files changed, 46 insertions(+), 149 deletions(-) -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virsystemd.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 96f81cd3fa..6417dc6ea7 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -964,8 +964,7 @@ virSystemdActivationNew(virSystemdActivationMap *map, VIR_DEBUG("Activated with %d FDs", nfds); act = g_new0(virSystemdActivation, 1); - if (!(act->fds = virHashNew(virSystemdActivationEntryFree))) - goto error; + act->fds = virHashNew(virSystemdActivationEntryFree); fdnames = getenv("LISTEN_FDNAMES"); if (fdnames) { -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virsystemd.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index 6417dc6ea7..f90c17e767 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -958,11 +958,10 @@ virSystemdActivationNew(virSystemdActivationMap *map, size_t nmap, int nfds) { - virSystemdActivation *act; + g_autoptr(virSystemdActivation) act = g_new0(virSystemdActivation, 1); const char *fdnames; VIR_DEBUG("Activated with %d FDs", nfds); - act = g_new0(virSystemdActivation, 1); act->fds = virHashNew(virSystemdActivationEntryFree); @@ -976,10 +975,9 @@ virSystemdActivationNew(virSystemdActivationMap *map, } VIR_DEBUG("Created activation object for %d FDs", nfds); - return act; + return g_steal_pointer(&act); error: - virSystemdActivationFree(act); return NULL; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virsystemd.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c index f90c17e767..99c92b6f52 100644 --- a/src/util/virsystemd.c +++ b/src/util/virsystemd.c @@ -968,17 +968,14 @@ virSystemdActivationNew(virSystemdActivationMap *map, fdnames = getenv("LISTEN_FDNAMES"); if (fdnames) { if (virSystemdActivationInitFromNames(act, nfds, fdnames) < 0) - goto error; + return NULL; } else { if (virSystemdActivationInitFromMap(act, nfds, map, nmap) < 0) - goto error; + return NULL; } VIR_DEBUG("Created activation object for %d FDs", nfds); return g_steal_pointer(&act); - - error: - return NULL; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index da4f71daf1..dbb6b1f80e 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -200,8 +200,6 @@ virNWFilterCreateVarsFrom(GHashTable *vars1, GHashTable *vars2) { GHashTable *res = virHashNew(virNWFilterVarValueHashFree); - if (!res) - return NULL; if (virNWFilterHashTablePutAll(vars1, res) < 0) goto error; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index dbb6b1f80e..d17be401e6 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -199,7 +199,7 @@ static GHashTable * virNWFilterCreateVarsFrom(GHashTable *vars1, GHashTable *vars2) { - GHashTable *res = virHashNew(virNWFilterVarValueHashFree); + g_autoptr(GHashTable) res = virHashNew(virNWFilterVarValueHashFree); if (virNWFilterHashTablePutAll(vars1, res) < 0) goto error; @@ -207,10 +207,9 @@ virNWFilterCreateVarsFrom(GHashTable *vars1, if (virNWFilterHashTablePutAll(vars2, res) < 0) goto error; - return res; + return g_steal_pointer(&res); error: - virHashFree(res); return NULL; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index d17be401e6..a425285e8c 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -202,15 +202,12 @@ virNWFilterCreateVarsFrom(GHashTable *vars1, g_autoptr(GHashTable) res = virHashNew(virNWFilterVarValueHashFree); if (virNWFilterHashTablePutAll(vars1, res) < 0) - goto error; + return NULL; if (virNWFilterHashTablePutAll(vars2, res) < 0) - goto error; + return NULL; return g_steal_pointer(&res); - - error: - return NULL; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/nwfilter/nwfilter_gentech_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index a425285e8c..f98af5d513 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -261,8 +261,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/nwfilter/nwfilter_ebiptables_driver.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 345562bab6..ec17d43c4e 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -3338,9 +3338,6 @@ ebiptablesApplyNewRules(const char *ifname, size_t nsubchains = 0; int ret = -1; - if (!chains_in_set || !chains_out_set) - goto cleanup; - if (nrules) qsort(rules, nrules, sizeof(rules[0]), virNWFilterRuleInstSortPtr); -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/hyperv/hyperv_wmi.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/hyperv/hyperv_wmi.c b/src/hyperv/hyperv_wmi.c index b113283aeb..dc2c6471ab 100644 --- a/src/hyperv/hyperv_wmi.c +++ b/src/hyperv/hyperv_wmi.c @@ -275,7 +275,7 @@ hypervCreateEmbeddedParam(hypervWmiClassInfo *classInfo) { size_t i; size_t count; - g_autoptr(GHashTable) table = NULL; + g_autoptr(GHashTable) table = virHashNew(NULL); XmlSerializerInfo *typeinfo = NULL; typeinfo = classInfo->serializerInfo; @@ -284,10 +284,6 @@ hypervCreateEmbeddedParam(hypervWmiClassInfo *classInfo) for (count = 0; typeinfo[count].name != NULL; count++) ; - table = virHashNew(NULL); - if (table == NULL) - return NULL; - for (i = 0; typeinfo[i].name != NULL; i++) { XmlSerializerInfo *item = &typeinfo[i]; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/qemusecuritymock.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c index 5daf27ccd7..87aadf564e 100644 --- a/tests/qemusecuritymock.c +++ b/tests/qemusecuritymock.c @@ -100,20 +100,9 @@ init_hash(void) if (xattr_paths) return; - if (!(xattr_paths = virHashNew(g_free))) { - fprintf(stderr, "Unable to create hash table for XATTR paths\n"); - abort(); - } - - if (!(chown_paths = virHashNew(g_free))) { - fprintf(stderr, "Unable to create hash table for chowned paths\n"); - abort(); - } - - if (!(selinux_paths = virHashNew(g_free))) { - fprintf(stderr, "Unable to create hash table for selinux labels\n"); - abort(); - } + xattr_paths = virHashNew(g_free); + chown_paths = virHashNew(g_free); + selinux_paths = virHashNew(g_free); } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/libxl/libxl_logger.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libxl/libxl_logger.c b/src/libxl/libxl_logger.c index 0807383d39..f7b5c8ee16 100644 --- a/src/libxl/libxl_logger.c +++ b/src/libxl/libxl_logger.c @@ -149,9 +149,7 @@ libxlLoggerNew(const char *logDir, virLogPriority minLevel) break; } logger.logDir = logDir; - - if ((logger.files = virHashNew(libxlLoggerFileFree)) == NULL) - return NULL; + logger.files = virHashNew(libxlLoggerFileFree); path = g_strdup_printf("%s/libxl-driver.log", logDir); -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_block.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 6627d044cd..2815eb54fa 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -365,10 +365,7 @@ qemuBlockNodeNamesDetect(virQEMUDriver *driver, GHashTable * qemuBlockGetNodeData(virJSONValue *data) { - g_autoptr(GHashTable) nodedata = NULL; - - if (!(nodedata = virHashNew(virJSONValueHashFree))) - return NULL; + g_autoptr(GHashTable) nodedata = virHashNew(virJSONValueHashFree); if (virJSONValueArrayForeachSteal(data, qemuBlockNamedNodesArrayToHash, nodedata) < 0) -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_block.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 2815eb54fa..8150241015 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -231,22 +231,16 @@ qemuBlockNodeNameGetBackingChain(virJSONValue *namednodes, virJSONValue *blockstats) { struct qemuBlockNodeNameGetBackingChainData data; - g_autoptr(GHashTable) namednodestable = NULL; - g_autoptr(GHashTable) disks = NULL; + g_autoptr(GHashTable) namednodestable = virHashNew(virJSONValueHashFree); + g_autoptr(GHashTable) disks = virHashNew(qemuBlockNodeNameBackingChainDataHashEntryFree); memset(&data, 0, sizeof(data)); - if (!(namednodestable = virHashNew(virJSONValueHashFree))) - return NULL; - if (virJSONValueArrayForeachSteal(namednodes, qemuBlockNamedNodesArrayToHash, namednodestable) < 0) return NULL; - if (!(disks = virHashNew(qemuBlockNodeNameBackingChainDataHashEntryFree))) - return NULL; - data.nodenamestable = namednodestable; data.disks = disks; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_domain.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ac1d8ef151..b919da6eab 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1788,8 +1788,7 @@ qemuDomainObjPrivateAlloc(void *opaque) if (!(priv->devs = virChrdevAlloc())) goto error; - if (!(priv->blockjobs = virHashNew(virObjectFreeHashData))) - goto error; + priv->blockjobs = virHashNew(virObjectFreeHashData); /* agent commands block by default, user can choose different behavior */ priv->agentTimeout = VIR_DOMAIN_AGENT_RESPONSE_TIMEOUT_BLOCK; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_interop_config.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_interop_config.c b/src/qemu/qemu_interop_config.c index ea2afcc070..848e8f7381 100644 --- a/src/qemu/qemu_interop_config.c +++ b/src/qemu/qemu_interop_config.c @@ -88,7 +88,7 @@ qemuInteropFetchConfigs(const char *name, char ***configs, bool privileged) { - g_autoptr(GHashTable) files = NULL; + g_autoptr(GHashTable) files = virHashNew(g_free); g_autofree char *homeConfig = NULL; g_autofree char *xdgConfig = NULL; g_autofree char *sysLocation = virFileBuildPath(QEMU_SYSTEM_LOCATION, name, NULL); @@ -117,9 +117,6 @@ qemuInteropFetchConfigs(const char *name, homeConfig = g_strdup_printf("%s/qemu/%s", xdgConfig, name); } - if (!(files = virHashNew(g_free))) - return -1; - if (qemuBuildFileList(files, sysLocation) < 0) return -1; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virchrdev.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c index 91f2b5a233..5d6de68427 100644 --- a/src/conf/virchrdev.c +++ b/src/conf/virchrdev.c @@ -265,13 +265,9 @@ virChrdevs *virChrdevAlloc(void) /* there will hardly be any devices most of the time, the hash * does not have to be huge */ - if (!(devs->hash = virHashNew(virChrdevHashEntryFree))) - goto error; + devs->hash = virHashNew(virChrdevHashEntryFree); return devs; - error: - virChrdevFree(devs); - return NULL; } /** -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/hypervisor/virclosecallbacks.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/hypervisor/virclosecallbacks.c b/src/hypervisor/virclosecallbacks.c index b9d4940b60..2578a71f03 100644 --- a/src/hypervisor/virclosecallbacks.c +++ b/src/hypervisor/virclosecallbacks.c @@ -69,10 +69,6 @@ virCloseCallbacksNew(void) return NULL; closeCallbacks->list = virHashNew(g_free); - if (!closeCallbacks->list) { - virObjectUnref(closeCallbacks); - return NULL; - } return closeCallbacks; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_addr.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index f011721beb..53b39923e8 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1388,8 +1388,7 @@ virDomainCCWAddressSetCreate(void) addrs = g_new0(virDomainCCWAddressSet, 1); - if (!(addrs->defined = virHashNew(g_free))) - goto error; + addrs->defined = virHashNew(g_free); /* must use cssid = 0xfe (254) for virtio-ccw devices */ addrs->next.cssid = 254; @@ -1397,10 +1396,6 @@ virDomainCCWAddressSetCreate(void) addrs->next.devno = 0; addrs->next.assigned = 0; return addrs; - - error: - virDomainCCWAddressSetFree(addrs); - return NULL; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_conf.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e05ea9ba88..6937863db7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5924,10 +5924,7 @@ virDomainDefCollectBootOrder(virDomainDef *def G_GNUC_UNUSED, static int virDomainDefBootOrderPostParse(virDomainDef *def) { - g_autoptr(GHashTable) bootHash = NULL; - - if (!(bootHash = virHashNew(NULL))) - return -1; + g_autoptr(GHashTable) bootHash = virHashNew(NULL); if (virDomainDeviceInfoIterate(def, virDomainDefCollectBootOrder, bootHash) < 0) return -1; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/domain_validate.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index df2ab47361..aab377fbbd 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1261,8 +1261,7 @@ virDomainDefValidateAliases(const virDomainDef *def, int ret = -1; /* We are not storing copies of aliases. Don't free them. */ - if (!(data.aliases = virHashNew(NULL))) - goto cleanup; + data.aliases = virHashNew(NULL); if (virDomainDeviceInfoIterateFlags((virDomainDef *) def, virDomainDeviceDefValidateAliasesIterator, -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virdomainmomentobjlist.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/conf/virdomainmomentobjlist.c b/src/conf/virdomainmomentobjlist.c index c0896685a9..17b9c16ae7 100644 --- a/src/conf/virdomainmomentobjlist.c +++ b/src/conf/virdomainmomentobjlist.c @@ -274,10 +274,6 @@ virDomainMomentObjListNew(void) moments = g_new0(virDomainMomentObjList, 1); moments->objs = virHashNew(virDomainMomentObjListDataFree); - if (!moments->objs) { - VIR_FREE(moments); - return NULL; - } return moments; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virdomainobjlist.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 5f9fd9aabc..43d09692a9 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -75,12 +75,8 @@ virDomainObjList *virDomainObjListNew(void) if (!(doms = virObjectRWLockableNew(virDomainObjListClass))) return NULL; - if (!(doms->objs = virHashNew(virObjectFreeHashData)) || - !(doms->objsName = virHashNew(virObjectFreeHashData))) { - virObjectUnref(doms); - return NULL; - } - + doms->objs = virHashNew(virObjectFreeHashData); + doms->objsName = virHashNew(virObjectFreeHashData); return doms; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virfilecache.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c index 2f55deefb6..64348dc1e6 100644 --- a/src/util/virfilecache.c +++ b/src/util/virfilecache.c @@ -242,8 +242,7 @@ virFileCacheNew(const char *dir, if (!(cache = virObjectNew(virFileCacheClass))) return NULL; - if (!(cache->table = virHashNew(virObjectFreeHashData))) - goto cleanup; + cache->table = virHashNew(virObjectFreeHashData); cache->dir = g_strdup(dir); @@ -252,10 +251,6 @@ virFileCacheNew(const char *dir, cache->handlers = *handlers; return cache; - - cleanup: - virObjectUnref(cache); - return NULL; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virhash.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/util/virhash.c b/src/util/virhash.c index d2b591c0b7..a9996c9fc0 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -102,10 +102,7 @@ virHashAtomicNew(virHashDataFree dataFree) if (!(hash = virObjectLockableNew(virHashAtomicClass))) return NULL; - if (!(hash->hash = virHashNew(dataFree))) { - virObjectUnref(hash); - return NULL; - } + hash->hash = virHashNew(dataFree); return hash; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virinterfaceobj.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c index c8fda4990f..a73208f1fc 100644 --- a/src/conf/virinterfaceobj.c +++ b/src/conf/virinterfaceobj.c @@ -142,10 +142,7 @@ virInterfaceObjListNew(void) if (!(interfaces = virObjectRWLockableNew(virInterfaceObjListClass))) return NULL; - if (!(interfaces->objsName = virHashNew(virObjectFreeHashData))) { - virObjectUnref(interfaces); - return NULL; - } + interfaces->objsName = virHashNew(virObjectFreeHashData); return interfaces; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/locking/lock_daemon.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 05fc68dc54..f5e5375e80 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -153,8 +153,7 @@ virLockDaemonNew(virLockDaemonConfig *config, bool privileged) virObjectUnref(srv); srv = NULL; - if (!(lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree))) - goto error; + lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree); if (!(lockd->defaultLockspace = virLockSpaceNew(NULL))) goto error; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/locking/lock_daemon.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index f5e5375e80..fa08acbc76 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -211,8 +211,7 @@ virLockDaemonNewPostExecRestart(virJSONValue *object, bool privileged) g_mutex_init(&lockd->lock); - if (!(lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree))) - goto error; + lockd->lockspaces = virHashNew(virLockDaemonLockSpaceDataFree); if (!(child = virJSONValueObjectGet(object, "defaultLockspace"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virlockspace.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index 9986d8df41..edf2ec907f 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -247,8 +247,7 @@ virLockSpace *virLockSpaceNew(const char *directory) lockspace->dir = g_strdup(directory); - if (!(lockspace->resources = virHashNew(virLockSpaceResourceDataFree))) - goto error; + lockspace->resources = virHashNew(virLockSpaceResourceDataFree); if (directory) { if (virFileExists(directory)) { -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virlockspace.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index edf2ec907f..0c4beaf9fe 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -293,8 +293,7 @@ virLockSpace *virLockSpaceNewPostExecRestart(virJSONValue *object) return NULL; } - if (!(lockspace->resources = virHashNew(virLockSpaceResourceDataFree))) - goto error; + virHashNew(virLockSpaceResourceDataFree); if (virJSONValueObjectHasKey(object, "directory")) { const char *dir = virJSONValueObjectGetString(object, "directory"); -- 2.31.1

On Thu, Jul 22, 2021 at 09:50:21 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virlockspace.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c index edf2ec907f..0c4beaf9fe 100644 --- a/src/util/virlockspace.c +++ b/src/util/virlockspace.c @@ -293,8 +293,7 @@ virLockSpace *virLockSpaceNewPostExecRestart(virJSONValue *object) return NULL; }
- if (!(lockspace->resources = virHashNew(virLockSpaceResourceDataFree))) - goto error; + virHashNew(virLockSpaceResourceDataFree);
lockspace->resources = virHashNew(virLockSpaceResourceDataFree);

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/rpc/virnetdaemon.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 1a17753f42..b6f3233f64 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -141,8 +141,7 @@ virNetDaemonNew(void) if (!(dmn = virObjectLockableNew(virNetDaemonClass))) return NULL; - if (!(dmn->servers = virHashNew(virObjectFreeHashData))) - goto error; + dmn->servers = virHashNew(virObjectFreeHashData); #ifndef WIN32 dmn->sigwrite = dmn->sigread = -1; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virnetworkobj.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 341d6b4292..66f916c749 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -350,10 +350,7 @@ virNetworkObjListNew(void) if (!(nets = virObjectRWLockableNew(virNetworkObjListClass))) return NULL; - if (!(nets->objs = virHashNew(virObjectFreeHashData))) { - virObjectUnref(nets); - return NULL; - } + nets->objs = virHashNew(virObjectFreeHashData); return nets; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virnetworkobj.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 66f916c749..ea021892c7 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -118,8 +118,7 @@ virNetworkObjNew(void) virBitmapSetBitExpand(obj->classIdMap, 2) < 0) goto error; - if (!(obj->ports = virHashNew(virNetworkObjPortFree))) - goto error; + obj->ports = virHashNew(virNetworkObjPortFree); virObjectLock(obj); -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virnodedeviceobj.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c index b213592b56..e8f6792138 100644 --- a/src/conf/virnodedeviceobj.c +++ b/src/conf/virnodedeviceobj.c @@ -456,10 +456,7 @@ virNodeDeviceObjListNew(void) if (!(devs = virObjectRWLockableNew(virNodeDeviceObjListClass))) return NULL; - if (!(devs->objs = virHashNew(virObjectFreeHashData))) { - virObjectUnref(devs); - return NULL; - } + devs->objs = virHashNew(virObjectFreeHashData); return devs; } -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_capabilities.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 6cea8c2eca..3804687080 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3045,8 +3045,7 @@ virQEMUCapsProbeQMPHostCPU(virQEMUCaps *qemuCaps, qemuMonitorCPUProperty *nmProp; size_t i; - if (!(hash = virHashNew(NULL))) - goto cleanup; + hash = virHashNew(NULL); for (i = 0; i < modelInfo->nprops; i++) { prop = modelInfo->props + i; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/security/security_selinux.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 0c2cf1d1c7..0bff9b7bae 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -687,8 +687,7 @@ virSecuritySELinuxLXCInitialize(virSecurityManager *mgr) goto error; } - if (!(data->mcs = virHashNew(NULL))) - goto error; + data->mcs = virHashNew(NULL); return 0; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/security/security_selinux.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 0bff9b7bae..5f98d4d47a 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -758,8 +758,7 @@ virSecuritySELinuxQEMUInitialize(virSecurityManager *mgr) VIR_DEBUG("Loaded file context '%s', content context '%s'", data->file_context, data->content_context); - if (!(data->mcs = virHashNew(NULL))) - goto error; + data->mcs = virHashNew(NULL); return 0; -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virstorageobj.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 24957d6012..6c8a06e8bc 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -401,12 +401,8 @@ virStoragePoolObjListNew(void) if (!(pools = virObjectRWLockableNew(virStoragePoolObjListClass))) return NULL; - if (!(pools->objs = virHashNew(virObjectFreeHashData)) || - !(pools->objsName = virHashNew(virObjectFreeHashData))) { - virObjectUnref(pools); - return NULL; - } - + pools->objs = virHashNew(virObjectFreeHashData); + pools->objsName = virHashNew(virObjectFreeHashData); return pools; } -- 2.31.1

On Thu, Jul 22, 2021 at 09:50:29 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virstorageobj.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 24957d6012..6c8a06e8bc 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -401,12 +401,8 @@ virStoragePoolObjListNew(void) if (!(pools = virObjectRWLockableNew(virStoragePoolObjListClass))) return NULL;
- if (!(pools->objs = virHashNew(virObjectFreeHashData)) || - !(pools->objsName = virHashNew(virObjectFreeHashData))) { - virObjectUnref(pools); - return NULL; - } - + pools->objs = virHashNew(virObjectFreeHashData); + pools->objsName = virHashNew(virObjectFreeHashData);
Preferrably keep the newline before 'return'.
return pools; }
-- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virstorageobj.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 6c8a06e8bc..2b6932d7e5 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -170,12 +170,9 @@ virStorageVolObjListNew(void) if (!(vols = virObjectRWLockableNew(virStorageVolObjListClass))) return NULL; - if (!(vols->objsKey = virHashNew(virObjectFreeHashData)) || - !(vols->objsName = virHashNew(virObjectFreeHashData)) || - !(vols->objsPath = virHashNew(virObjectFreeHashData))) { - virObjectUnref(vols); - return NULL; - } + vols->objsKey = virHashNew(virObjectFreeHashData); + vols->objsName = virHashNew(virObjectFreeHashData); + vols->objsPath = virHashNew(virObjectFreeHashData); return vols; } -- 2.31.1

On Thu, Jul 22, 2021 at 09:49:52 +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.
Not split up further as per https://listman.redhat.com/archives/libvir-list/2021-July/msg00575.html
After you fix the two small issues pointed out in specific patches: Series: Reviewed-by: Peter Krempa <pkrempa@redhat.com> I've also identified a few cleanups irrelevant to your series: https://listman.redhat.com/archives/libvir-list/2021-July/msg00725.html
participants (2)
-
Peter Krempa
-
Tim Wiederhake