The g_strsplit() function can return NULL if and only if either
the input string is NULL or delimiter is NULL or an empty string.
In neither of places we call it any of the conditions is true and
thus we don't need to check for the return value.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/libxl/xen_xl.c     | 2 --
 src/util/vircgroupv2.c | 2 --
 src/util/virprocess.c  | 2 --
 src/util/virresctrl.c  | 2 --
 4 files changed, 8 deletions(-)

diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
index 7604e3d534..869083a1d1 100644
--- a/src/libxl/xen_xl.c
+++ b/src/libxl/xen_xl.c
@@ -265,8 +265,6 @@ xenParseXLCPUID(virConf *conf, virDomainDef *def)
     }
 
     cpuid_pairs = g_strsplit(cpuid_str, ",", 0);
-    if (!cpuid_pairs)
-        return -1;
 
     if (!cpuid_pairs[0])
         return 0;
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index 4c110940cf..f00a8f154b 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -286,8 +286,6 @@ virCgroupV2ParseControllersFile(virCgroup *group,
     virTrimSpaces(contStr, NULL);
 
     contList = g_strsplit(contStr, " ", 20);
-    if (!contList)
-        return -1;
 
     tmp = contList;
 
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index b559a4257e..06767dbf51 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1837,8 +1837,6 @@ virProcessGetSchedInfo(unsigned long long *cpuWait,
         return -1;
 
     lines = g_strsplit(data, "\n", 0);
-    if (!lines)
-        return -1;
 
     for (i = 0; lines[i] != NULL; i++) {
         const char *line = lines[i];
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
index edbf078654..fe45ad3c64 100644
--- a/src/util/virresctrl.c
+++ b/src/util/virresctrl.c
@@ -1646,8 +1646,6 @@ virResctrlAllocParseCacheLine(virResctrlInfo *resctrl,
     }
 
     caches = g_strsplit(tmp, ";", 0);
-    if (!caches)
-        return 0;
 
     for (next = caches; *next; next++) {
         if (virResctrlAllocParseProcessCache(resctrl, alloc, level, type, *next) < 0)
-- 
2.34.1

On Fri, Jan 07, 2022 at 02:39:17PM +0100, Michal Privoznik wrote:
> The g_strsplit() function can return NULL if and only if either
> the input string is NULL or delimiter is NULL or an empty string.
> In neither of places we call it any of the conditions is true and
> thus we don't need to check for the return value.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/libxl/xen_xl.c     | 2 --
>  src/util/vircgroupv2.c | 2 --
>  src/util/virprocess.c  | 2 --
>  src/util/virresctrl.c  | 2 --
>  4 files changed, 8 deletions(-)
> 
> diff --git a/src/libxl/xen_xl.c b/src/libxl/xen_xl.c
> index 7604e3d534..869083a1d1 100644
> --- a/src/libxl/xen_xl.c
> +++ b/src/libxl/xen_xl.c
> @@ -265,8 +265,6 @@ xenParseXLCPUID(virConf *conf, virDomainDef *def)
>      }
>  
>      cpuid_pairs = g_strsplit(cpuid_str, ",", 0);
> -    if (!cpuid_pairs)
> -        return -1;
>  
>      if (!cpuid_pairs[0])
>          return 0;

But the following pattern:

if (!(identifier = g_strsplit(instr, delim, N)))
    return -1;

can be found in many more places and it is semantically equivalent to what
you're removing in your patch, so all of those should be adjusted. While it
is true that NULL is returned only under those 3 conditions you mentioned,
I don't think this patch is really desirable as a simple typo like "" for the
delimiter which can go unnoticed easily in a large patch series is a problem
which may or may not pop up immediately depending on what tests have been
performed that would exercise the code.

So, unless you have a very compelling point on why we'd benefit from this patch
it's a NACK from me.

Erik