From 55a8d118d6e9e28fc889be88c275b396f39ee4a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Tue, 19 Nov 2024 15:41:22 +0100
Subject: [PATCH] basic/strv: return NULL from strv_skip

strv_skip was written to carefully return the original array, but this turns
out to be an unnecessary complication. After the previous patch, no caller
cares about the distinction between NULL and { NULL }, but various callers need
to wrap the process the returned value with strv_isempty(), sometimes more than
once. Let's always return NULL for an empty result to allow callers to be
simplified.
---
 src/basic/strv.c     |  6 ++++--
 src/home/homectl.c   | 22 +++++++++++-----------
 src/test/test-strv.c | 20 ++++++++++++--------
 3 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/src/basic/strv.c b/src/basic/strv.c
index 184804cea0..a4ddaa5e78 100644
--- a/src/basic/strv.c
+++ b/src/basic/strv.c
@@ -976,14 +976,16 @@ bool strv_fnmatch_full(
 }
 
 char** strv_skip(char **l, size_t n) {
-
         while (n > 0) {
                 if (strv_isempty(l))
-                        return l;
+                        return NULL;
 
                 l++, n--;
         }
 
+        /* To simplify callers, always return NULL instead of a zero-item array. */
+        if (strv_isempty(l))
+                return NULL;
         return l;
 }
 
diff --git a/src/home/homectl.c b/src/home/homectl.c
index acceadf176..23570ca31b 100644
--- a/src/home/homectl.c
+++ b/src/home/homectl.c
@@ -760,16 +760,16 @@ static int inspect_homes(int argc, char *argv[], void *userdata) {
         pager_open(arg_pager_flags);
 
         char **args = strv_skip(argv, 1);
-        if (strv_isempty(args)) {
+        if (args) {
+                STRV_FOREACH(arg, args)
+                        RET_GATHER(r, inspect_home(bus, *arg));
+                return r;
+        } else {
                 _cleanup_free_ char *myself = getusername_malloc();
                 if (!myself)
                         return log_oom();
 
                 return inspect_home(bus, myself);
-        } else {
-                STRV_FOREACH(arg, args)
-                        RET_GATHER(r, inspect_home(bus, *arg));
-                return r;
         }
 }
 
@@ -818,17 +818,17 @@ static int authenticate_homes(int argc, char *argv[], void *userdata) {
         (void) polkit_agent_open_if_enabled(arg_transport, arg_ask_password);
 
         char **args = strv_skip(argv, 1);
-        if (strv_isempty(args)) {
+        if (args) {
+                STRV_FOREACH(arg, args)
+                        RET_GATHER(r, authenticate_home(bus, *arg));
+
+                return r;
+        } else {
                 _cleanup_free_ char *myself = getusername_malloc();
                 if (!myself)
                         return log_oom();
 
                 return authenticate_home(bus, myself);
-        } else {
-                STRV_FOREACH(arg, args)
-                        RET_GATHER(r, authenticate_home(bus, *arg));
-
-                return r;
         }
 }
 
diff --git a/src/test/test-strv.c b/src/test/test-strv.c
index d64cc55c5b..028765665b 100644
--- a/src/test/test-strv.c
+++ b/src/test/test-strv.c
@@ -1004,17 +1004,21 @@ TEST(strv_skip) {
         test_strv_skip_one(STRV_MAKE("foo", "bar", "baz"), 0, STRV_MAKE("foo", "bar", "baz"));
         test_strv_skip_one(STRV_MAKE("foo", "bar", "baz"), 1, STRV_MAKE("bar", "baz"));
         test_strv_skip_one(STRV_MAKE("foo", "bar", "baz"), 2, STRV_MAKE("baz"));
-        test_strv_skip_one(STRV_MAKE("foo", "bar", "baz"), 3, STRV_MAKE(NULL));
-        test_strv_skip_one(STRV_MAKE("foo", "bar", "baz"), 4, STRV_MAKE(NULL));
-        test_strv_skip_one(STRV_MAKE("foo", "bar", "baz"), 55, STRV_MAKE(NULL));
+        test_strv_skip_one(STRV_MAKE("foo", "bar", "baz"), 3, NULL);
+        test_strv_skip_one(STRV_MAKE("foo", "bar", "baz"), 4, NULL);
+        test_strv_skip_one(STRV_MAKE("foo", "bar", "baz"), 55, NULL);
 
         test_strv_skip_one(STRV_MAKE("quux"), 0, STRV_MAKE("quux"));
-        test_strv_skip_one(STRV_MAKE("quux"), 1, STRV_MAKE(NULL));
-        test_strv_skip_one(STRV_MAKE("quux"), 55, STRV_MAKE(NULL));
+        test_strv_skip_one(STRV_MAKE("quux"), 1, NULL);
+        test_strv_skip_one(STRV_MAKE("quux"), 55, NULL);
 
-        test_strv_skip_one(STRV_MAKE(NULL), 0, STRV_MAKE(NULL));
-        test_strv_skip_one(STRV_MAKE(NULL), 1, STRV_MAKE(NULL));
-        test_strv_skip_one(STRV_MAKE(NULL), 55, STRV_MAKE(NULL));
+        test_strv_skip_one(STRV_MAKE(NULL), 0, NULL);
+        test_strv_skip_one(STRV_MAKE(NULL), 1, NULL);
+        test_strv_skip_one(STRV_MAKE(NULL), 55, NULL);
+
+        test_strv_skip_one(NULL, 0, NULL);
+        test_strv_skip_one(NULL, 1, NULL);
+        test_strv_skip_one(NULL, 55, NULL);
 }
 
 TEST(strv_extend_n) {