tree-wide: replace strv_split_full() with strv_split_extract() everywhere

Behaviour is not identical, as shown by the tests in test-strv.
The combination of EXTRACT_UNQUOTE without EXTRACT_RELAX only appears in
the test, so it doesn't seem particularly important. OTOH, the difference
in handling of squished parameters could make a difference. New behaviour
is what both bash and python do, so I think we can ignore this corner case.

This change has the following advantages:
- the duplication of code paths that do a very similar thing is removed
- extract_one_word() / strv_split_extract() return a proper error code.

diff --git a/src/basic/strv.c b/src/basic/strv.c
index a172ca2fe9174b99b019e6e9fa0d08b86288277d..a5916926ff3b90b805cd74f072bf3d310c12f9a5 100644 (file)
--- a/src/basic/strv.c
+++ b/src/basic/strv.c
@@ -256,44 +256,6 @@ int strv_extend_strv_concat(char ***a, char * const *b, const char *suffix) {
         return 0;
 }
 
-char **strv_split_full(const char *s, const char *separator, SplitFlags flags) {
-        const char *word, *state;
-        size_t l;
-        size_t n, i;
-        char **r;
-
-        assert(s);
-
-        if (!separator)
-                separator = WHITESPACE;
-
-        s += strspn(s, separator);
-        if (isempty(s))
-                return new0(char*, 1);
-
-        n = 0;
-        _FOREACH_WORD(word, l, s, separator, flags, state)
-                n++;
-
-        r = new(char*, n+1);
-        if (!r)
-                return NULL;
-
-        i = 0;
-        _FOREACH_WORD(word, l, s, separator, flags, state) {
-                r[i] = strndup(word, l);
-                if (!r[i]) {
-                        strv_free(r);
-                        return NULL;
-                }
-
-                i++;
-        }
-
-        r[i] = NULL;
-        return r;
-}
-
 char **strv_split_newlines(const char *s) {
         char **l;
         size_t n;

diff --git a/src/basic/strv.h b/src/basic/strv.h
index bc0b04b56b3262a580a68b7647132d86c65ac8b7..9e15820f28f881a0fb46f454a525ceaf9f8919b1 100644 (file)
--- a/src/basic/strv.h
+++ b/src/basic/strv.h
@@ -72,13 +72,19 @@ static inline bool strv_isempty(char * const *l) {
         return !l || !*l;
 }
 
-char **strv_split_full(const char *s, const char *separator, SplitFlags flags);
-static inline char **strv_split(const char *s, const char *separator) {
-        return strv_split_full(s, separator, 0);
-}
 char **strv_split_newlines(const char *s);
 
 int strv_split_extract(char ***t, const char *s, const char *separators, ExtractFlags flags);
+static inline char **strv_split(const char *s, const char *separators) {
+        char **ret;
+        int r;
+
+        r = strv_split_extract(&ret, s, separators, 0);
+        if (r < 0)
+                return NULL;
+
+        return ret;
+}
 
 /* Given a string containing white-space separated tuples of words themselves separated by ':',
  * returns a vector of strings. If the second element in a tuple is missing, the corresponding

diff --git a/src/test/test-strv.c b/src/test/test-strv.c
index fda5948f4998ead8142f4284b9f1cb0977eab33c..2e9922fa5ec33132ed59a9cba9418fb7b49fa256 100644 (file)
--- a/src/test/test-strv.c
+++ b/src/test/test-strv.c
@@ -100,6 +100,12 @@ static const char* const input_table_quoted[] = {
         NULL,
 };
 
+static const char* const input_table_quoted_joined[] = {
+        "one",
+        "  two\t three " " four  five",
+        NULL,
+};
+
 static const char* const input_table_one[] = {
         "one",
         NULL,
@@ -281,47 +287,39 @@ static void test_strv_split(void) {
 
         strv_free_erase(l);
 
-        l = strv_split_full("    one    two\t three", NULL, 0);
-        assert_se(l);
+        assert_se(strv_split_extract(&l, "    one    two\t three", NULL, 0) == 3);
         assert_se(strv_equal(l, (char**) input_table_multiple));
 
         strv_free_erase(l);
 
-        l = strv_split_full("    'one'  \"  two\t three \" ' four  five'", NULL, SPLIT_QUOTES);
-        assert_se(l);
+        assert_se(strv_split_extract(&l, "    'one'  \"  two\t three \" ' four  five'", NULL, EXTRACT_UNQUOTE) == 3);
         assert_se(strv_equal(l, (char**) input_table_quoted));
 
-        strv_free_erase(l);
+        l = strv_free_erase(l);
 
-        /* missing last quote ignores the last element. */
-        l = strv_split_full("    'one'  \"  two\t three \" ' four  five'  ' ignored element ", NULL, SPLIT_QUOTES);
-        assert_se(l);
-        assert_se(strv_equal(l, (char**) input_table_quoted));
+        /* missing last quote causes extraction to fail. */
+        assert_se(strv_split_extract(&l, "    'one'  \"  two\t three \" ' four  five", NULL, EXTRACT_UNQUOTE) == -EINVAL);
+        assert_se(!l);
 
-        strv_free_erase(l);
-
-        /* missing last quote, but the last element is _not_ ignored with SPLIT_RELAX. */
-        l = strv_split_full("    'one'  \"  two\t three \" ' four  five", NULL, SPLIT_QUOTES | SPLIT_RELAX);
-        assert_se(l);
+        /* missing last quote, but the last element is _not_ ignored with EXTRACT_RELAX. */
+        assert_se(strv_split_extract(&l, "    'one'  \"  two\t three \" ' four  five", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 3);
         assert_se(strv_equal(l, (char**) input_table_quoted));
 
-        strv_free_erase(l);
+        l = strv_free_erase(l);
 
-        /* missing separator between */
-        l = strv_split_full("    'one'  \"  two\t three \"' four  five'", NULL, SPLIT_QUOTES | SPLIT_RELAX);
-        assert_se(l);
-        assert_se(strv_equal(l, (char**) input_table_quoted));
+        /* missing separator between items */
+        assert_se(strv_split_extract(&l, "    'one'  \"  two\t three \"' four  five'", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 2);
+        assert_se(strv_equal(l, (char**) input_table_quoted_joined));
 
-        strv_free_erase(l);
+        l = strv_free_erase(l);
 
-        l = strv_split_full("    'one'  \"  two\t three \"' four  five", NULL, SPLIT_QUOTES | SPLIT_RELAX);
-        assert_se(l);
-        assert_se(strv_equal(l, (char**) input_table_quoted));
+        assert_se(strv_split_extract(&l, "    'one'  \"  two\t three \"' four  five", NULL,
+                                     EXTRACT_UNQUOTE | EXTRACT_RELAX | EXTRACT_CUNESCAPE_RELAX) == 2);
+        assert_se(strv_equal(l, (char**) input_table_quoted_joined));
 
-        strv_free_erase(l);
+        l = strv_free_erase(l);
 
-        l = strv_split_full("\\", NULL, SPLIT_QUOTES | SPLIT_RELAX);
-        assert_se(l);
+        assert_se(strv_split_extract(&l, "\\", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX | EXTRACT_CUNESCAPE_RELAX) == 1);
         assert_se(strv_equal(l, STRV_MAKE("\\")));
 }
 
@@ -333,59 +331,58 @@ static void test_strv_split_empty(void) {
         l = strv_split("", WHITESPACE);
         assert_se(l);
         assert_se(strv_isempty(l));
+        l = strv_free(l);
 
-        strv_free(l);
-        l = strv_split("", NULL);
-        assert_se(l);
+        assert_se(l = strv_split("", NULL));
         assert_se(strv_isempty(l));
+        l = strv_free(l);
 
-        strv_free(l);
-        l = strv_split_full("", NULL, 0);
+        assert_se(strv_split_extract(&l, "", NULL, 0) == 0);
         assert_se(l);
         assert_se(strv_isempty(l));
+        l = strv_free(l);
 
-        strv_free(l);
-        l = strv_split_full("", NULL, SPLIT_QUOTES);
+        assert_se(strv_split_extract(&l, "", NULL, EXTRACT_UNQUOTE) == 0);
         assert_se(l);
         assert_se(strv_isempty(l));
+        l = strv_free(l);
 
-        strv_free(l);
-        l = strv_split_full("", WHITESPACE, SPLIT_QUOTES);
+        assert_se(strv_split_extract(&l, "", WHITESPACE, EXTRACT_UNQUOTE) == 0);
         assert_se(l);
         assert_se(strv_isempty(l));
+        l = strv_free(l);
 
-        strv_free(l);
-        l = strv_split_full("", WHITESPACE, SPLIT_QUOTES | SPLIT_RELAX);
+        assert_se(strv_split_extract(&l, "", WHITESPACE, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 0);
         assert_se(l);
         assert_se(strv_isempty(l));
-
         strv_free(l);
+
         l = strv_split("    ", WHITESPACE);
         assert_se(l);
         assert_se(strv_isempty(l));
-
         strv_free(l);
+
         l = strv_split("    ", NULL);
         assert_se(l);
         assert_se(strv_isempty(l));
+        l = strv_free(l);
 
-        strv_free(l);
-        l = strv_split_full("    ", NULL, 0);
+        assert_se(strv_split_extract(&l, "    ", NULL, 0) == 0);
         assert_se(l);
         assert_se(strv_isempty(l));
+        l = strv_free(l);
 
-        strv_free(l);
-        l = strv_split_full("    ", WHITESPACE, SPLIT_QUOTES);
+        assert_se(strv_split_extract(&l, "    ", WHITESPACE, EXTRACT_UNQUOTE) == 0);
         assert_se(l);
         assert_se(strv_isempty(l));
+        l = strv_free(l);
 
-        strv_free(l);
-        l = strv_split_full("    ", NULL, SPLIT_QUOTES);
+        assert_se(strv_split_extract(&l, "    ", NULL, EXTRACT_UNQUOTE) == 0);
         assert_se(l);
         assert_se(strv_isempty(l));
+        l = strv_free(l);
 
-        strv_free(l);
-        l = strv_split_full("    ", NULL, SPLIT_QUOTES | SPLIT_RELAX);
+        assert_se(strv_split_extract(&l, "    ", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 0);
         assert_se(l);
         assert_se(strv_isempty(l));
 }

diff --git a/src/udev/udev-builtin.c b/src/udev/udev-builtin.c
index b41fbfc39a9436de1d89ed387e30b6889eef16d5..ac443d66e63665659fb68971272e06cda8309696 100644 (file)
--- a/src/udev/udev-builtin.c
+++ b/src/udev/udev-builtin.c
@@ -109,6 +109,7 @@ UdevBuiltinCommand udev_builtin_lookup(const char *command) {
 
 int udev_builtin_run(sd_device *dev, UdevBuiltinCommand cmd, const char *command, bool test) {
         _cleanup_strv_free_ char **argv = NULL;
+        int r;
 
         assert(dev);
         assert(cmd >= 0 && cmd < _UDEV_BUILTIN_MAX);
@@ -117,9 +118,10 @@ int udev_builtin_run(sd_device *dev, UdevBuiltinCommand cmd, const char *command
         if (!builtins[cmd])
                 return -EOPNOTSUPP;
 
-        argv = strv_split_full(command, NULL, SPLIT_QUOTES | SPLIT_RELAX);
-        if (!argv)
-                return -ENOMEM;
+        r = strv_split_extract(&argv, command, NULL,
+                               EXTRACT_UNQUOTE | EXTRACT_RELAX | EXTRACT_RETAIN_ESCAPE);
+        if (r < 0)
+                return r;
 
         /* we need '0' here to reset the internal state */
         optind = 0;

diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index e1daac21ed7034c7759a0bd77fefc263e7a8b6fd..03e80b724dd983a8b5a9cb3a207d7a6166205d82 100644 (file)
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -747,9 +747,9 @@ int udev_event_spawn(UdevEvent *event,
                         return log_device_error_errno(event->dev, errno,
                                                       "Failed to create pipe for command '%s': %m", cmd);
 
-        argv = strv_split_full(cmd, NULL, SPLIT_QUOTES|SPLIT_RELAX);
-        if (!argv)
-                return log_oom();
+        r = strv_split_extract(&argv, cmd, NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX | EXTRACT_RETAIN_ESCAPE);
+        if (r < 0)
+                return log_device_error_errno(event->dev, r, "Failed to split command: %m");
 
         if (isempty(argv[0]))
                 return log_device_error_errno(event->dev, SYNTHETIC_ERRNO(EINVAL),

diff --git a/src/xdg-autostart-generator/xdg-autostart-service.c b/src/xdg-autostart-generator/xdg-autostart-service.c
index 4a30f9e43354fe5e8016f0edd15534c26bf11beb..efd2ae3ec62e5004bdba44e08607fb7c2bfa24af 100644 (file)
--- a/src/xdg-autostart-generator/xdg-autostart-service.c
+++ b/src/xdg-autostart-generator/xdg-autostart-service.c
@@ -385,9 +385,9 @@ int xdg_autostart_format_exec_start(
          * NOTE: Technically, XDG only specifies " as quotes, while this also
          *       accepts '.
          */
-        exec_split = strv_split_full(exec, WHITESPACE, SPLIT_QUOTES | SPLIT_RELAX);
-        if (!exec_split)
-                return -ENOMEM;
+        r = strv_split_extract(&exec_split, exec, NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX);
+        if (r < 0)
+                return r;
 
         if (strv_isempty(exec_split))
                 return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), "Exec line is empty");