[libvirt PATCH 0/3] virHashNew refactorings - part VIII

"virHashNew" cannot return NULL, yet we check for NULL in various places. See https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html. * Patch #1 is a fixed version of https://listman.redhat.com/archives/libvir-list/2021-July/msg00720.html. * Patch #2 is a fixed version of https://listman.redhat.com/archives/libvir-list/2021-July/msg00721.html. * For patch #3, see https://listman.redhat.com/archives/libvir-list/2021-July/msg00691.html. I mixed something up and was under the impression that the libvirt code style forbade `for` loops with anything else than `size_t` typed variables. Tim Wiederhake (3): virLockSpaceNewPostExecRestart: `virHashNew` cannot return NULL WIP virStoragePoolObjListNew: `virHashNew` cannot return NULL virNWFilterParseParamAttributes: Simplify loop src/conf/nwfilter_params.c | 7 ++----- src/conf/virstorageobj.c | 7 ++----- src/util/virlockspace.c | 3 +-- 3 files changed, 5 insertions(+), 12 deletions(-) -- 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..46bd922f35 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; + lockspace->resources = virHashNew(virLockSpaceResourceDataFree); if (virJSONValueObjectHasKey(object, "directory")) { const char *dir = virJSONValueObjectGetString(object, "directory"); -- 2.31.1

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/virstorageobj.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 32ed55ff8b..18a7718de9 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -398,11 +398,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

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/nwfilter_params.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index 35ea0256c3..ca7b62874c 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -700,9 +700,8 @@ virNWFilterParseParamAttributes(xmlNodePtr cur) { g_autoptr(GHashTable) table = virHashNew(virNWFilterVarValueHashFree); - cur = xmlFirstElementChild(cur); - - while (cur != NULL) { + for (cur = xmlFirstElementChild(cur); cur != NULL; + cur = xmlNextElementSibling(cur)) { if (virXMLNodeNameEqual(cur, "parameter")) { g_autofree char *nam = virXMLPropString(cur, "name"); g_autofree char *val = virXMLPropString(cur, "value"); @@ -710,7 +709,6 @@ virNWFilterParseParamAttributes(xmlNodePtr cur) if (nam == NULL || !isValidVarName(nam) || val == NULL || !isValidVarValue(val)) { - cur = xmlNextElementSibling(cur); continue; } @@ -725,7 +723,6 @@ virNWFilterParseParamAttributes(xmlNodePtr cur) } value = NULL; } - cur = xmlNextElementSibling(cur); } return g_steal_pointer(&table); -- 2.31.1

ping On Fri, 2021-07-23 at 11:56 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/nwfilter_params.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c index 35ea0256c3..ca7b62874c 100644 --- a/src/conf/nwfilter_params.c +++ b/src/conf/nwfilter_params.c @@ -700,9 +700,8 @@ virNWFilterParseParamAttributes(xmlNodePtr cur) { g_autoptr(GHashTable) table = virHashNew(virNWFilterVarValueHashFree); - cur = xmlFirstElementChild(cur); - - while (cur != NULL) { + for (cur = xmlFirstElementChild(cur); cur != NULL; + cur = xmlNextElementSibling(cur)) { if (virXMLNodeNameEqual(cur, "parameter")) { g_autofree char *nam = virXMLPropString(cur, "name"); g_autofree char *val = virXMLPropString(cur, "value"); @@ -710,7 +709,6 @@ virNWFilterParseParamAttributes(xmlNodePtr cur) if (nam == NULL || !isValidVarName(nam) || val == NULL || !isValidVarValue(val)) { - cur = xmlNextElementSibling(cur); continue; } @@ -725,7 +723,6 @@ virNWFilterParseParamAttributes(xmlNodePtr cur) } value = NULL; } - cur = xmlNextElementSibling(cur); } return g_steal_pointer(&table);

On a %A in %Y, Tim Wiederhake wrote:
ping
Sorry, I thought you already pushed it based on Peter's reply.
On Fri, 2021-07-23 at 11:56 +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/conf/nwfilter_params.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

On Fri, Jul 23, 2021 at 11:56:45 +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.
* Patch #1 is a fixed version of https://listman.redhat.com/archives/libvir-list/2021-July/msg00720.html.
* Patch #2 is a fixed version of https://listman.redhat.com/archives/libvir-list/2021-July/msg00721.html.
These have R-b from previous posting since the requested changes are minor. Also note that patch 2 now has broken summary (WIP).
participants (3)
-
Jano Tomko
-
Peter Krempa
-
Tim Wiederhake