strv: multiple cleanups

- turn strv_merge into strv_extend_strv.
   appending strv b to the end of strv a instead of creating a new strv
- strv_append: remove in favor of strv_extend and strv_push.
- strv_remove: write slightly more elegant
- strv_remove_prefix: remove unused function
- strv_overlap: use strv_contains
- strv_printf: STRV_FOREACH handles NULL correctly

8 files changed, 97 insertions, 279 deletions

diff --git a/src/core/main.c b/src/core/main.c
index d052c8debc..6db42991e8 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -601,15 +601,12 @@ static int config_parse_join_controllers(const char *unit,
                                 if (strv_overlap(*a, l)) {
                                         char **c;
 
-                                        c = strv_merge(*a, l);
-                                        if (!c) {
+                                        if (strv_extend_strv(&l, *a) < 0) {
                                                 strv_free(l);
                                                 strv_free_free(t);
                                                 return log_oom();
                                         }
 
-                                        strv_free(l);
-                                        l = c;
                                 } else {
                                         char **c;
 
@@ -1853,10 +1850,11 @@ finish:
                         shutdown_verb,
                         NULL
                 };
-                char **env_block;
+                _cleanup_strv_free_ char **env_block = NULL;
+                env_block = strv_copy(environ);
 
                 if (arm_reboot_watchdog && arg_shutdown_watchdog > 0) {
-                        char e[32];
+                        char *e;
 
                         /* If we reboot let's set the shutdown
                          * watchdog and tell the shutdown binary to
@@ -1864,15 +1862,11 @@ finish:
                         watchdog_set_timeout(&arg_shutdown_watchdog);
                         watchdog_close(false);
 
-                        /* Tell the binary how often to ping */
-                        snprintf(e, sizeof(e), "WATCHDOG_USEC="USEC_FMT, arg_shutdown_watchdog);
-                        char_array_0(e);
-
-                        env_block = strv_append(environ, e);
-                } else {
-                        env_block = strv_copy(environ);
+                        /* Tell the binary how often to ping, ignore failure */
+                        if (asprintf(&e, "WATCHDOG_USEC="USEC_FMT, arg_shutdown_watchdog) > 0)
+                                strv_push(&env_block, e);
+                } else
                         watchdog_close(true);
-                }
 
                 /* Avoid the creation of new processes forked by the
                  * kernel; at this point, we will not listen to the
@@ -1881,7 +1875,6 @@ finish:
                         cg_uninstall_release_agent(SYSTEMD_CGROUP_CONTROLLER);
 
                 execve(SYSTEMD_SHUTDOWN_BINARY_PATH, (char **) command_line, env_block);
-                free(env_block);
                 log_error("Failed to execute shutdown binary, freezing: %m");
         }
 
diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c
index 5d141a8935..01987f2e3c 100644
--- a/src/modules-load/modules-load.c
+++ b/src/modules-load/modules-load.c
@@ -64,13 +64,9 @@ static int add_modules(const char *p) {
         if (!k)
                 return log_oom();
 
-        t = strv_merge(arg_proc_cmdline_modules, k);
-        if (!t)
+        if (strv_extend_strv(&arg_proc_cmdline_modules, k) < 0)
                 return log_oom();
 
-        strv_free(arg_proc_cmdline_modules);
-        arg_proc_cmdline_modules = t;
-
         return 0;
 }
 
diff --git a/src/shared/path-lookup.c b/src/shared/path-lookup.c
index 1a47ea9ce7..e2ca9420d9 100644
--- a/src/shared/path-lookup.c
+++ b/src/shared/path-lookup.c
@@ -90,9 +90,9 @@ static char** user_dirs(
         };
 
         const char *home, *e;
-        char *config_home = NULL, *data_home = NULL;
-        char **config_dirs = NULL, **data_dirs = NULL;
-        char **r = NULL, **t;
+        _cleanup_free_ char *config_home = NULL, *data_home = NULL;
+        _cleanup_strv_free_ char **config_dirs = NULL, **data_dirs = NULL;
+        char **r = NULL;
 
         /* Implement the mechanisms defined in
          *
@@ -150,89 +150,48 @@ static char** user_dirs(
                 goto fail;
 
         /* Now merge everything we found. */
-        if (generator_early) {
-                t = strv_append(r, generator_early);
-                if (!t)
+        if (generator_early)
+                if (strv_extend(&r, generator_early) < 0)
                         goto fail;
-                strv_free(r);
-                r = t;
-        }
 
-        if (config_home) {
-                t = strv_append(r, config_home);
-                if (!t)
+        if (config_home)
+                if (strv_extend(&r, config_home) < 0)
                         goto fail;
-                strv_free(r);
-                r = t;
-        }
 
-        if (!strv_isempty(config_dirs)) {
-                t = strv_merge_concat(r, config_dirs, "/systemd/user");
-                if (!t)
-                        goto finish;
-                strv_free(r);
-                r = t;
-        }
+        if (!strv_isempty(config_dirs))
+                if (strv_extend_strv_concat(&r, config_dirs, "/systemd/user") < 0)
+                        goto fail;
 
-        t = strv_merge(r, (char**) config_unit_paths);
-        if (!t)
+        if (strv_extend_strv(&r, (char**) config_unit_paths) < 0)
                 goto fail;
-        strv_free(r);
-        r = t;
 
-        if (generator) {
-                t = strv_append(r, generator);
-                if (!t)
+        if (generator)
+                if (strv_extend(&r, generator) < 0)
                         goto fail;
-                strv_free(r);
-                r = t;
-        }
 
-        if (data_home) {
-                t = strv_append(r, data_home);
-                if (!t)
+        if (data_home)
+                if (strv_extend(&r, data_home) < 0)
                         goto fail;
-                strv_free(r);
-                r = t;
-        }
 
-        if (!strv_isempty(data_dirs)) {
-                t = strv_merge_concat(r, data_dirs, "/systemd/user");
-                if (!t)
+        if (!strv_isempty(data_dirs))
+                if (strv_extend_strv_concat(&r, data_dirs, "/systemd/user") < 0)
                         goto fail;
-                strv_free(r);
-                r = t;
-        }
 
-        t = strv_merge(r, (char**) data_unit_paths);
-        if (!t)
+        if (strv_extend_strv(&r, (char**) data_unit_paths) < 0)
                 goto fail;
-        strv_free(r);
-        r = t;
 
-        if (generator_late) {
-                t = strv_append(r, generator_late);
-                if (!t)
+        if (generator_late)
+                if (strv_extend(&r, generator_late) < 0)
                         goto fail;
-                strv_free(r);
-                r = t;
-        }
 
         if (!path_strv_make_absolute_cwd(r))
                 goto fail;
 
-finish:
-        free(config_home);
-        strv_free(config_dirs);
-        free(data_home);
-        strv_free(data_dirs);
-
         return r;
 
 fail:
         strv_free(r);
-        r = NULL;
-        goto finish;
+        return NULL;
 }
 
 int lookup_paths_init(
diff --git a/src/shared/strv.c b/src/shared/strv.c
index 607c221ae6..13deba7be7 100644
--- a/src/shared/strv.c
+++ b/src/shared/strv.c
@@ -166,72 +166,38 @@ char **strv_new(const char *x, ...) {
         return r;
 }
 
-char **strv_merge(char **a, char **b) {
-        char **r, **k;
-
-        if (!a)
-                return strv_copy(b);
-
-        if (!b)
-                return strv_copy(a);
-
-        r = new(char*, strv_length(a) + strv_length(b) + 1);
-        if (!r)
-                return NULL;
-
-        for (k = r; *a; k++, a++) {
-                *k = strdup(*a);
-                if (!*k)
-                        goto fail;
-        }
+int strv_extend_strv(char ***a, char **b) {
+        int r;
+        char **s;
 
-        for (; *b; k++, b++) {
-                *k = strdup(*b);
-                if (!*k)
-                        goto fail;
+        STRV_FOREACH(s, b) {
+                r = strv_extend(a, *s);
+                if (r < 0)
+                        return r;
         }
 
-        *k = NULL;
-        return r;
-
-fail:
-        strv_free(r);
-        return NULL;
+        return 0;
 }
 
-char **strv_merge_concat(char **a, char **b, const char *suffix) {
-        char **r, **k;
-
-        /* Like strv_merge(), but appends suffix to all strings in b, before adding */
+int strv_extend_strv_concat(char ***a, char **b, const char *suffix) {
+        int r;
+        char **s;
 
-        if (!b)
-                return strv_copy(a);
+        STRV_FOREACH(s, b) {
+                char *v;
 
-        r = new(char*, strv_length(a) + strv_length(b) + 1);
-        if (!r)
-                return NULL;
+                v = strappend(*s, suffix);
+                if (!v)
+                        return -ENOMEM;
 
-        k = r;
-        if (a)
-                for (; *a; k++, a++) {
-                        *k = strdup(*a);
-                        if (!*k)
-                                goto fail;
+                r = strv_push(a, v);
+                if (r < 0) {
+                        free(v);
+                        return r;
                 }
-
-        for (; *b; k++, b++) {
-                *k = strappend(*b, suffix);
-                if (!*k)
-                        goto fail;
         }
 
-        *k = NULL;
-        return r;
-
-fail:
-        strv_free(r);
-        return NULL;
-
+        return 0;
 }
 
 char **strv_split(const char *s, const char *separator) {
@@ -393,37 +359,6 @@ char *strv_join_quoted(char **l) {
         return NULL;
 }
 
-char **strv_append(char **l, const char *s) {
-        char **r, **k;
-
-        if (!l)
-                return strv_new(s, NULL);
-
-        if (!s)
-                return strv_copy(l);
-
-        r = new(char*, strv_length(l)+2);
-        if (!r)
-                return NULL;
-
-        for (k = r; *l; k++, l++) {
-                *k = strdup(*l);
-                if (!*k)
-                        goto fail;
-        }
-
-        k[0] = strdup(s);
-        if (!k[0])
-                goto fail;
-
-        k[1] = NULL;
-        return r;
-
-fail:
-        strv_free(r);
-        return NULL;
-}
-
 int strv_push(char ***l, char *value) {
         char **c;
         unsigned n;
@@ -484,40 +419,11 @@ char **strv_remove(char **l, const char *s) {
         /* Drops every occurrence of s in the string list, edits
          * in-place. */
 
-        for (f = t = l; *f; f++) {
-
-                if (streq(*f, s)) {
-                        free(*f);
-                        continue;
-                }
-
-                *(t++) = *f;
-        }
-
-        *t = NULL;
-        return l;
-}
-
-char **strv_remove_prefix(char **l, const char *s) {
-        char **f, **t;
-
-        if (!l)
-                return NULL;
-
-        assert(s);
-
-        /* Drops every occurrence of a string prefixed with s in the
-         * string list, edits in-place. */
-
-        for (f = t = l; *f; f++) {
-
-                if (startswith(*f, s)) {
+        for (f = t = l; *f; f++)
+                if (streq(*f, s))
                         free(*f);
-                        continue;
-                }
-
-                *(t++) = *f;
-        }
+                else
+                        *(t++) = *f;
 
         *t = NULL;
         return l;
@@ -586,14 +492,11 @@ char **strv_split_nulstr(const char *s) {
 }
 
 bool strv_overlap(char **a, char **b) {
-        char **i, **j;
+        char **i;
 
-        STRV_FOREACH(i, a) {
-                STRV_FOREACH(j, b) {
-                        if (streq(*i, *j))
-                                return true;
-                }
-        }
+        STRV_FOREACH(i, a)
+                if (strv_contains(b, *i))
+                        return true;
 
         return false;
 }
@@ -616,9 +519,6 @@ char **strv_sort(char **l) {
 void strv_print(char **l) {
         char **s;
 
-        if (!l)
-                return;
-
         STRV_FOREACH(s, l)
                 puts(*s);
 }
diff --git a/src/shared/strv.h b/src/shared/strv.h
index daf9ad0d0b..715bc5419c 100644
--- a/src/shared/strv.h
+++ b/src/shared/strv.h
@@ -36,14 +36,12 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(char**, strv_free);
 char **strv_copy(char * const *l);
 unsigned strv_length(char * const *l) _pure_;
 
-char **strv_merge(char **a, char **b);
-char **strv_merge_concat(char **a, char **b, const char *suffix);
-char **strv_append(char **l, const char *s);
+int strv_extend_strv(char ***a, char **b);
+int strv_extend_strv_concat(char ***a, char **b, const char *suffix);
 int strv_extend(char ***l, const char *value);
 int strv_push(char ***l, char *value);
 
 char **strv_remove(char **l, const char *s);
-char **strv_remove_prefix(char **l, const char *s);
 char **strv_uniq(char **l);
 
 #define strv_contains(l, s) (!!strv_find((l), (s)))
diff --git a/src/sysctl/sysctl.c b/src/sysctl/sysctl.c
index 7ebe1e2df5..57112a7ef9 100644
--- a/src/sysctl/sysctl.c
+++ b/src/sysctl/sysctl.c
@@ -251,13 +251,9 @@ static int parse_argv(int argc, char *argv[]) {
                                 if (*p == '.')
                                         *p = '/';
 
-                        l = strv_append(arg_prefixes, optarg);
-                        if (!l)
+                        if (strv_extend(&arg_prefixes, optarg) < 0)
                                 return log_oom();
 
-                        strv_free(arg_prefixes);
-                        arg_prefixes = l;
-
                         break;
                 }
 
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 01a4489f4d..dd95df14e7 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -1327,7 +1327,7 @@ static int list_dependencies_one(
                 char ***units,
                 unsigned int branches) {
 
-        _cleanup_strv_free_ char **deps = NULL, **u;
+        _cleanup_strv_free_ char **deps = NULL;
         char **c;
         int r = 0;
 
@@ -1335,8 +1335,8 @@ static int list_dependencies_one(
         assert(name);
         assert(units);
 
-        u = strv_append(*units, name);
-        if (!u)
+        r = strv_extend(units, name);
+        if (r < 0)
                 return log_oom();
 
         r = list_dependencies_get_dependencies(bus, name, &deps);
@@ -1348,7 +1348,7 @@ static int list_dependencies_one(
         STRV_FOREACH(c, deps) {
                 int state;
 
-                if (strv_contains(u, *c)) {
+                if (strv_contains(*units, *c)) {
                         if (!arg_plain) {
                                 r = list_dependencies_print("...", level + 1, (branches << 1) | (c[1] == NULL ? 0 : 1), 1);
                                 if (r < 0)
@@ -1368,17 +1368,14 @@ static int list_dependencies_one(
                         return r;
 
                 if (arg_all || unit_name_to_type(*c) == UNIT_TARGET) {
-                       r = list_dependencies_one(bus, *c, level + 1, &u, (branches << 1) | (c[1] == NULL ? 0 : 1));
+                       r = list_dependencies_one(bus, *c, level + 1, units, (branches << 1) | (c[1] == NULL ? 0 : 1));
                        if (r < 0)
                                return r;
                 }
         }
 
-        if (arg_plain) {
-                strv_free(*units);
-                *units = u;
-                u = NULL;
-        }
+        if (!arg_plain)
+                strv_remove(*units, name);
 
         return 0;
 }
diff --git a/src/test/test-strv.c b/src/test/test-strv.c
index d58d99c99f..c8c5fc97a8 100644
--- a/src/test/test-strv.c
+++ b/src/test/test-strv.c
@@ -214,22 +214,6 @@ static void test_strv_split_nulstr(void) {
         assert_se(streq(l[3], "str3"));
 }
 
-static void test_strv_remove_prefix(void) {
-        unsigned i = 0;
-        char **s;
-        _cleanup_strv_free_ char **l = NULL;
-
-        l = strv_new("_one", "_two", "_three", NULL);
-        assert(l);
-
-        l = strv_remove_prefix(l, "_");
-        assert(l);
-
-        STRV_FOREACH(s, l) {
-                assert_se(streq(*s, input_table_multiple[i++]));
-        }
-}
-
 static void test_strv_parse_nulstr(void) {
         _cleanup_strv_free_ char **l = NULL;
         const char nulstr[] = "fuck\0fuck2\0fuck3\0\0fuck5\0\0xxx";
@@ -289,58 +273,54 @@ static void test_strv_sort(void) {
         assert_se(streq(input_table[4], "durian"));
 }
 
-static void test_strv_merge_concat(void) {
-         _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL;
+static void test_strv_extend_strv_concat(void) {
+         _cleanup_strv_free_ char **a = NULL, **b = NULL;
 
         a = strv_new("without", "suffix", NULL);
         b = strv_new("with", "suffix", NULL);
         assert_se(a);
         assert_se(b);
 
-        c = strv_merge_concat(a, b, "_suffix");
-        assert_se(c);
+        assert_se(strv_extend_strv_concat(&a, b, "_suffix") >= 0);
 
-        assert_se(streq(c[0], "without"));
-        assert_se(streq(c[1], "suffix"));
-        assert_se(streq(c[2], "with_suffix"));
-        assert_se(streq(c[3], "suffix_suffix"));
+        assert_se(streq(a[0], "without"));
+        assert_se(streq(a[1], "suffix"));
+        assert_se(streq(a[2], "with_suffix"));
+        assert_se(streq(a[3], "suffix_suffix"));
 }
 
-static void test_strv_merge(void) {
-         _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL;
+static void test_strv_extend_strv(void) {
+         _cleanup_strv_free_ char **a = NULL, **b = NULL;
 
         a = strv_new("abc", "def", "ghi", NULL);
         b = strv_new("jkl", "mno", "pqr", NULL);
         assert_se(a);
         assert_se(b);
 
-        c = strv_merge(a, b);
-        assert_se(c);
+        assert_se(strv_extend_strv(&a, b) >= 0);
 
-        assert_se(streq(c[0], "abc"));
-        assert_se(streq(c[1], "def"));
-        assert_se(streq(c[2], "ghi"));
-        assert_se(streq(c[3], "jkl"));
-        assert_se(streq(c[4], "mno"));
-        assert_se(streq(c[5], "pqr"));
+        assert_se(streq(a[0], "abc"));
+        assert_se(streq(a[1], "def"));
+        assert_se(streq(a[2], "ghi"));
+        assert_se(streq(a[3], "jkl"));
+        assert_se(streq(a[4], "mno"));
+        assert_se(streq(a[5], "pqr"));
 
-        assert_se(strv_length(c) == 6);
+        assert_se(strv_length(a) == 6);
 }
 
-static void test_strv_append(void) {
-        _cleanup_strv_free_ char **a = NULL, **b = NULL, **c = NULL;
+static void test_strv_extend(void) {
+        _cleanup_strv_free_ char **a = NULL, **b = NULL;
 
         a = strv_new("test", "test1", NULL);
         assert_se(a);
-        b = strv_append(a, "test2");
-        c = strv_append(NULL, "test3");
-        assert_se(b);
-        assert_se(c);
+        assert_se(strv_extend(&a, "test2") >= 0);
+        assert_se(strv_extend(&b, "test3") >= 0);
 
-        assert_se(streq(b[0], "test"));
-        assert_se(streq(b[1], "test1"));
-        assert_se(streq(b[2], "test2"));
-        assert_se(streq(c[0], "test3"));
+        assert_se(streq(a[0], "test"));
+        assert_se(streq(a[1], "test1"));
+        assert_se(streq(a[2], "test2"));
+        assert_se(streq(b[0], "test3"));
 }
 
 static void test_strv_foreach(void) {
@@ -436,12 +416,11 @@ int main(int argc, char *argv[]) {
         test_strv_split_newlines();
         test_strv_split_nulstr();
         test_strv_parse_nulstr();
-        test_strv_remove_prefix();
         test_strv_overlap();
         test_strv_sort();
-        test_strv_merge();
-        test_strv_merge_concat();
-        test_strv_append();
+        test_strv_extend_strv();
+        test_strv_extend_strv_concat();
+        test_strv_extend();
         test_strv_from_stdarg_alloca();
 
         return 0;