From 0176728a732ab025211d99486e3863f7a39df0bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Jul 2020 11:31:03 +0200 Subject: [PATCH 01/24] test-string-util,test-extract-word: add log headers --- src/test/test-extract-word.c | 6 ++++ src/test/test-string-util.c | 70 +++++++++++++++++++++++++++++++----- 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/test/test-extract-word.c b/src/test/test-extract-word.c index c71e4d32bf..cc1f29385f 100644 --- a/src/test/test-extract-word.c +++ b/src/test/test-extract-word.c @@ -11,6 +11,8 @@ static void test_extract_first_word(void) { const char *p, *original; char *t; + log_info("/* %s */", __func__); + p = original = "foobar waldo"; assert_se(extract_first_word(&p, &t, NULL, 0) > 0); assert_se(streq(t, "foobar")); @@ -387,6 +389,8 @@ static void test_extract_first_word_and_warn(void) { const char *p, *original; char *t; + log_info("/* %s */", __func__); + p = original = "foobar waldo"; assert_se(extract_first_word_and_warn(&p, &t, NULL, 0, NULL, "fake", 1, original) > 0); assert_se(streq(t, "foobar")); @@ -531,6 +535,8 @@ static void test_extract_many_words(void) { const char *p, *original; char *a, *b, *c, *d, *e, *f; + log_info("/* %s */", __func__); + p = original = "foobar waldi piep"; assert_se(extract_many_words(&p, NULL, 0, &a, &b, &c, NULL) == 3); assert_se(isempty(p)); diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 1127d398a5..33c5f83575 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -10,8 +10,9 @@ #include "util.h" static void test_string_erase(void) { - char *x; + log_info("/* %s */", __func__); + char *x; x = strdupa(""); assert_se(streq(string_erase(x), "")); @@ -33,17 +34,17 @@ static void test_string_erase(void) { } static void test_free_and_strndup_one(char **t, const char *src, size_t l, const char *expected, bool change) { - int r; - log_debug("%s: \"%s\", \"%s\", %zd (expect \"%s\", %s)", __func__, strnull(*t), strnull(src), l, strnull(expected), yes_no(change)); - r = free_and_strndup(t, src, l); + int r = free_and_strndup(t, src, l); assert_se(streq_ptr(*t, expected)); assert_se(r == change); /* check that change occurs only when necessary */ } static void test_free_and_strndup(void) { + log_info("/* %s */", __func__); + static const struct test_case { const char *src; size_t len; @@ -91,6 +92,7 @@ static void test_free_and_strndup(void) { } static void test_ascii_strcasecmp_n(void) { + log_info("/* %s */", __func__); assert_se(ascii_strcasecmp_n("", "", 0) == 0); assert_se(ascii_strcasecmp_n("", "", 1) == 0); @@ -118,6 +120,8 @@ static void test_ascii_strcasecmp_n(void) { } static void test_ascii_strcasecmp_nn(void) { + log_info("/* %s */", __func__); + assert_se(ascii_strcasecmp_nn("", 0, "", 0) == 0); assert_se(ascii_strcasecmp_nn("", 0, "", 1) < 0); assert_se(ascii_strcasecmp_nn("", 1, "", 0) > 0); @@ -137,6 +141,8 @@ static void test_ascii_strcasecmp_nn(void) { static void test_cellescape(void) { char buf[40]; + log_info("/* %s */", __func__); + assert_se(streq(cellescape(buf, 1, ""), "")); assert_se(streq(cellescape(buf, 1, "1"), "")); assert_se(streq(cellescape(buf, 1, "12"), "")); @@ -216,19 +222,24 @@ static void test_cellescape(void) { } static void test_streq_ptr(void) { + log_info("/* %s */", __func__); + assert_se(streq_ptr(NULL, NULL)); assert_se(!streq_ptr("abc", "cdef")); } static void test_strstrip(void) { - char *r; - char input[] = " hello, waldo. "; + log_info("/* %s */", __func__); - r = strstrip(input); - assert_se(streq(r, "hello, waldo.")); + char *ret, input[] = " hello, waldo. "; + + ret = strstrip(input); + assert_se(streq(ret, "hello, waldo.")); } static void test_strextend(void) { + log_info("/* %s */", __func__); + _cleanup_free_ char *str = NULL; assert_se(strextend(&str, NULL)); @@ -240,6 +251,8 @@ static void test_strextend(void) { } static void test_strextend_with_separator(void) { + log_info("/* %s */", __func__); + _cleanup_free_ char *str = NULL; assert_se(strextend_with_separator(&str, NULL, NULL)); @@ -263,6 +276,8 @@ static void test_strextend_with_separator(void) { } static void test_strrep(void) { + log_info("/* %s */", __func__); + _cleanup_free_ char *one, *three, *zero; one = strrep("waldo", 1); three = strrep("waldo", 3); @@ -288,11 +303,15 @@ static void test_string_has_cc(void) { } static void test_ascii_strlower(void) { + log_info("/* %s */", __func__); + char a[] = "AabBcC Jk Ii Od LKJJJ kkd LK"; assert_se(streq(ascii_strlower(a), "aabbcc jk ii od lkjjj kkd lk")); } static void test_strshorten(void) { + log_info("/* %s */", __func__); + char s[] = "foobar"; assert_se(strlen(strshorten(s, 6)) == 6); @@ -302,6 +321,8 @@ static void test_strshorten(void) { } static void test_strjoina(void) { + log_info("/* %s */", __func__); + char *actual; actual = strjoina("", "foo", "bar"); @@ -359,6 +380,8 @@ static void test_strjoin(void) { } static void test_strcmp_ptr(void) { + log_info("/* %s */", __func__); + assert_se(strcmp_ptr(NULL, NULL) == 0); assert_se(strcmp_ptr("", NULL) > 0); assert_se(strcmp_ptr("foo", NULL) > 0); @@ -371,6 +394,8 @@ static void test_strcmp_ptr(void) { } static void test_foreach_word(void) { + log_info("/* %s */", __func__); + const char *word, *state; size_t l; int i = 0; @@ -412,6 +437,8 @@ static void check(const char *test, char** expected, bool trailing) { } static void test_foreach_word_quoted(void) { + log_info("/* %s */", __func__); + check("test a b c 'd' e '' '' hhh '' '' \"a b c\"", STRV_MAKE("test", "a", @@ -437,6 +464,8 @@ static void test_foreach_word_quoted(void) { } static void test_endswith(void) { + log_info("/* %s */", __func__); + assert_se(endswith("foobar", "bar")); assert_se(endswith("foobar", "")); assert_se(endswith("foobar", "foobar")); @@ -447,6 +476,8 @@ static void test_endswith(void) { } static void test_endswith_no_case(void) { + log_info("/* %s */", __func__); + assert_se(endswith_no_case("fooBAR", "bar")); assert_se(endswith_no_case("foobar", "")); assert_se(endswith_no_case("foobar", "FOOBAR")); @@ -457,6 +488,8 @@ static void test_endswith_no_case(void) { } static void test_delete_chars(void) { + log_info("/* %s */", __func__); + char *s, input[] = " hello, waldo. abc"; s = delete_chars(input, WHITESPACE); @@ -465,6 +498,7 @@ static void test_delete_chars(void) { } static void test_delete_trailing_chars(void) { + log_info("/* %s */", __func__); char *s, input1[] = " \n \r k \n \r ", @@ -489,6 +523,8 @@ static void test_delete_trailing_chars(void) { } static void test_delete_trailing_slashes(void) { + log_info("/* %s */", __func__); + char s1[] = "foobar//", s2[] = "foobar/", s3[] = "foobar", @@ -502,6 +538,8 @@ static void test_delete_trailing_slashes(void) { } static void test_skip_leading_chars(void) { + log_info("/* %s */", __func__); + char input1[] = " \n \r k \n \r ", input2[] = "kkkkthiskkkiskkkaktestkkk", input3[] = "abcdef"; @@ -514,11 +552,15 @@ static void test_skip_leading_chars(void) { } static void test_in_charset(void) { + log_info("/* %s */", __func__); + assert_se(in_charset("dddaaabbbcccc", "abcd")); assert_se(!in_charset("dddaaabbbcccc", "abc f")); } static void test_split_pair(void) { + log_info("/* %s */", __func__); + _cleanup_free_ char *a = NULL, *b = NULL; assert_se(split_pair("", "", &a, &b) == -EINVAL); @@ -541,6 +583,8 @@ static void test_split_pair(void) { } static void test_first_word(void) { + log_info("/* %s */", __func__); + assert_se(first_word("Hello", "")); assert_se(first_word("Hello", "Hello")); assert_se(first_word("Hello world", "Hello")); @@ -555,12 +599,16 @@ static void test_first_word(void) { } static void test_strlen_ptr(void) { + log_info("/* %s */", __func__); + assert_se(strlen_ptr("foo") == 3); assert_se(strlen_ptr("") == 0); assert_se(strlen_ptr(NULL) == 0); } static void test_memory_startswith(void) { + log_info("/* %s */", __func__); + assert_se(streq(memory_startswith("", 0, ""), "")); assert_se(streq(memory_startswith("", 1, ""), "")); assert_se(streq(memory_startswith("x", 2, ""), "x")); @@ -573,6 +621,8 @@ static void test_memory_startswith(void) { } static void test_memory_startswith_no_case(void) { + log_info("/* %s */", __func__); + assert_se(streq(memory_startswith_no_case("", 0, ""), "")); assert_se(streq(memory_startswith_no_case("", 1, ""), "")); assert_se(streq(memory_startswith_no_case("x", 2, ""), "x")); @@ -605,6 +655,8 @@ static void test_string_truncate_lines_one(const char *input, size_t n_lines, co } static void test_string_truncate_lines(void) { + log_info("/* %s */", __func__); + test_string_truncate_lines_one("", 0, "", false); test_string_truncate_lines_one("", 1, "", false); test_string_truncate_lines_one("", 2, "", false); @@ -676,6 +728,8 @@ static void test_string_extract_lines_one(const char *input, size_t i, const cha } static void test_string_extract_line(void) { + log_info("/* %s */", __func__); + test_string_extract_lines_one("", 0, "", false); test_string_extract_lines_one("", 1, "", false); test_string_extract_lines_one("", 2, "", false); From 53cd7f337489433734c0d134b2a569727dd61a38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Jul 2020 14:06:26 +0200 Subject: [PATCH 02/24] basic: add string_contains_word() This wraps the common pattern of using extract_first_word() in a loop to look for a matching word. --- src/basic/string-util.c | 20 ++++++++++++++ src/basic/string-util.h | 1 + src/test/test-string-util.c | 54 +++++++++++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 755a37f667..6d366a3510 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -8,6 +8,7 @@ #include "alloc-util.h" #include "escape.h" +#include "extract-word.h" #include "fileio.h" #include "gunicode.h" #include "locale-util.h" @@ -1207,3 +1208,22 @@ int string_extract_line(const char *s, size_t i, char **ret) { c++; } } + +int string_contains_word(const char *string, const char *separators, const char *word) { + /* In the default mode with no separators specified, we split on whitespace and + * don't coalesce separators. */ + const ExtractFlags flags = separators ? EXTRACT_DONT_COALESCE_SEPARATORS : 0; + + for (const char *p = string;;) { + _cleanup_free_ char *w = NULL; + int r; + + r = extract_first_word(&p, &w, separators, flags); + if (r < 0) + return r; + if (r == 0) + return false; + if (streq(w, word)) + return true; + } +} diff --git a/src/basic/string-util.h b/src/basic/string-util.h index cf8c74b822..e0ed5cee8c 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -280,3 +280,4 @@ char* string_erase(char *x); int string_truncate_lines(const char *s, size_t n_lines, char **ret); int string_extract_line(const char *s, size_t i, char **ret); +int string_contains_word(const char *string, const char *separators, const char *word); diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 33c5f83575..0834e0b969 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -796,6 +796,59 @@ static void test_string_extract_line(void) { test_string_extract_lines_one("\n\n\nx\n", 3, "x", false); } +static void test_string_contains_word(void) { + log_info("/* %s */", __func__); + + assert_se( string_contains_word("a b cc", NULL, "a")); + assert_se( string_contains_word("a b cc", NULL, "b")); + assert_se(!string_contains_word("a b cc", NULL, "c")); + assert_se( string_contains_word("a b cc", NULL, "cc")); + assert_se(!string_contains_word("a b cc", NULL, "d")); + assert_se(!string_contains_word("a b cc", NULL, "a b")); + assert_se(!string_contains_word("a b cc", NULL, "a b c")); + assert_se(!string_contains_word("a b cc", NULL, "b c")); + assert_se(!string_contains_word("a b cc", NULL, "b cc")); + assert_se(!string_contains_word("a b cc", NULL, "a ")); + assert_se(!string_contains_word("a b cc", NULL, " b ")); + assert_se(!string_contains_word("a b cc", NULL, " cc")); + + assert_se( string_contains_word(" a b\t\tcc", NULL, "a")); + assert_se( string_contains_word(" a b\t\tcc", NULL, "b")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, "c")); + assert_se( string_contains_word(" a b\t\tcc", NULL, "cc")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, "d")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, "a b")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, "a b\t\tc")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, "b\t\tc")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, "b\t\tcc")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, "a ")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, " b ")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, " cc")); + + assert_se(!string_contains_word(" a b\t\tcc", NULL, "")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, " ")); + assert_se(!string_contains_word(" a b\t\tcc", NULL, " ")); + assert_se( string_contains_word(" a b\t\tcc", " ", "")); + assert_se( string_contains_word(" a b\t\tcc", "\t", "")); + assert_se( string_contains_word(" a b\t\tcc", WHITESPACE, "")); + + assert_se( string_contains_word("a:b:cc", ":#", "a")); + assert_se( string_contains_word("a:b:cc", ":#", "b")); + assert_se(!string_contains_word("a:b:cc", ":#", "c")); + assert_se( string_contains_word("a:b:cc", ":#", "cc")); + assert_se(!string_contains_word("a:b:cc", ":#", "d")); + assert_se(!string_contains_word("a:b:cc", ":#", "a:b")); + assert_se(!string_contains_word("a:b:cc", ":#", "a:b:c")); + assert_se(!string_contains_word("a:b:cc", ":#", "b:c")); + assert_se(!string_contains_word("a#b#cc", ":#", "b:cc")); + assert_se( string_contains_word("a#b#cc", ":#", "b")); + assert_se( string_contains_word("a#b#cc", ":#", "cc")); + assert_se(!string_contains_word("a:b:cc", ":#", "a:")); + assert_se(!string_contains_word("a:b cc", ":#", "b")); + assert_se( string_contains_word("a:b cc", ":#", "b cc")); + assert_se(!string_contains_word("a:b:cc", ":#", ":cc")); +} + int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); @@ -831,6 +884,7 @@ int main(int argc, char *argv[]) { test_memory_startswith_no_case(); test_string_truncate_lines(); test_string_extract_line(); + test_string_contains_word(); return 0; } From 81823e6c12771721e9a729f6280a9de26fd70bad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Jul 2020 16:05:45 +0200 Subject: [PATCH 03/24] sd-login: use string_contains_word() --- src/libsystemd/sd-login/sd-login.c | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index 746c895b61..6412002f49 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -12,6 +12,7 @@ #include "dirent-util.h" #include "env-file.h" #include "escape.h" +#include "extract-word.h" #include "fd-util.h" #include "format-util.h" #include "fs-util.h" @@ -22,6 +23,7 @@ #include "parse-util.h" #include "path-util.h" #include "socket-util.h" +#include "stdio-util.h" #include "string-util.h" #include "strv.h" #include "user-util.h" @@ -331,35 +333,29 @@ static int file_of_seat(const char *seat, char **_p) { } _public_ int sd_uid_is_on_seat(uid_t uid, int require_active, const char *seat) { - _cleanup_free_ char *t = NULL, *s = NULL, *p = NULL; - size_t l; + _cleanup_free_ char *filename = NULL, *content = NULL; int r; - const char *word, *variable, *state; assert_return(uid_is_valid(uid), -EINVAL); - r = file_of_seat(seat, &p); + r = file_of_seat(seat, &filename); if (r < 0) return r; - variable = require_active ? "ACTIVE_UID" : "UIDS"; - - r = parse_env_file(NULL, p, variable, &s); + r = parse_env_file(NULL, filename, + require_active ? "ACTIVE_UID" : "UIDS", + &content); if (r == -ENOENT) return 0; if (r < 0) return r; - if (isempty(s)) + if (isempty(content)) return 0; - if (asprintf(&t, UID_FMT, uid) < 0) - return -ENOMEM; + char t[DECIMAL_STR_MAX(uid_t)]; + xsprintf(t, UID_FMT, uid); - FOREACH_WORD(word, l, s, state) - if (strneq(t, word, l)) - return 1; - - return 0; + return string_contains_word(content, ",", t); } static int uid_get_array(uid_t uid, const char *variable, char ***array) { From 46bf625aca8aa6f1f8608611dccddce948ded4d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Jul 2020 10:34:44 +0200 Subject: [PATCH 04/24] Add string_contains_word_strv() I had to move STRV_MAKE to macro.h. There is a circular dependency between extract-word.h, strv.h, and string-util.h that makes it hard to define the inline function otherwise. --- src/basic/extract-word.c | 1 + src/basic/macro.h | 3 +++ src/basic/string-util.c | 17 +++++++++++++---- src/basic/string-util.h | 6 +++++- src/basic/strv.h | 4 ---- src/test/test-string-util.c | 30 ++++++++++++++++++++++++++++++ 6 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/basic/extract-word.c b/src/basic/extract-word.c index 1a53da334a..d64dddd641 100644 --- a/src/basic/extract-word.c +++ b/src/basic/extract-word.c @@ -14,6 +14,7 @@ #include "log.h" #include "macro.h" #include "string-util.h" +#include "strv.h" #include "utf8.h" int extract_first_word(const char **p, char **ret, const char *separators, ExtractFlags flags) { diff --git a/src/basic/macro.h b/src/basic/macro.h index ceea8176f5..41c2c3289e 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -538,6 +538,9 @@ static inline int __coverity_check_and_return__(int condition) { (y) = (_t); \ } while (false) +#define STRV_MAKE(...) ((char**) ((const char*[]) { __VA_ARGS__, NULL })) +#define STRV_MAKE_EMPTY ((char*[1]) { NULL }) + /* Iterates through a specified list of pointers. Accepts NULL pointers, but uses (void*) -1 as internal marker for EOL. */ #define FOREACH_POINTER(p, x, ...) \ for (typeof(p) *_l = (typeof(p)[]) { ({ p = x; }), ##__VA_ARGS__, (void*) -1 }; \ diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 6d366a3510..c4f86b4ee2 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -15,6 +15,7 @@ #include "macro.h" #include "memory-util.h" #include "string-util.h" +#include "strv.h" #include "terminal-util.h" #include "utf8.h" #include "util.h" @@ -1209,11 +1210,13 @@ int string_extract_line(const char *s, size_t i, char **ret) { } } -int string_contains_word(const char *string, const char *separators, const char *word) { +int string_contains_word_strv(const char *string, const char *separators, char **words, const char **ret_word) { /* In the default mode with no separators specified, we split on whitespace and * don't coalesce separators. */ const ExtractFlags flags = separators ? EXTRACT_DONT_COALESCE_SEPARATORS : 0; + const char *found = NULL; + for (const char *p = string;;) { _cleanup_free_ char *w = NULL; int r; @@ -1222,8 +1225,14 @@ int string_contains_word(const char *string, const char *separators, const char if (r < 0) return r; if (r == 0) - return false; - if (streq(w, word)) - return true; + break; + + found = strv_find(words, w); + if (found) + break; } + + if (ret_word) + *ret_word = found; + return !!found; } diff --git a/src/basic/string-util.h b/src/basic/string-util.h index e0ed5cee8c..087fb7c907 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -280,4 +280,8 @@ char* string_erase(char *x); int string_truncate_lines(const char *s, size_t n_lines, char **ret); int string_extract_line(const char *s, size_t i, char **ret); -int string_contains_word(const char *string, const char *separators, const char *word); + +int string_contains_word_strv(const char *string, const char *separators, char **words, const char **ret_word); +static inline int string_contains_word(const char *string, const char *separators, const char *word) { + return string_contains_word_strv(string, separators, STRV_MAKE(word), NULL); +} diff --git a/src/basic/strv.h b/src/basic/strv.h index e57dfff69b..bc0b04b56b 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -123,10 +123,6 @@ bool strv_overlap(char * const *a, char * const *b) _pure_; char **strv_sort(char **l); void strv_print(char * const *l); -#define STRV_MAKE(...) ((char**) ((const char*[]) { __VA_ARGS__, NULL })) - -#define STRV_MAKE_EMPTY ((char*[1]) { NULL }) - #define strv_from_stdarg_alloca(first) \ ({ \ char **_l; \ diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 0834e0b969..b39fda0313 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -796,6 +796,35 @@ static void test_string_extract_line(void) { test_string_extract_lines_one("\n\n\nx\n", 3, "x", false); } +static void test_string_contains_word_strv(void) { + log_info("/* %s */", __func__); + + const char *w; + + assert_se(string_contains_word_strv("a b cc", NULL, STRV_MAKE("a", "b"), NULL)); + + assert_se(string_contains_word_strv("a b cc", NULL, STRV_MAKE("a", "b"), &w)); + assert_se(streq(w, "a")); + + assert_se(!string_contains_word_strv("a b cc", NULL, STRV_MAKE("d"), &w)); + assert_se(w == NULL); + + assert_se(string_contains_word_strv("a b cc", NULL, STRV_MAKE("b", "a"), &w)); + assert_se(streq(w, "a")); + + assert_se(string_contains_word_strv("b a b cc", NULL, STRV_MAKE("b", "a", "b"), &w)); + assert_se(streq(w, "b")); + + assert_se(string_contains_word_strv("a b cc", NULL, STRV_MAKE("b", ""), &w)); + assert_se(streq(w, "b")); + + assert_se(!string_contains_word_strv("a b cc", NULL, STRV_MAKE(""), &w)); + assert_se(w == NULL); + + assert_se(string_contains_word_strv("a b cc", " ", STRV_MAKE(""), &w)); + assert_se(streq(w, "")); +} + static void test_string_contains_word(void) { log_info("/* %s */", __func__); @@ -884,6 +913,7 @@ int main(int argc, char *argv[]) { test_memory_startswith_no_case(); test_string_truncate_lines(); test_string_extract_line(); + test_string_contains_word_strv(); test_string_contains_word(); return 0; From 434fef6de399ea7b95077572258def799acf02ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Jul 2020 11:34:10 +0200 Subject: [PATCH 05/24] shared/sleep-config: more logging and port to extract_first_word() --- src/shared/sleep-config.c | 65 ++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 0dccc8f970..e7d2efb2d2 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -100,9 +100,8 @@ int parse_sleep_config(SleepConfig **ret_sleep_config) { } int can_sleep_state(char **types) { - char **type; + _cleanup_free_ char *text = NULL; int r; - _cleanup_free_ char *p = NULL; if (strv_isempty(types)) return true; @@ -113,34 +112,27 @@ int can_sleep_state(char **types) { return false; } - r = read_one_line_file("/sys/power/state", &p); + r = read_one_line_file("/sys/power/state", &text); if (r < 0) { log_debug_errno(r, "Failed to read /sys/power/state, cannot sleep: %m"); return false; } - STRV_FOREACH(type, types) { - const char *word, *state; - size_t l, k; - - k = strlen(*type); - FOREACH_WORD_SEPARATOR(word, l, p, WHITESPACE, state) - if (l == k && memcmp(word, *type, l) == 0) { - log_debug("Sleep mode \"%s\" is supported by the kernel.", *type); - return true; - } - } - - if (DEBUG_LOGGING) { + const char *found; + r = string_contains_word_strv(text, NULL, types, &found); + if (r < 0) + return log_debug_errno(r, "Failed to parse /sys/power/state: %m"); + if (r > 0) + log_debug("Sleep mode \"%s\" is supported by the kernel.", found); + else if (DEBUG_LOGGING) { _cleanup_free_ char *t = strv_join(types, "/"); log_debug("Sleep mode %s not supported by the kernel, sorry.", strnull(t)); } - return false; + return r; } int can_sleep_disk(char **types) { - _cleanup_free_ char *p = NULL; - char **type; + _cleanup_free_ char *text = NULL; int r; if (strv_isempty(types)) @@ -152,29 +144,38 @@ int can_sleep_disk(char **types) { return false; } - r = read_one_line_file("/sys/power/disk", &p); + r = read_one_line_file("/sys/power/disk", &text); if (r < 0) { log_debug_errno(r, "Couldn't read /sys/power/disk: %m"); return false; } - STRV_FOREACH(type, types) { - const char *word, *state; - size_t l, k; + for (const char *p = text;;) { + _cleanup_free_ char *word = NULL; - k = strlen(*type); - FOREACH_WORD_SEPARATOR(word, l, p, WHITESPACE, state) { - if (l == k && memcmp(word, *type, l) == 0) - return true; + r = extract_first_word(&p, &word, NULL, 0); + if (r < 0) + return log_debug_errno(r, "Failed to parse /sys/power/disk: %m"); + if (r == 0) + break; - if (l == k + 2 && - word[0] == '[' && - memcmp(word + 1, *type, l - 2) == 0 && - word[l-1] == ']') - return true; + char *s = word; + size_t l = strlen(s); + if (s[0] == '[' && s[l-1] == ']') { + s[l-1] = '\0'; + s++; + } + + if (strv_contains(types, s)) { + log_debug("Disk sleep mode \"%s\" is supported by the kernel.", s); + return true; } } + if (DEBUG_LOGGING) { + _cleanup_free_ char *t = strv_join(types, "/"); + log_debug("Disk sleep mode %s not supported by the kernel, sorry.", strnull(t)); + } return false; } From 46ed9f4ce1dc3d5db17d9274af510e7407580b3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Jul 2020 11:47:38 +0200 Subject: [PATCH 06/24] logind: use extract_first_word() --- src/login/logind-inhibit.c | 35 ++++++++++++++++++++++------------- src/login/logind-inhibit.h | 2 +- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index 1d335f914c..254201d23a 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -8,6 +8,7 @@ #include "alloc-util.h" #include "env-file.h" +#include "errno-list.h" #include "escape.h" #include "fd-util.h" #include "fileio.h" @@ -483,31 +484,39 @@ const char *inhibit_what_to_string(InhibitWhat w) { return buffer; } -InhibitWhat inhibit_what_from_string(const char *s) { +int inhibit_what_from_string(const char *s) { InhibitWhat what = 0; - const char *word, *state; - size_t l; - FOREACH_WORD_SEPARATOR(word, l, s, ":", state) { - if (l == 8 && strneq(word, "shutdown", l)) + for (const char *p = s;;) { + _cleanup_free_ char *word = NULL; + int r; + + /* A sanity check that our return values fit in an int */ + assert_cc((int) _INHIBIT_WHAT_MAX == _INHIBIT_WHAT_MAX); + + r = extract_first_word(&p, &word, ":", EXTRACT_DONT_COALESCE_SEPARATORS); + if (r < 0) + return r; + if (r == 0) + return what; + + if (streq(word, "shutdown")) what |= INHIBIT_SHUTDOWN; - else if (l == 5 && strneq(word, "sleep", l)) + else if (streq(word, "sleep")) what |= INHIBIT_SLEEP; - else if (l == 4 && strneq(word, "idle", l)) + else if (streq(word, "idle")) what |= INHIBIT_IDLE; - else if (l == 16 && strneq(word, "handle-power-key", l)) + else if (streq(word, "handle-power-key")) what |= INHIBIT_HANDLE_POWER_KEY; - else if (l == 18 && strneq(word, "handle-suspend-key", l)) + else if (streq(word, "handle-suspend-key")) what |= INHIBIT_HANDLE_SUSPEND_KEY; - else if (l == 20 && strneq(word, "handle-hibernate-key", l)) + else if (streq(word, "handle-hibernate-key")) what |= INHIBIT_HANDLE_HIBERNATE_KEY; - else if (l == 17 && strneq(word, "handle-lid-switch", l)) + else if (streq(word, "handle-lid-switch")) what |= INHIBIT_HANDLE_LID_SWITCH; else return _INHIBIT_WHAT_INVALID; } - - return what; } static const char* const inhibit_mode_table[_INHIBIT_MODE_MAX] = { diff --git a/src/login/logind-inhibit.h b/src/login/logind-inhibit.h index cea67a08c5..7eaecee0b4 100644 --- a/src/login/logind-inhibit.h +++ b/src/login/logind-inhibit.h @@ -66,7 +66,7 @@ InhibitWhat manager_inhibit_what(Manager *m, InhibitMode mm); bool manager_is_inhibited(Manager *m, InhibitWhat w, InhibitMode mm, dual_timestamp *since, bool ignore_inactive, bool ignore_uid, uid_t uid, Inhibitor **offending); const char *inhibit_what_to_string(InhibitWhat k); -InhibitWhat inhibit_what_from_string(const char *s); +int inhibit_what_from_string(const char *s); const char *inhibit_mode_to_string(InhibitMode k); InhibitMode inhibit_mode_from_string(const char *s); From ae7ef63f21dd8f717a61fc71e189eaff73eeb685 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Jul 2020 12:01:21 +0200 Subject: [PATCH 07/24] basic/cgroup-util: port over to string_contains_word() --- src/basic/cgroup-util.c | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index e94fcfad02..6210347553 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -652,14 +652,13 @@ int cg_remove_xattr(const char *controller, const char *path, const char *name) return 0; } -int cg_pid_get_path(const char *controller, pid_t pid, char **path) { +int cg_pid_get_path(const char *controller, pid_t pid, char **ret_path) { _cleanup_fclose_ FILE *f = NULL; const char *fs, *controller_str; int unified, r; - size_t cs = 0; - assert(path); assert(pid >= 0); + assert(ret_path); if (controller) { if (!cg_controller_is_valid(controller)) @@ -675,8 +674,6 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) { controller_str = SYSTEMD_CGROUP_CONTROLLER_LEGACY; else controller_str = controller; - - cs = strlen(controller_str); } fs = procfs_file_alloca(pid, "cgroup"); @@ -688,13 +685,13 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) { for (;;) { _cleanup_free_ char *line = NULL; - char *e, *p; + char *e; r = read_line(f, LONG_LINE_MAX, &line); if (r < 0) return r; if (r == 0) - break; + return -ENODATA; if (unified) { e = startswith(line, "0:"); @@ -706,9 +703,6 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) { continue; } else { char *l; - size_t k; - const char *word, *state; - bool found = false; l = strchr(line, ':'); if (!l) @@ -718,31 +712,27 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **path) { e = strchr(l, ':'); if (!e) continue; - *e = 0; - FOREACH_WORD_SEPARATOR(word, k, l, ",", state) - if (k == cs && memcmp(word, controller_str, cs) == 0) { - found = true; - break; - } - if (!found) + + r = string_contains_word(l, ",", controller_str); + if (r < 0) + return r; + if (r == 0) continue; } - p = strdup(e + 1); - if (!p) + char *path = strdup(e + 1); + if (!path) return -ENOMEM; /* Truncate suffix indicating the process is a zombie */ - e = endswith(p, " (deleted)"); + e = endswith(path, " (deleted)"); if (e) *e = 0; - *path = p; + *ret_path = path; return 0; } - - return -ENODATA; } int cg_install_release_agent(const char *controller, const char *agent) { From 87a4d416e5126b6fb2528ae192a6a6a8033539ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 29 Jul 2020 12:09:08 +0200 Subject: [PATCH 08/24] sd-device: use extract_first_word() --- src/libsystemd/sd-device/device-private.c | 41 +++++++++++------------ 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index 1ad7713ec7..aa1ac3905a 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -316,34 +316,33 @@ static int device_amend(sd_device *device, const char *key, const char *value) { if (r < 0) return log_device_debug_errno(device, r, "sd-device: Failed to set SEQNUM to '%s': %m", value); } else if (streq(key, "DEVLINKS")) { - const char *word, *state; - size_t l; + for (const char *p = value;;) { + _cleanup_free_ char *word = NULL; - FOREACH_WORD(word, l, value, state) { - char devlink[l + 1]; - - strncpy(devlink, word, l); - devlink[l] = '\0'; - - r = device_add_devlink(device, devlink); + r = extract_first_word(&p, &word, NULL, 0); if (r < 0) - return log_device_debug_errno(device, r, "sd-device: Failed to add devlink '%s': %m", devlink); + return r; + if (r == 0) + break; + + r = device_add_devlink(device, word); + if (r < 0) + return log_device_debug_errno(device, r, "sd-device: Failed to add devlink '%s': %m", word); } } else if (STR_IN_SET(key, "TAGS", "CURRENT_TAGS")) { - const char *word, *state; - size_t l; + for (const char *p = value;;) { + _cleanup_free_ char *word = NULL; - FOREACH_WORD_SEPARATOR(word, l, value, ":", state) { - char tag[l + 1]; - - (void) strncpy(tag, word, l); - tag[l] = '\0'; - - r = device_add_tag(device, tag, streq(key, "CURRENT_TAGS")); + r = extract_first_word(&p, &word, ":", EXTRACT_DONT_COALESCE_SEPARATORS); if (r < 0) - return log_device_debug_errno(device, r, "sd-device: Failed to add tag '%s': %m", tag); - } + return r; + if (r == 0) + break; + r = device_add_tag(device, word, streq(key, "CURRENT_TAGS")); + if (r < 0) + return log_device_debug_errno(device, r, "sd-device: Failed to add tag '%s': %m", word); + } } else { r = device_add_property_internal(device, key, value); if (r < 0) From aa3b40c3f98f28312967b3f9ba1f353efd9c9873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Jul 2020 12:43:07 +0200 Subject: [PATCH 09/24] Fix output value of sd_seat_get_sessions() and drop FOREACH_WORD use sd_seat_get_sessions() would return 0 in the 'n_uids' (now 'ret_n_uids') output parameter when 'uid' (now 'ret_uids') was passed as NULL. While at it, drop FOREACH_WORD() use. Also use any whitespace as separator. In practice this shouldn't matter, since logind always uses spaces, but it seems nicer to not specify this explicitly, and the default is more flexible. --- man/sd_seat_get_active.xml | 31 +++++----- src/libsystemd/sd-login/sd-login.c | 93 +++++++++++++++--------------- src/systemd/sd-login.h | 6 +- 3 files changed, 63 insertions(+), 67 deletions(-) diff --git a/man/sd_seat_get_active.xml b/man/sd_seat_get_active.xml index cf70b35785..94401caa72 100644 --- a/man/sd_seat_get_active.xml +++ b/man/sd_seat_get_active.xml @@ -38,9 +38,9 @@ int sd_seat_get_sessions const char *seat - char ***sessions - uid_t **uid - unsigned int *n_uids + char ***ret_sessions + uid_t **ret_uids + unsigned int *ret_n_uids @@ -68,21 +68,16 @@ free3 call after use. - sd_seat_get_sessions() may be used to - determine all sessions on the specified seat. Returns two arrays, - one (NULL terminated) with the session - identifiers of the sessions and one with the user identifiers of - the Unix users the sessions belong to. An additional parameter may - be used to return the number of entries in the latter array. This - value is the same the return value, if the latter is nonnegative. - The two arrays and the last parameter may be passed as - NULL in case these values need not to be - determined. The arrays and the strings referenced by them need to - be freed with the libc - free3 - call after use. Note that instead of an empty array - NULL may be returned and should be considered - equivalent to an empty array. + sd_seat_get_sessions() may be used to determine all sessions on the specified + seat. Returns two arrays, one (NULL terminated) with the session identifiers of the + sessions and one with the user identifiers of the Unix users the sessions belong to. An additional + parameter may be used to return the number of entries in the latter array. This value is the same as the + return value if the return value is nonnegative. The output parameters may be passed as + NULL in case these output values are not needed. The arrays and the strings + referenced by them need to be freed with the libc free3 call after + use. Note that instead of an empty array NULL may be returned and should be + considered equivalent to an empty array. sd_seat_can_tty() may be used to determine whether a specific seat provides TTY functionality, i.e. diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index 6412002f49..14e0468720 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -378,7 +378,7 @@ static int uid_get_array(uid_t uid, const char *variable, char ***array) { if (r < 0) return r; - a = strv_split(s, " "); + a = strv_split(s, NULL); if (!a) return -ENOMEM; @@ -650,73 +650,70 @@ _public_ int sd_seat_get_active(const char *seat, char **session, uid_t *uid) { return 0; } -_public_ int sd_seat_get_sessions(const char *seat, char ***sessions, uid_t **uids, unsigned *n_uids) { - _cleanup_free_ char *p = NULL, *s = NULL, *t = NULL; - _cleanup_strv_free_ char **a = NULL; - _cleanup_free_ uid_t *b = NULL; - unsigned n = 0; +_public_ int sd_seat_get_sessions( + const char *seat, + char ***ret_sessions, + uid_t **ret_uids, + unsigned *ret_n_uids) { + + _cleanup_free_ char *fname = NULL, *session_line = NULL, *uid_line = NULL; + _cleanup_strv_free_ char **sessions = NULL; + _cleanup_free_ uid_t *uids = NULL; + unsigned n_sessions = 0; int r; - r = file_of_seat(seat, &p); + r = file_of_seat(seat, &fname); if (r < 0) return r; - r = parse_env_file(NULL, p, - "SESSIONS", &s, - "UIDS", &t); + r = parse_env_file(NULL, fname, + "SESSIONS", &session_line, + "UIDS", &uid_line); if (r == -ENOENT) return -ENXIO; if (r < 0) return r; - if (s) { - a = strv_split(s, " "); - if (!a) + if (session_line) { + sessions = strv_split(session_line, NULL); + if (!sessions) return -ENOMEM; - } - if (uids && t) { - const char *word, *state; - size_t l; + n_sessions = strv_length(sessions); + }; - FOREACH_WORD(word, l, t, state) - n++; + if (ret_uids && uid_line) { + uids = new(uid_t, n_sessions); + if (!uids) + return -ENOMEM; - if (n > 0) { - unsigned i = 0; + size_t n = 0; + for (const char *p = uid_line;;) { + _cleanup_free_ char *word = NULL; - b = new(uid_t, n); - if (!b) - return -ENOMEM; + r = extract_first_word(&p, &word, NULL, 0); + if (r < 0) + return r; + if (r == 0) + break; - FOREACH_WORD(word, l, t, state) { - _cleanup_free_ char *k = NULL; - - k = strndup(word, l); - if (!k) - return -ENOMEM; - - r = parse_uid(k, b + i); - if (r < 0) - return r; - - i++; - } + r = parse_uid(word, &uids[n++]); + if (r < 0) + return r; } + + if (n != n_sessions) + return -EUCLEAN; } - r = (int) strv_length(a); + if (ret_sessions) + *ret_sessions = TAKE_PTR(sessions); + if (ret_uids) + *ret_uids = TAKE_PTR(uids); + if (ret_n_uids) + *ret_n_uids = n_sessions; - if (sessions) - *sessions = TAKE_PTR(a); - - if (uids) - *uids = TAKE_PTR(b); - - if (n_uids) - *n_uids = n; - - return r; + return n_sessions; } static int seat_get_can(const char *seat, const char *variable) { diff --git a/src/systemd/sd-login.h b/src/systemd/sd-login.h index e18f01bb67..360f44d341 100644 --- a/src/systemd/sd-login.h +++ b/src/systemd/sd-login.h @@ -180,7 +180,11 @@ int sd_seat_get_active(const char *seat, char **session, uid_t *uid); /* Return sessions and users on seat. Returns number of sessions. * If sessions is NULL, this returns only the number of sessions. */ -int sd_seat_get_sessions(const char *seat, char ***sessions, uid_t **uid, unsigned *n_uids); +int sd_seat_get_sessions( + const char *seat, + char ***ret_sessions, + uid_t **ret_uids, + unsigned *ret_n_uids); /* Return whether the seat is multi-session capable */ int sd_seat_can_multi_session(const char *seat) _sd_deprecated_; From 0ef14adc1c724eee8d5f4db3d298786b209afaa5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Jul 2020 12:56:51 +0200 Subject: [PATCH 10/24] Rewrite sd_machine_get_ifindices() to avoid FOREACH_WORD() If we fail to parse the index, the failure is propogated as -EUNCLEAN. (-EINVAL would be confused with invalid args to the function itself.) --- man/sd_machine_get_class.xml | 2 +- src/libsystemd/sd-login/sd-login.c | 46 +++++++++++++++--------------- src/systemd/sd-login.h | 2 +- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/man/sd_machine_get_class.xml b/man/sd_machine_get_class.xml index cd259c863f..0a0d601899 100644 --- a/man/sd_machine_get_class.xml +++ b/man/sd_machine_get_class.xml @@ -35,7 +35,7 @@ int sd_machine_get_ifindices const char* machine - int **ifindices + int **ret_ifindices diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index 14e0468720..3828fa58e4 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -894,47 +894,47 @@ _public_ int sd_machine_get_class(const char *machine, char **class) { return 0; } -_public_ int sd_machine_get_ifindices(const char *machine, int **ifindices) { - _cleanup_free_ char *netif = NULL; - size_t l, allocated = 0, nr = 0; - int *ni = NULL; - const char *p, *word, *state; +_public_ int sd_machine_get_ifindices(const char *machine, int **ret_ifindices) { + _cleanup_free_ char *netif_line = NULL; + const char *p; int r; assert_return(machine_name_is_valid(machine), -EINVAL); - assert_return(ifindices, -EINVAL); + assert_return(ret_ifindices, -EINVAL); p = strjoina("/run/systemd/machines/", machine); - r = parse_env_file(NULL, p, "NETIF", &netif); + r = parse_env_file(NULL, p, "NETIF", &netif_line); if (r == -ENOENT) return -ENXIO; if (r < 0) return r; - if (!netif) { - *ifindices = NULL; + if (!netif_line) { + *ret_ifindices = NULL; return 0; } - FOREACH_WORD(word, l, netif, state) { - char buf[l+1]; - int ifi; + _cleanup_strv_free_ char **tt = strv_split(netif_line, NULL); + if (!tt) + return -ENOMEM; - *(char*) (mempcpy(buf, word, l)) = 0; + size_t n = 0; + int *ifindices = new(int, strv_length(tt)); + if (!ifindices) + return -ENOMEM; - ifi = parse_ifindex(buf); - if (ifi < 0) - continue; + for (size_t i = 0; tt[i]; i++) { + int ind; - if (!GREEDY_REALLOC(ni, allocated, nr+1)) { - free(ni); - return -ENOMEM; - } + ind = parse_ifindex(tt[i]); + if (ind < 0) + /* Return -EUCLEAN to distinguish from -EINVAL for invalid args */ + return ind == -EINVAL ? -EUCLEAN : ind; - ni[nr++] = ifi; + ifindices[n++] = ind; } - *ifindices = ni; - return nr; + *ret_ifindices = ifindices; + return n; } static int MONITOR_TO_FD(sd_login_monitor *m) { diff --git a/src/systemd/sd-login.h b/src/systemd/sd-login.h index 360f44d341..6a8c206259 100644 --- a/src/systemd/sd-login.h +++ b/src/systemd/sd-login.h @@ -199,7 +199,7 @@ int sd_seat_can_graphical(const char *seat); int sd_machine_get_class(const char *machine, char **clazz); /* Return the list if host-side network interface indices of a machine */ -int sd_machine_get_ifindices(const char *machine, int **ifindices); +int sd_machine_get_ifindices(const char *machine, int **ret_ifindices); /* Get all seats, store in *seats. Returns the number of seats. If * seats is NULL, this only returns the number of seats. */ From dd630d3cac6d72e2f4661273d2b28c95c545881c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 30 Jul 2020 13:08:52 +0200 Subject: [PATCH 11/24] Let sd_machine_get_ifindices() omit the output param too Nowadays we do that almost everywhere, let's also do it here. --- man/sd_machine_get_class.xml | 19 ++++++++++--------- src/libsystemd/sd-login/sd-login.c | 17 +++++++++++------ 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/man/sd_machine_get_class.xml b/man/sd_machine_get_class.xml index 0a0d601899..a8db371230 100644 --- a/man/sd_machine_get_class.xml +++ b/man/sd_machine_get_class.xml @@ -53,21 +53,22 @@ project='man-pages'>free3 call after use. - sd_machine_get_ifindices() may be used - to determine the numeric indices of the network interfaces on the - host that are pointing towards the specified locally running - virtual machine or container that is registered with + sd_machine_get_ifindices() may be used to determine the numeric indices of the + network interfaces on the host that are pointing towards the specified locally running virtual machine or + container. The vm or container must be registered with systemd-machined.service8. - The returned array needs to be freed with the libc free3 - call after use. + The output parameter ret_ifindices may be passed as NULL when + the output value is not needed. The returned array needs to be freed with the libc free3 call after + use. Return Value - On success, these calls return 0 or a positive integer. On failure, these calls return a negative - errno-style error code. + On success, these functions return a non-negative integer. + sd_machine_get_ifindices() returns the number of the relevant network interfaces. + On failure, these calls return a negative errno-style error code. Errors diff --git a/src/libsystemd/sd-login/sd-login.c b/src/libsystemd/sd-login/sd-login.c index 3828fa58e4..601a27ab57 100644 --- a/src/libsystemd/sd-login/sd-login.c +++ b/src/libsystemd/sd-login/sd-login.c @@ -900,7 +900,6 @@ _public_ int sd_machine_get_ifindices(const char *machine, int **ret_ifindices) int r; assert_return(machine_name_is_valid(machine), -EINVAL); - assert_return(ret_ifindices, -EINVAL); p = strjoina("/run/systemd/machines/", machine); r = parse_env_file(NULL, p, "NETIF", &netif_line); @@ -918,9 +917,12 @@ _public_ int sd_machine_get_ifindices(const char *machine, int **ret_ifindices) return -ENOMEM; size_t n = 0; - int *ifindices = new(int, strv_length(tt)); - if (!ifindices) - return -ENOMEM; + int *ifindices; + if (ret_ifindices) { + ifindices = new(int, strv_length(tt)); + if (!ifindices) + return -ENOMEM; + } for (size_t i = 0; tt[i]; i++) { int ind; @@ -930,10 +932,13 @@ _public_ int sd_machine_get_ifindices(const char *machine, int **ret_ifindices) /* Return -EUCLEAN to distinguish from -EINVAL for invalid args */ return ind == -EINVAL ? -EUCLEAN : ind; - ifindices[n++] = ind; + if (ret_ifindices) + ifindices[n] = ind; + n++; } - *ret_ifindices = ifindices; + if (ret_ifindices) + *ret_ifindices = ifindices; return n; } From 7896ad8f6601c8e8ffec5ca6bd9a035f9b5de805 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 11:19:25 +0200 Subject: [PATCH 12/24] core/load-fragment: use extract_first_word() This is much nicer, and also fixes a potential overflow when we used 'word' in log_error() as if it was a NUL-terminated string. --- src/core/load-fragment.c | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index a93f12b27c..7c5730ac0e 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -4430,15 +4430,13 @@ int config_parse_set_status( void *data, void *userdata) { - size_t l; - const char *word, *state; - int r; ExitStatusSet *status_set = data; + int r; assert(filename); assert(lvalue); assert(rvalue); - assert(data); + assert(status_set); /* Empty assignment resets the list */ if (isempty(rvalue)) { @@ -4446,25 +4444,26 @@ int config_parse_set_status( return 0; } - FOREACH_WORD(word, l, rvalue, state) { - _cleanup_free_ char *temp; + for (const char *p = rvalue;;) { + _cleanup_free_ char *word = NULL; Bitmap *bitmap; - temp = strndup(word, l); - if (!temp) - return log_oom(); + r = extract_first_word(&p, &word, NULL, 0); + if (r < 0) + return log_error_errno(r, "Failed to parse %s: %m", lvalue); + if (r == 0) + return 0; /* We need to call exit_status_from_string() first, because we want * to parse numbers as exit statuses, not signals. */ - r = exit_status_from_string(temp); + r = exit_status_from_string(word); if (r >= 0) { assert(r >= 0 && r < 256); bitmap = &status_set->status; } else { - r = signal_from_string(temp); - - if (r <= 0) { + r = signal_from_string(word); + if (r < 0) { log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse value, ignoring: %s", word); continue; @@ -4476,10 +4475,6 @@ int config_parse_set_status( if (r < 0) return log_error_errno(r, "Failed to set signal or status %s: %m", word); } - if (!isempty(state)) - log_syntax(unit, LOG_ERR, filename, line, 0, "Trailing garbage, ignoring."); - - return 0; } int config_parse_namespace_path_strv( From dd2fff3a189a956170978ddeda6618b0f84ab2a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 11:23:44 +0200 Subject: [PATCH 13/24] cryptsetup: use extract_first_word() --- src/cryptsetup/cryptsetup.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index f9e627da46..7d0571f147 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -288,19 +288,19 @@ static int parse_one_option(const char *option) { } static int parse_options(const char *options) { - const char *word, *state; - size_t l; - int r; - assert(options); - FOREACH_WORD_SEPARATOR(word, l, options, ",", state) { - _cleanup_free_ char *o; + for (;;) { + _cleanup_free_ char *word = NULL; + int r; - o = strndup(word, l); - if (!o) - return -ENOMEM; - r = parse_one_option(o); + r = extract_first_word(&options, &word, ",", EXTRACT_DONT_COALESCE_SEPARATORS); + if (r < 0) + return log_debug_errno(r, "Failed to parse options: %m"); + if (r == 0) + break; + + r = parse_one_option(word); if (r < 0) return r; } From cc24f0b8725af5d8bb8f601f49327ed2f8e6d382 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 11:28:11 +0200 Subject: [PATCH 14/24] delta: use extract_first_word() --- src/delta/delta.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/delta/delta.c b/src/delta/delta.c index 29e5120375..b37bad8447 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -545,28 +545,33 @@ static int help(void) { } static int parse_flags(const char *flag_str, int flags) { - const char *word, *state; - size_t l; + for (;;) { + _cleanup_free_ char *word = NULL; + int r; - FOREACH_WORD_SEPARATOR(word, l, flag_str, ",", state) { - if (strneq("masked", word, l)) + r = extract_first_word(&flag_str, &word, ",", EXTRACT_DONT_COALESCE_SEPARATORS); + if (r < 0) + return r; + if (r == 0) + return flags; + + if (streq(word, "masked")) flags |= SHOW_MASKED; - else if (strneq ("equivalent", word, l)) + else if (streq(word, "equivalent")) flags |= SHOW_EQUIVALENT; - else if (strneq("redirected", word, l)) + else if (streq(word, "redirected")) flags |= SHOW_REDIRECTED; - else if (strneq("overridden", word, l)) + else if (streq(word, "overridden")) flags |= SHOW_OVERRIDDEN; - else if (strneq("unchanged", word, l)) + else if (streq(word, "unchanged")) flags |= SHOW_UNCHANGED; - else if (strneq("extended", word, l)) + else if (streq(word, "extended")) flags |= SHOW_EXTENDED; - else if (strneq("default", word, l)) + else if (streq(word, "default")) flags |= SHOW_DEFAULTS; else return -EINVAL; } - return flags; } static int parse_argv(int argc, char *argv[]) { From da277e90a4de21fc7d9871f1b534c8c7f312ff8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 11:38:08 +0200 Subject: [PATCH 15/24] sd-journal: use extract_first_word() --- src/journal/sd-journal.c | 61 ++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c index 2023cc0c01..bb9daa9719 100644 --- a/src/journal/sd-journal.c +++ b/src/journal/sd-journal.c @@ -951,74 +951,69 @@ _public_ int sd_journal_get_cursor(sd_journal *j, char **cursor) { } _public_ int sd_journal_seek_cursor(sd_journal *j, const char *cursor) { - const char *word, *state; - size_t l; unsigned long long seqnum, monotonic, realtime, xor_hash; - bool - seqnum_id_set = false, - seqnum_set = false, - boot_id_set = false, - monotonic_set = false, - realtime_set = false, - xor_hash_set = false; + bool seqnum_id_set = false, + seqnum_set = false, + boot_id_set = false, + monotonic_set = false, + realtime_set = false, + xor_hash_set = false; sd_id128_t seqnum_id, boot_id; + int r; assert_return(j, -EINVAL); assert_return(!journal_pid_changed(j), -ECHILD); assert_return(!isempty(cursor), -EINVAL); - FOREACH_WORD_SEPARATOR(word, l, cursor, ";", state) { - char *item; - int k = 0; + for (const char *p = cursor;;) { + _cleanup_free_ char *word = NULL; - if (l < 2 || word[1] != '=') + r = extract_first_word(&p, &word, ";", EXTRACT_DONT_COALESCE_SEPARATORS); + if (r < 0) + return r; + if (r == 0) + break; + + if (word[0] == '\0' || word[1] != '=') return -EINVAL; - item = strndup(word, l); - if (!item) - return -ENOMEM; - switch (word[0]) { - case 's': seqnum_id_set = true; - k = sd_id128_from_string(item+2, &seqnum_id); + r = sd_id128_from_string(word + 2, &seqnum_id); + if (r < 0) + return r; break; case 'i': seqnum_set = true; - if (sscanf(item+2, "%llx", &seqnum) != 1) - k = -EINVAL; + if (sscanf(word + 2, "%llx", &seqnum) != 1) + return -EINVAL; break; case 'b': boot_id_set = true; - k = sd_id128_from_string(item+2, &boot_id); + r = sd_id128_from_string(word + 2, &boot_id); break; case 'm': monotonic_set = true; - if (sscanf(item+2, "%llx", &monotonic) != 1) - k = -EINVAL; + if (sscanf(word + 2, "%llx", &monotonic) != 1) + return -EINVAL; break; case 't': realtime_set = true; - if (sscanf(item+2, "%llx", &realtime) != 1) - k = -EINVAL; + if (sscanf(word + 2, "%llx", &realtime) != 1) + return -EINVAL; break; case 'x': xor_hash_set = true; - if (sscanf(item+2, "%llx", &xor_hash) != 1) - k = -EINVAL; + if (sscanf(word + 2, "%llx", &xor_hash) != 1) + return -EINVAL; break; } - - free(item); - - if (k < 0) - return k; } if ((!seqnum_set || !seqnum_id_set) && From 2417658d6aa948473de9dbf1f5b250cc6a22cec1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 11:47:03 +0200 Subject: [PATCH 16/24] getty-generator: use extract_first_word() --- src/getty-generator/getty-generator.c | 136 +++++++++++++------------- 1 file changed, 66 insertions(+), 70 deletions(-) diff --git a/src/getty-generator/getty-generator.c b/src/getty-generator/getty-generator.c index be38612060..04dcacfd59 100644 --- a/src/getty-generator/getty-generator.c +++ b/src/getty-generator/getty-generator.c @@ -99,89 +99,85 @@ static int verify_tty(const char *name) { return 0; } +static int run_container(void) { + _cleanup_free_ char *container_ttys = NULL; + int r; + + log_debug("Automatically adding console shell."); + + r = add_symlink("console-getty.service", "console-getty.service"); + if (r < 0) + return r; + + /* When $container_ttys is set for PID 1, spawn gettys on all ptys named therein. + * Note that despite the variable name we only support ptys here. */ + + (void) getenv_for_pid(1, "container_ttys", &container_ttys); + + for (const char *p = container_ttys;;) { + _cleanup_free_ char *word = NULL; + + r = extract_first_word(&p, &word, NULL, 0); + if (r < 0) + return log_error_errno(r, "Failed to parse $container_ttys: %m"); + if (r == 0) + return 0; + + const char *tty = word; + + /* First strip off /dev/ if it is specified */ + tty = path_startswith(tty, "/dev/") ?: tty; + + /* Then, make sure it's actually a pty */ + tty = path_startswith(tty, "pts/"); + if (!tty) + continue; + + r = add_container_getty(tty); + if (r < 0) + return r; + } +} + static int run(const char *dest, const char *dest_early, const char *dest_late) { - _cleanup_free_ char *active = NULL; - const char *j; int r; assert_se(arg_dest = dest); - if (detect_container() > 0) { - _cleanup_free_ char *container_ttys = NULL; + if (detect_container() > 0) + /* Add console shell and look at $container_ttys, but don't do add any + * further magic if we are in a container. */ + return run_container(); - log_debug("Automatically adding console shell."); + /* Automatically add in a serial getty on all active kernel consoles */ + _cleanup_free_ char *active = NULL; + (void) read_one_line_file("/sys/class/tty/console/active", &active); + for (const char *p = active;;) { + _cleanup_free_ char *tty = NULL; - r = add_symlink("console-getty.service", "console-getty.service"); + r = extract_first_word(&p, &tty, NULL, 0); + if (r < 0) + return log_error_errno(r, "Failed to parse /sys/class/tty/console/active: %m"); + if (r == 0) + break; + + /* We assume that gettys on virtual terminals are started via manual configuration and do + * this magic only for non-VC terminals. */ + + if (isempty(tty) || tty_is_vc(tty)) + continue; + + if (verify_tty(tty) < 0) + continue; + + r = add_serial_getty(tty); if (r < 0) return r; - - /* When $container_ttys is set for PID 1, spawn - * gettys on all ptys named therein. Note that despite - * the variable name we only support ptys here. */ - - r = getenv_for_pid(1, "container_ttys", &container_ttys); - if (r > 0) { - const char *word, *state; - size_t l; - - FOREACH_WORD(word, l, container_ttys, state) { - const char *t; - char tty[l + 1]; - - memcpy(tty, word, l); - tty[l] = 0; - - /* First strip off /dev/ if it is specified */ - t = path_startswith(tty, "/dev/"); - if (!t) - t = tty; - - /* Then, make sure it's actually a pty */ - t = path_startswith(t, "pts/"); - if (!t) - continue; - - r = add_container_getty(t); - if (r < 0) - return r; - } - } - - /* Don't add any further magic if we are in a container */ - return 0; - } - - if (read_one_line_file("/sys/class/tty/console/active", &active) >= 0) { - const char *word, *state; - size_t l; - - /* Automatically add in a serial getty on all active - * kernel consoles */ - FOREACH_WORD(word, l, active, state) { - _cleanup_free_ char *tty = NULL; - - tty = strndup(word, l); - if (!tty) - return log_oom(); - - /* We assume that gettys on virtual terminals are - * started via manual configuration and do this magic - * only for non-VC terminals. */ - - if (isempty(tty) || tty_is_vc(tty)) - continue; - - if (verify_tty(tty) < 0) - continue; - - r = add_serial_getty(tty); - if (r < 0) - return r; - } } /* Automatically add in a serial getty on the first * virtualizer console */ + const char *j; FOREACH_STRING(j, "hvc0", "xvc0", From 087908c140f00164dc70e8b323c89dbd47929da7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 11:57:03 +0200 Subject: [PATCH 17/24] nspawn: use extract_first_word() --- src/nspawn/nspawn-setuid.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index d0e575fef2..fa2002d578 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -88,13 +88,12 @@ int change_uid_gid_raw( int change_uid_gid(const char *user, char **_home) { char *x, *u, *g, *h; - const char *word, *state; _cleanup_free_ gid_t *gids = NULL; _cleanup_free_ char *home = NULL, *line = NULL; _cleanup_fclose_ FILE *f = NULL; _cleanup_close_ int fd = -1; unsigned n_gids = 0; - size_t sz = 0, l; + size_t sz = 0; uid_t uid; gid_t gid; pid_t pid; @@ -208,16 +207,19 @@ int change_uid_gid(const char *user, char **_home) { x += strcspn(x, WHITESPACE); x += strspn(x, WHITESPACE); - FOREACH_WORD(word, l, x, state) { - char c[l+1]; + for (const char *p = x;;) { + _cleanup_free_ char *word = NULL; - memcpy(c, word, l); - c[l] = 0; + r = extract_first_word(&p, &word, NULL, 0); + if (r < 0) + return log_error_errno(r, "Failed to parse group data from getent: %m"); + if (r == 0) + break; if (!GREEDY_REALLOC(gids, sz, n_gids+1)) return log_oom(); - r = parse_gid(c, &gids[n_gids++]); + r = parse_gid(word, &gids[n_gids++]); if (r < 0) return log_error_errno(r, "Failed to parse group data from getent: %m"); } From ecaf258eb4fc997205159e588072668024a049ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 12:07:49 +0200 Subject: [PATCH 18/24] Use extract_first_word() in generated conf parsers --- src/shared/conf-parser.h | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/shared/conf-parser.h b/src/shared/conf-parser.h index 7c9f5531b0..57787ea033 100644 --- a/src/shared/conf-parser.h +++ b/src/shared/conf-parser.h @@ -239,10 +239,10 @@ typedef enum Disabled { #define DEFINE_CONFIG_PARSE_ENUMV(function, name, type, invalid, msg) \ CONFIG_PARSER_PROTOTYPE(function) { \ - type **enums = data, x, *ys; \ + type **enums = data; \ _cleanup_free_ type *xs = NULL; \ - const char *word, *state; \ - size_t l, i = 0; \ + size_t i = 0; \ + int r; \ \ assert(filename); \ assert(lvalue); \ @@ -255,29 +255,32 @@ typedef enum Disabled { \ *xs = invalid; \ \ - FOREACH_WORD(word, l, rvalue, state) { \ + for (const char *p = rvalue;;) { \ _cleanup_free_ char *en = NULL; \ - type *new_xs; \ + type x, *new_xs; \ \ - en = strndup(word, l); \ - if (!en) \ + r = extract_first_word(&p, &en, NULL, 0); \ + if (r == -ENOMEM) \ return log_oom(); \ + if (r < 0) \ + return log_syntax(unit, LOG_ERR, filename, line, 0, \ + msg ": %s", en); \ + if (r == 0) \ + break; \ \ if ((x = name##_from_string(en)) < 0) { \ - log_syntax(unit, LOG_WARNING, filename, line, 0, \ + log_syntax(unit, LOG_WARNING, filename, line, 0, \ msg ", ignoring: %s", en); \ continue; \ } \ \ - for (ys = xs; x != invalid && *ys != invalid; ys++) { \ - if (*ys == x) { \ - log_syntax(unit, LOG_NOTICE, filename, \ - line, 0, \ - "Duplicate entry, ignoring: %s", \ + for (type *ys = xs; x != invalid && *ys != invalid; ys++) \ + if (*ys == x) { \ + log_syntax(unit, LOG_NOTICE, filename, line, 0, \ + "Duplicate entry, ignoring: %s", \ en); \ x = invalid; \ } \ - } \ \ if (x == invalid) \ continue; \ @@ -292,6 +295,5 @@ typedef enum Disabled { *(xs + i) = invalid; \ } \ \ - free_and_replace(*enums, xs); \ - return 0; \ + return free_and_replace(*enums, xs); \ } From 0e8d1859383a4f333d7ffc801b17e33fa19df026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 14:27:14 +0200 Subject: [PATCH 19/24] shared/fstab-util: use free_and_str[n]dup() No functional change. I'm keeping this separate to make review easier. --- src/shared/fstab-util.c | 53 +++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/src/shared/fstab-util.c b/src/shared/fstab-util.c index 806dda8475..0e1b61aa95 100644 --- a/src/shared/fstab-util.c +++ b/src/shared/fstab-util.c @@ -81,16 +81,17 @@ int fstab_is_mount_point(const char *mount) { int fstab_filter_options(const char *opts, const char *names, const char **ret_namefound, char **ret_value, char **ret_filtered) { - const char *name, *n = NULL, *x; + const char *name, *namefound = NULL, *x; _cleanup_strv_free_ char **stor = NULL; _cleanup_free_ char *v = NULL, **strv = NULL; + int r; assert(names && *names); if (!opts) goto answer; - /* If !value and !filtered, this function is not allowed to fail. */ + /* If !ret_value and !ret_filtered, this function is not allowed to fail. */ if (!ret_filtered) { const char *word, *state; @@ -103,28 +104,23 @@ int fstab_filter_options(const char *opts, const char *names, if (!strneq(word, name, strlen(name))) continue; - /* we know that the string is NUL - * terminated, so *x is valid */ + /* We know that the string is NUL terminated, so *x is valid */ x = word + strlen(name); if (IN_SET(*x, '\0', '=', ',')) { - n = name; + namefound = name; if (ret_value) { - free(v); - if (IN_SET(*x, '\0', ',')) - v = NULL; - else { - assert(*x == '='); - x++; - v = strndup(x, l - strlen(name) - 1); - if (!v) - return -ENOMEM; - } + bool eq = *x == '='; + assert(eq || IN_SET(*x, ',', '\0')); + + r = free_and_strndup(&v, + eq ? x + 1 : NULL, + eq ? l - strlen(name) - 1 : 0); + if (r < 0) + return r; } } } } else { - char **t, **s; - stor = strv_split(opts, ","); if (!stor) return -ENOMEM; @@ -132,7 +128,8 @@ int fstab_filter_options(const char *opts, const char *names, if (!strv) return -ENOMEM; - for (s = t = strv; *s; s++) { + char **t = strv; + for (char **s = strv; *s; s++) { NULSTR_FOREACH(name, names) { x = startswith(*s, name); if (x && IN_SET(*x, '\0', '=')) @@ -144,18 +141,12 @@ int fstab_filter_options(const char *opts, const char *names, continue; found: /* Keep the last occurrence found */ - n = name; + namefound = name; if (ret_value) { - free(v); - if (*x == '\0') - v = NULL; - else { - assert(*x == '='); - x++; - v = strdup(x); - if (!v) - return -ENOMEM; - } + assert(IN_SET(*x, '=', '\0')); + r = free_and_strdup(&v, *x == '=' ? x + 1 : NULL); + if (r < 0) + return r; } } *t = NULL; @@ -163,7 +154,7 @@ int fstab_filter_options(const char *opts, const char *names, answer: if (ret_namefound) - *ret_namefound = n; + *ret_namefound = namefound; if (ret_filtered) { char *f; @@ -176,7 +167,7 @@ answer: if (ret_value) *ret_value = TAKE_PTR(v); - return !!n; + return !!namefound; } int fstab_extract_values(const char *opts, const char *name, char ***values) { From 45638a63c0998137187a48a521015da35ab15a3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 14:40:23 +0200 Subject: [PATCH 20/24] shared/fstab-util: replace FOREACH_WORD_SEPARATOR() with open-coded loop The tricky part here is that the function is not allowed to fail in this code path. Initially, I wanted to change the return value to allow it to fail, but this cascades through all the places where fstab_test_option() and friends are used; updating all those sites would be a lot of work. And since quoting is not allowed here, a simple loop with strcspn() is easy to do. --- src/shared/fstab-util.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/shared/fstab-util.c b/src/shared/fstab-util.c index 0e1b61aa95..d883eca5c7 100644 --- a/src/shared/fstab-util.c +++ b/src/shared/fstab-util.c @@ -94,12 +94,11 @@ int fstab_filter_options(const char *opts, const char *names, /* If !ret_value and !ret_filtered, this function is not allowed to fail. */ if (!ret_filtered) { - const char *word, *state; - size_t l; + for (const char *word = opts;;) { + const char *end = word + strcspn(word, ","); - FOREACH_WORD_SEPARATOR(word, l, opts, ",", state) NULSTR_FOREACH(name, names) { - if (l < strlen(name)) + if (end < word + strlen(name)) continue; if (!strneq(word, name, strlen(name))) continue; @@ -114,12 +113,20 @@ int fstab_filter_options(const char *opts, const char *names, r = free_and_strndup(&v, eq ? x + 1 : NULL, - eq ? l - strlen(name) - 1 : 0); + eq ? end - x - 1 : 0); if (r < 0) return r; } + + break; } } + + if (*end) + word = end + 1; + else + break; + } } else { stor = strv_split(opts, ","); if (!stor) From d59d954d7f6f5f3772b12e8078875183e0b27889 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 15:07:23 +0200 Subject: [PATCH 21/24] test-string-util: stop testing FOREACH_WORD --- src/test/test-string-util.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index b39fda0313..196c96aa8c 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -396,26 +396,34 @@ static void test_strcmp_ptr(void) { static void test_foreach_word(void) { log_info("/* %s */", __func__); - const char *word, *state; - size_t l; - int i = 0; - const char test[] = "test abc d\te f "; + const char *test = "test abc d\te f "; const char * const expected[] = { "test", "abc", "d", "e", "f", - "", - NULL }; - FOREACH_WORD(word, l, test, state) - assert_se(strneq(expected[i++], word, l)); + size_t i = 0; + int r; + for (const char *p = test;;) { + _cleanup_free_ char *word = NULL; + + r = extract_first_word(&p, &word, NULL, 0); + if (r == 0) { + assert_se(i == ELEMENTSOF(expected)); + break; + } + assert_se(r > 0); + + assert_se(streq(expected[i++], word)); + } } static void check(const char *test, char** expected, bool trailing) { - int i = 0, r; + size_t i = 0; + int r; printf("<<<%s>>>\n", test); for (;;) { From 0645b83a40d1c782f173c4d8440ab2fc82a75006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 16:21:14 +0200 Subject: [PATCH 22/24] 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. --- src/basic/strv.c | 38 -------- src/basic/strv.h | 14 ++- src/test/test-strv.c | 95 +++++++++---------- src/udev/udev-builtin.c | 8 +- src/udev/udev-event.c | 6 +- .../xdg-autostart-service.c | 6 +- 6 files changed, 67 insertions(+), 100 deletions(-) diff --git a/src/basic/strv.c b/src/basic/strv.c index a172ca2fe9..a5916926ff 100644 --- 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 bc0b04b56b..9e15820f28 100644 --- 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 fda5948f49..2e9922fa5e 100644 --- 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); + /* 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); + + /* 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 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); - 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); - /* 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)); + 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(" 'one' \" two\t three \"' four five", NULL, SPLIT_QUOTES | SPLIT_RELAX); - assert_se(l); - assert_se(strv_equal(l, (char**) input_table_quoted)); - - 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 = strv_split("", NULL)); + assert_se(strv_isempty(l)); + l = strv_free(l); + + 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, 0); + 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); + 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); + 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_full("", WHITESPACE, SPLIT_QUOTES | SPLIT_RELAX); - 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 b41fbfc39a..ac443d66e6 100644 --- 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 e1daac21ed..03e80b724d 100644 --- 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 4a30f9e433..efd2ae3ec6 100644 --- 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"); From 03b62851a90ba9a7bf74905458eafa8cd29a5430 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 31 Jul 2020 15:04:26 +0200 Subject: [PATCH 23/24] Remove FOREACH_WORD and friends --- .clang-format | 3 -- src/basic/string-util.c | 77 ----------------------------------------- src/basic/string-util.h | 18 ---------- 3 files changed, 98 deletions(-) diff --git a/.clang-format b/.clang-format index ab27960a67..8e5cfca535 100644 --- a/.clang-format +++ b/.clang-format @@ -74,9 +74,6 @@ ForEachMacros: - FOREACH_INOTIFY_EVENT - FOREACH_STRING - FOREACH_SUBSYSTEM - - _FOREACH_WORD - - FOREACH_WORD - - FOREACH_WORD_SEPARATOR - HASHMAP_FOREACH - HASHMAP_FOREACH_IDX - HASHMAP_FOREACH_KEY diff --git a/src/basic/string-util.c b/src/basic/string-util.c index c4f86b4ee2..ab725d0dab 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -112,83 +112,6 @@ char* first_word(const char *s, const char *word) { return (char*) p; } -static size_t strcspn_escaped(const char *s, const char *reject) { - bool escaped = false; - int n; - - for (n = 0; s[n] != '\0'; n++) { - if (escaped) - escaped = false; - else if (s[n] == '\\') - escaped = true; - else if (strchr(reject, s[n])) - break; - } - - return n; -} - -/* Split a string into words. */ -const char* split( - const char **state, - size_t *l, - const char *separator, - SplitFlags flags) { - - const char *current; - - assert(state); - assert(l); - - if (!separator) - separator = WHITESPACE; - - current = *state; - - if (*current == '\0') /* already at the end? */ - return NULL; - - current += strspn(current, separator); /* skip leading separators */ - if (*current == '\0') { /* at the end now? */ - *state = current; - return NULL; - } - - if (FLAGS_SET(flags, SPLIT_QUOTES)) { - - if (strchr(QUOTES, *current)) { - /* We are looking at a quote */ - *l = strcspn_escaped(current + 1, CHAR_TO_STR(*current)); - if (current[*l + 1] != *current || - (current[*l + 2] != 0 && !strchr(separator, current[*l + 2]))) { - /* right quote missing or garbage at the end */ - if (FLAGS_SET(flags, SPLIT_RELAX)) { - *state = current + *l + 1 + (current[*l + 1] != '\0'); - return current + 1; - } - *state = current; - return NULL; - } - *state = current++ + *l + 2; - - } else { - /* We are looking at a something that is not a quote */ - *l = strcspn_escaped(current, separator); - if (current[*l] && !strchr(separator, current[*l]) && !FLAGS_SET(flags, SPLIT_RELAX)) { - /* unfinished escape */ - *state = current; - return NULL; - } - *state = current + *l; - } - } else { - *l = strcspn(current, separator); - *state = current + *l; - } - - return current; -} - char *strnappend(const char *s, const char *suffix, size_t b) { size_t a; char *r; diff --git a/src/basic/string-util.h b/src/basic/string-util.h index 087fb7c907..cefbda3577 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -108,24 +108,6 @@ char *endswith_no_case(const char *s, const char *postfix) _pure_; char *first_word(const char *s, const char *word) _pure_; -typedef enum SplitFlags { - SPLIT_QUOTES = 0x01 << 0, - SPLIT_RELAX = 0x01 << 1, -} SplitFlags; - -/* Smelly. Do not use this anymore. Use extract_first_word() instead! */ -const char* split(const char **state, size_t *l, const char *separator, SplitFlags flags); - -/* Similar, don't use this anymore */ -#define FOREACH_WORD(word, length, s, state) \ - _FOREACH_WORD(word, length, s, WHITESPACE, 0, state) - -#define FOREACH_WORD_SEPARATOR(word, length, s, separator, state) \ - _FOREACH_WORD(word, length, s, separator, 0, state) - -#define _FOREACH_WORD(word, length, s, separator, flags, state) \ - for ((state) = (s), (word) = split(&(state), &(length), (separator), (flags)); (word); (word) = split(&(state), &(length), (separator), (flags))) - char *strnappend(const char *s, const char *suffix, size_t length); char *strjoin_real(const char *x, ...) _sentinel_; From 90e30d767a32dc93c9ee94ad8b8d665eecf79e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 3 Aug 2020 17:52:01 +0200 Subject: [PATCH 24/24] Rename strv_split_extract() to strv_split_full() Now that _full() is gone, we can rename _extract() to have the usual suffix we use for the more featureful version. --- src/basic/env-util.c | 2 +- src/basic/strv.c | 2 +- src/basic/strv.h | 4 +- src/journal-remote/journal-remote-main.c | 2 +- src/libsystemd/sd-daemon/sd-daemon.c | 2 +- src/locale/keymap-util.c | 6 +-- src/nspawn/nspawn-mount.c | 2 +- src/shared/bus-unit-util.c | 2 +- src/test/test-strv.c | 40 +++++++++---------- src/udev/udev-builtin.c | 2 +- src/udev/udev-event.c | 2 +- .../xdg-autostart-service.c | 2 +- 12 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/basic/env-util.c b/src/basic/env-util.c index b8dc98915f..179408c399 100644 --- a/src/basic/env-util.c +++ b/src/basic/env-util.c @@ -687,7 +687,7 @@ char **replace_env_argv(char **argv, char **env) { if (e) { int r; - r = strv_split_extract(&m, e, WHITESPACE, EXTRACT_RELAX|EXTRACT_UNQUOTE); + r = strv_split_full(&m, e, WHITESPACE, EXTRACT_RELAX|EXTRACT_UNQUOTE); if (r < 0) { ret[k] = NULL; strv_free(ret); diff --git a/src/basic/strv.c b/src/basic/strv.c index a5916926ff..e4ecf405b7 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -279,7 +279,7 @@ char **strv_split_newlines(const char *s) { return l; } -int strv_split_extract(char ***t, const char *s, const char *separators, ExtractFlags flags) { +int strv_split_full(char ***t, const char *s, const char *separators, ExtractFlags flags) { _cleanup_strv_free_ char **l = NULL; size_t n = 0, allocated = 0; int r; diff --git a/src/basic/strv.h b/src/basic/strv.h index 9e15820f28..9468edc6a6 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -74,12 +74,12 @@ static inline bool strv_isempty(char * const *l) { char **strv_split_newlines(const char *s); -int strv_split_extract(char ***t, const char *s, const char *separators, ExtractFlags flags); +int strv_split_full(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); + r = strv_split_full(&ret, s, separators, 0); if (r < 0) return NULL; diff --git a/src/journal-remote/journal-remote-main.c b/src/journal-remote/journal-remote-main.c index 77dfdefd64..0e028a9e4a 100644 --- a/src/journal-remote/journal-remote-main.c +++ b/src/journal-remote/journal-remote-main.c @@ -124,7 +124,7 @@ static int spawn_getter(const char *getter) { _cleanup_strv_free_ char **words = NULL; assert(getter); - r = strv_split_extract(&words, getter, WHITESPACE, EXTRACT_UNQUOTE); + r = strv_split_full(&words, getter, WHITESPACE, EXTRACT_UNQUOTE); if (r < 0) return log_error_errno(r, "Failed to split getter option: %m"); diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c index bbe7cd76d5..bdcbb106ce 100644 --- a/src/libsystemd/sd-daemon/sd-daemon.c +++ b/src/libsystemd/sd-daemon/sd-daemon.c @@ -101,7 +101,7 @@ _public_ int sd_listen_fds_with_names(int unset_environment, char ***names) { e = getenv("LISTEN_FDNAMES"); if (e) { - n_names = strv_split_extract(&l, e, ":", EXTRACT_DONT_COALESCE_SEPARATORS); + n_names = strv_split_full(&l, e, ":", EXTRACT_DONT_COALESCE_SEPARATORS); if (n_names < 0) { unsetenv_all(unset_environment); return n_names; diff --git a/src/locale/keymap-util.c b/src/locale/keymap-util.c index 233d081300..8e0cb74775 100644 --- a/src/locale/keymap-util.c +++ b/src/locale/keymap-util.c @@ -251,7 +251,7 @@ int x11_read_data(Context *c, sd_bus_message *m) { if (in_section && first_word(l, "Option")) { _cleanup_strv_free_ char **a = NULL; - r = strv_split_extract(&a, l, WHITESPACE, EXTRACT_UNQUOTE); + r = strv_split_full(&a, l, WHITESPACE, EXTRACT_UNQUOTE); if (r < 0) return r; @@ -274,7 +274,7 @@ int x11_read_data(Context *c, sd_bus_message *m) { } else if (!in_section && first_word(l, "Section")) { _cleanup_strv_free_ char **a = NULL; - r = strv_split_extract(&a, l, WHITESPACE, EXTRACT_UNQUOTE); + r = strv_split_full(&a, l, WHITESPACE, EXTRACT_UNQUOTE); if (r < 0) return -ENOMEM; @@ -489,7 +489,7 @@ static int read_next_mapping(const char* filename, if (IN_SET(l[0], 0, '#')) continue; - r = strv_split_extract(&b, l, WHITESPACE, EXTRACT_UNQUOTE); + r = strv_split_full(&b, l, WHITESPACE, EXTRACT_UNQUOTE); if (r < 0) return r; diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 5599c6a1b3..c49ed76979 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -301,7 +301,7 @@ int overlay_mount_parse(CustomMount **l, size_t *n, const char *s, bool read_onl CustomMount *m; int k; - k = strv_split_extract(&lower, s, ":", EXTRACT_DONT_COALESCE_SEPARATORS); + k = strv_split_full(&lower, s, ":", EXTRACT_DONT_COALESCE_SEPARATORS); if (k < 0) return k; if (k < 2) diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c index b6def087f6..6ae9f4b5a6 100644 --- a/src/shared/bus-unit-util.c +++ b/src/shared/bus-unit-util.c @@ -340,7 +340,7 @@ static int bus_append_exec_command(sd_bus_message *m, const char *field, const c return log_error_errno(r, "Failed to parse path: %m"); } - r = strv_split_extract(&l, eq, NULL, EXTRACT_UNQUOTE|EXTRACT_CUNESCAPE); + r = strv_split_full(&l, eq, NULL, EXTRACT_UNQUOTE|EXTRACT_CUNESCAPE); if (r < 0) return log_error_errno(r, "Failed to parse command line: %m"); diff --git a/src/test/test-strv.c b/src/test/test-strv.c index 2e9922fa5e..558ffeef51 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -238,7 +238,7 @@ static void test_strv_unquote(const char *quoted, char **list) { log_info("/* %s */", __func__); - r = strv_split_extract(&s, quoted, WHITESPACE, EXTRACT_UNQUOTE); + r = strv_split_full(&s, quoted, WHITESPACE, EXTRACT_UNQUOTE); assert_se(r == (int) strv_length(list)); assert_se(s); j = strv_join(s, " | "); @@ -257,7 +257,7 @@ static void test_invalid_unquote(const char *quoted) { log_info("/* %s */", __func__); - r = strv_split_extract(&s, quoted, WHITESPACE, EXTRACT_UNQUOTE); + r = strv_split_full(&s, quoted, WHITESPACE, EXTRACT_UNQUOTE); assert_se(s == NULL); assert_se(r == -EINVAL); } @@ -287,39 +287,39 @@ static void test_strv_split(void) { strv_free_erase(l); - assert_se(strv_split_extract(&l, " one two\t three", NULL, 0) == 3); + assert_se(strv_split_full(&l, " one two\t three", NULL, 0) == 3); assert_se(strv_equal(l, (char**) input_table_multiple)); strv_free_erase(l); - assert_se(strv_split_extract(&l, " 'one' \" two\t three \" ' four five'", NULL, EXTRACT_UNQUOTE) == 3); + assert_se(strv_split_full(&l, " 'one' \" two\t three \" ' four five'", NULL, EXTRACT_UNQUOTE) == 3); assert_se(strv_equal(l, (char**) input_table_quoted)); l = strv_free_erase(l); /* 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(strv_split_full(&l, " 'one' \" two\t three \" ' four five", NULL, EXTRACT_UNQUOTE) == -EINVAL); 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_split_full(&l, " 'one' \" two\t three \" ' four five", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 3); assert_se(strv_equal(l, (char**) input_table_quoted)); l = strv_free_erase(l); /* 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_split_full(&l, " 'one' \" two\t three \"' four five'", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 2); assert_se(strv_equal(l, (char**) input_table_quoted_joined)); l = strv_free_erase(l); - assert_se(strv_split_extract(&l, " 'one' \" two\t three \"' four five", NULL, + assert_se(strv_split_full(&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)); l = strv_free_erase(l); - assert_se(strv_split_extract(&l, "\\", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX | EXTRACT_CUNESCAPE_RELAX) == 1); + assert_se(strv_split_full(&l, "\\", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX | EXTRACT_CUNESCAPE_RELAX) == 1); assert_se(strv_equal(l, STRV_MAKE("\\"))); } @@ -337,22 +337,22 @@ static void test_strv_split_empty(void) { assert_se(strv_isempty(l)); l = strv_free(l); - assert_se(strv_split_extract(&l, "", NULL, 0) == 0); + assert_se(strv_split_full(&l, "", NULL, 0) == 0); assert_se(l); assert_se(strv_isempty(l)); l = strv_free(l); - assert_se(strv_split_extract(&l, "", NULL, EXTRACT_UNQUOTE) == 0); + assert_se(strv_split_full(&l, "", NULL, EXTRACT_UNQUOTE) == 0); assert_se(l); assert_se(strv_isempty(l)); l = strv_free(l); - assert_se(strv_split_extract(&l, "", WHITESPACE, EXTRACT_UNQUOTE) == 0); + assert_se(strv_split_full(&l, "", WHITESPACE, EXTRACT_UNQUOTE) == 0); assert_se(l); assert_se(strv_isempty(l)); l = strv_free(l); - assert_se(strv_split_extract(&l, "", WHITESPACE, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 0); + assert_se(strv_split_full(&l, "", WHITESPACE, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 0); assert_se(l); assert_se(strv_isempty(l)); strv_free(l); @@ -367,34 +367,34 @@ static void test_strv_split_empty(void) { assert_se(strv_isempty(l)); l = strv_free(l); - assert_se(strv_split_extract(&l, " ", NULL, 0) == 0); + assert_se(strv_split_full(&l, " ", NULL, 0) == 0); assert_se(l); assert_se(strv_isempty(l)); l = strv_free(l); - assert_se(strv_split_extract(&l, " ", WHITESPACE, EXTRACT_UNQUOTE) == 0); + assert_se(strv_split_full(&l, " ", WHITESPACE, EXTRACT_UNQUOTE) == 0); assert_se(l); assert_se(strv_isempty(l)); l = strv_free(l); - assert_se(strv_split_extract(&l, " ", NULL, EXTRACT_UNQUOTE) == 0); + assert_se(strv_split_full(&l, " ", NULL, EXTRACT_UNQUOTE) == 0); assert_se(l); assert_se(strv_isempty(l)); l = strv_free(l); - assert_se(strv_split_extract(&l, " ", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 0); + assert_se(strv_split_full(&l, " ", NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX) == 0); assert_se(l); assert_se(strv_isempty(l)); } -static void test_strv_split_extract(void) { +static void test_strv_split_full(void) { _cleanup_strv_free_ char **l = NULL; const char *str = ":foo\\:bar::waldo:"; int r; log_info("/* %s */", __func__); - r = strv_split_extract(&l, str, ":", EXTRACT_DONT_COALESCE_SEPARATORS); + r = strv_split_full(&l, str, ":", EXTRACT_DONT_COALESCE_SEPARATORS); assert_se(r == (int) strv_length(l)); assert_se(streq_ptr(l[0], "")); assert_se(streq_ptr(l[1], "foo:bar")); @@ -1023,7 +1023,7 @@ int main(int argc, char *argv[]) { test_strv_split(); test_strv_split_empty(); - test_strv_split_extract(); + test_strv_split_full(); test_strv_split_colon_pairs(); test_strv_split_newlines(); test_strv_split_nulstr(); diff --git a/src/udev/udev-builtin.c b/src/udev/udev-builtin.c index ac443d66e6..fdb98101d3 100644 --- a/src/udev/udev-builtin.c +++ b/src/udev/udev-builtin.c @@ -118,7 +118,7 @@ int udev_builtin_run(sd_device *dev, UdevBuiltinCommand cmd, const char *command if (!builtins[cmd]) return -EOPNOTSUPP; - r = strv_split_extract(&argv, command, NULL, + r = strv_split_full(&argv, command, NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX | EXTRACT_RETAIN_ESCAPE); if (r < 0) return r; diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 03e80b724d..616b493ff3 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -747,7 +747,7 @@ int udev_event_spawn(UdevEvent *event, return log_device_error_errno(event->dev, errno, "Failed to create pipe for command '%s': %m", cmd); - r = strv_split_extract(&argv, cmd, NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX | EXTRACT_RETAIN_ESCAPE); + r = strv_split_full(&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"); diff --git a/src/xdg-autostart-generator/xdg-autostart-service.c b/src/xdg-autostart-generator/xdg-autostart-service.c index efd2ae3ec6..80643236d5 100644 --- a/src/xdg-autostart-generator/xdg-autostart-service.c +++ b/src/xdg-autostart-generator/xdg-autostart-service.c @@ -385,7 +385,7 @@ int xdg_autostart_format_exec_start( * NOTE: Technically, XDG only specifies " as quotes, while this also * accepts '. */ - r = strv_split_extract(&exec_split, exec, NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX); + r = strv_split_full(&exec_split, exec, NULL, EXTRACT_UNQUOTE | EXTRACT_RELAX); if (r < 0) return r;