From 08af3cc5a581894afc9c48c6427fb0f82756bec2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 21:55:00 +0100 Subject: [PATCH 1/8] strv: move nulstr utilities to nulstr-util.[ch] Let's move them out of the generic, already very long strv.[ch] module into the more specific nulst-util.[ch] No code changes. --- src/basic/conf-files.c | 1 + src/basic/fileio.c | 1 + src/basic/nulstr-util.c | 113 ++++++++++++++++++++++ src/basic/nulstr-util.h | 15 +++ src/basic/process-util.c | 1 + src/basic/strv.c | 112 --------------------- src/basic/strv.h | 14 --- src/libsystemd/sd-bus/bus-creds.c | 1 + src/libsystemd/sd-path/sd-path.c | 1 + src/shared/ask-password-api.c | 1 + src/shared/condition.c | 3 +- src/systemctl/fuzz-systemctl-parse-argv.c | 1 + src/test/meson.build | 2 + src/test/test-nulstr-util.c | 39 ++++++++ src/test/test-strbuf.c | 1 + src/test/test-strv.c | 61 ------------ src/tmpfiles/tmpfiles.c | 1 + 17 files changed, 180 insertions(+), 188 deletions(-) create mode 100644 src/test/test-nulstr-util.c diff --git a/src/basic/conf-files.c b/src/basic/conf-files.c index 348f7dcc70..3fbb2cda00 100644 --- a/src/basic/conf-files.c +++ b/src/basic/conf-files.c @@ -13,6 +13,7 @@ #include "hashmap.h" #include "log.h" #include "macro.h" +#include "nulstr-util.h" #include "path-util.h" #include "set.h" #include "sort-util.h" diff --git a/src/basic/fileio.c b/src/basic/fileio.c index a5c44f184a..5078cae2df 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -21,6 +21,7 @@ #include "log.h" #include "macro.h" #include "mkdir.h" +#include "nulstr-util.h" #include "parse-util.h" #include "path-util.h" #include "socket-util.h" diff --git a/src/basic/nulstr-util.c b/src/basic/nulstr-util.c index 8eb19256a9..05154d71c2 100644 --- a/src/basic/nulstr-util.c +++ b/src/basic/nulstr-util.c @@ -2,6 +2,119 @@ #include "nulstr-util.h" #include "string-util.h" +#include "strv.h" + +char** strv_parse_nulstr(const char *s, size_t l) { + /* l is the length of the input data, which will be split at NULs into + * elements of the resulting strv. Hence, the number of items in the resulting strv + * will be equal to one plus the number of NUL bytes in the l bytes starting at s, + * unless s[l-1] is NUL, in which case the final empty string is not stored in + * the resulting strv, and length is equal to the number of NUL bytes. + * + * Note that contrary to a normal nulstr which cannot contain empty strings, because + * the input data is terminated by any two consequent NUL bytes, this parser accepts + * empty strings in s. + */ + + size_t c = 0, i = 0; + char **v; + + assert(s || l <= 0); + + if (l <= 0) + return new0(char*, 1); + + for (const char *p = s; p < s + l; p++) + if (*p == 0) + c++; + + if (s[l-1] != 0) + c++; + + v = new0(char*, c+1); + if (!v) + return NULL; + + for (const char *p = s; p < s + l; ) { + const char *e; + + e = memchr(p, 0, s + l - p); + + v[i] = strndup(p, e ? e - p : s + l - p); + if (!v[i]) { + strv_free(v); + return NULL; + } + + i++; + + if (!e) + break; + + p = e + 1; + } + + assert(i == c); + + return v; +} + +char** strv_split_nulstr(const char *s) { + char **r = NULL; + + NULSTR_FOREACH(i, s) + if (strv_extend(&r, i) < 0) { + strv_free(r); + return NULL; + } + + if (!r) + return strv_new(NULL); + + return r; +} + +int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) { + /* A valid nulstr with two NULs at the end will be created, but + * q will be the length without the two trailing NULs. Thus the output + * string is a valid nulstr and can be iterated over using NULSTR_FOREACH, + * and can also be parsed by strv_parse_nulstr as long as the length + * is provided separately. + */ + + _cleanup_free_ char *m = NULL; + size_t n = 0; + + assert(ret); + assert(ret_size); + + STRV_FOREACH(i, l) { + size_t z; + + z = strlen(*i); + + if (!GREEDY_REALLOC(m, n + z + 2)) + return -ENOMEM; + + memcpy(m + n, *i, z + 1); + n += z + 1; + } + + if (!m) { + m = new0(char, 2); + if (!m) + return -ENOMEM; + n = 1; + } else + /* make sure there is a second extra NUL at the end of resulting nulstr */ + m[n] = '\0'; + + assert(n > 0); + *ret = TAKE_PTR(m); + *ret_size = n - 1; + + return 0; +} const char* nulstr_get(const char *nulstr, const char *needle) { if (!nulstr) diff --git a/src/basic/nulstr-util.h b/src/basic/nulstr-util.h index 26bf7be871..8c2849b111 100644 --- a/src/basic/nulstr-util.h +++ b/src/basic/nulstr-util.h @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include #include #include @@ -15,3 +16,17 @@ const char* nulstr_get(const char *nulstr, const char *needle); static inline bool nulstr_contains(const char *nulstr, const char *needle) { return nulstr_get(nulstr, needle); } + +char** strv_parse_nulstr(const char *s, size_t l); +char** strv_split_nulstr(const char *s); +int strv_make_nulstr(char * const *l, char **p, size_t *n); + +static inline int strv_from_nulstr(char ***a, const char *nulstr) { + char **t; + + t = strv_split_nulstr(nulstr); + if (!t) + return -ENOMEM; + *a = t; + return 0; +} diff --git a/src/basic/process-util.c b/src/basic/process-util.c index dd913bc323..183ff8b84e 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -37,6 +37,7 @@ #include "missing_sched.h" #include "missing_syscall.h" #include "namespace-util.h" +#include "nulstr-util.h" #include "parse-util.h" #include "path-util.h" #include "process-util.h" diff --git a/src/basic/strv.c b/src/basic/strv.c index 7d9bf8d63a..74e87046cc 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -623,118 +623,6 @@ char** strv_remove(char **l, const char *s) { return l; } -char** strv_parse_nulstr(const char *s, size_t l) { - /* l is the length of the input data, which will be split at NULs into - * elements of the resulting strv. Hence, the number of items in the resulting strv - * will be equal to one plus the number of NUL bytes in the l bytes starting at s, - * unless s[l-1] is NUL, in which case the final empty string is not stored in - * the resulting strv, and length is equal to the number of NUL bytes. - * - * Note that contrary to a normal nulstr which cannot contain empty strings, because - * the input data is terminated by any two consequent NUL bytes, this parser accepts - * empty strings in s. - */ - - size_t c = 0, i = 0; - char **v; - - assert(s || l <= 0); - - if (l <= 0) - return new0(char*, 1); - - for (const char *p = s; p < s + l; p++) - if (*p == 0) - c++; - - if (s[l-1] != 0) - c++; - - v = new0(char*, c+1); - if (!v) - return NULL; - - for (const char *p = s; p < s + l; ) { - const char *e; - - e = memchr(p, 0, s + l - p); - - v[i] = strndup(p, e ? e - p : s + l - p); - if (!v[i]) { - strv_free(v); - return NULL; - } - - i++; - - if (!e) - break; - - p = e + 1; - } - - assert(i == c); - - return v; -} - -char** strv_split_nulstr(const char *s) { - char **r = NULL; - - NULSTR_FOREACH(i, s) - if (strv_extend(&r, i) < 0) { - strv_free(r); - return NULL; - } - - if (!r) - return strv_new(NULL); - - return r; -} - -int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) { - /* A valid nulstr with two NULs at the end will be created, but - * q will be the length without the two trailing NULs. Thus the output - * string is a valid nulstr and can be iterated over using NULSTR_FOREACH, - * and can also be parsed by strv_parse_nulstr as long as the length - * is provided separately. - */ - - _cleanup_free_ char *m = NULL; - size_t n = 0; - - assert(ret); - assert(ret_size); - - STRV_FOREACH(i, l) { - size_t z; - - z = strlen(*i); - - if (!GREEDY_REALLOC(m, n + z + 2)) - return -ENOMEM; - - memcpy(m + n, *i, z + 1); - n += z + 1; - } - - if (!m) { - m = new0(char, 2); - if (!m) - return -ENOMEM; - n = 1; - } else - /* make sure there is a second extra NUL at the end of resulting nulstr */ - m[n] = '\0'; - - assert(n > 0); - *ret = TAKE_PTR(m); - *ret_size = n - 1; - - return 0; -} - bool strv_overlap(char * const *a, char * const *b) { STRV_FOREACH(i, a) if (strv_contains(b, *i)) diff --git a/src/basic/strv.h b/src/basic/strv.h index d6f5ac6ba5..87a7038a54 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -124,20 +124,6 @@ static inline char *strv_join(char * const *l, const char *separator) { return strv_join_full(l, separator, NULL, false); } -char** strv_parse_nulstr(const char *s, size_t l); -char** strv_split_nulstr(const char *s); -int strv_make_nulstr(char * const *l, char **p, size_t *n); - -static inline int strv_from_nulstr(char ***a, const char *nulstr) { - char **t; - - t = strv_split_nulstr(nulstr); - if (!t) - return -ENOMEM; - *a = t; - return 0; -} - bool strv_overlap(char * const *a, char * const *b) _pure_; #define _STRV_FOREACH_BACKWARDS(s, l, h, i) \ diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c index 45e7473c29..6e53e942df 100644 --- a/src/libsystemd/sd-bus/bus-creds.c +++ b/src/libsystemd/sd-bus/bus-creds.c @@ -15,6 +15,7 @@ #include "fileio.h" #include "format-util.h" #include "hexdecoct.h" +#include "nulstr-util.h" #include "parse-util.h" #include "process-util.h" #include "string-util.h" diff --git a/src/libsystemd/sd-path/sd-path.c b/src/libsystemd/sd-path/sd-path.c index 601f61bf63..ded64a66ef 100644 --- a/src/libsystemd/sd-path/sd-path.c +++ b/src/libsystemd/sd-path/sd-path.c @@ -7,6 +7,7 @@ #include "fd-util.h" #include "fileio.h" #include "fs-util.h" +#include "nulstr-util.h" #include "path-lookup.h" #include "path-util.h" #include "string-util.h" diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index 84d7b31bad..e0488ff194 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -34,6 +34,7 @@ #include "memory-util.h" #include "missing_syscall.h" #include "mkdir-label.h" +#include "nulstr-util.h" #include "process-util.h" #include "random-util.h" #include "signal-util.h" diff --git a/src/shared/condition.c b/src/shared/condition.c index f404d99878..d5fdbbf9e0 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -34,12 +34,13 @@ #include "fs-util.h" #include "glob-util.h" #include "hostname-util.h" -#include "initrd-util.h" #include "ima-util.h" +#include "initrd-util.h" #include "limits-util.h" #include "list.h" #include "macro.h" #include "mountpoint-util.h" +#include "nulstr-util.h" #include "os-util.h" #include "parse-util.h" #include "path-util.h" diff --git a/src/systemctl/fuzz-systemctl-parse-argv.c b/src/systemctl/fuzz-systemctl-parse-argv.c index 588c8b56c5..92f6ecaa8d 100644 --- a/src/systemctl/fuzz-systemctl-parse-argv.c +++ b/src/systemctl/fuzz-systemctl-parse-argv.c @@ -6,6 +6,7 @@ #include "env-util.h" #include "fd-util.h" #include "fuzz.h" +#include "nulstr-util.h" #include "selinux-util.h" #include "static-destruct.h" #include "stdio-util.h" diff --git a/src/test/meson.build b/src/test/meson.build index 9d193651ed..4f72e00f57 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -557,6 +557,8 @@ tests += [ [files('test-strv.c')], + [files('test-nulstr-util.c')], + [files('test-path-util.c')], [files('test-rm-rf.c')], diff --git a/src/test/test-nulstr-util.c b/src/test/test-nulstr-util.c new file mode 100644 index 0000000000..e707416abf --- /dev/null +++ b/src/test/test-nulstr-util.c @@ -0,0 +1,39 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "alloc-util.h" +#include "nulstr-util.h" +#include "strv.h" +#include "tests.h" + +TEST(strv_split_nulstr) { + _cleanup_strv_free_ char **l = NULL; + const char nulstr[] = "str0\0str1\0str2\0str3\0"; + + l = strv_split_nulstr(nulstr); + assert_se(l); + + assert_se(streq(l[0], "str0")); + assert_se(streq(l[1], "str1")); + assert_se(streq(l[2], "str2")); + assert_se(streq(l[3], "str3")); +} + +TEST(strv_parse_nulstr) { + _cleanup_strv_free_ char **l = NULL; + const char nulstr[] = "hoge\0hoge2\0hoge3\0\0hoge5\0\0xxx"; + + l = strv_parse_nulstr(nulstr, sizeof(nulstr)-1); + assert_se(l); + puts("Parse nulstr:"); + strv_print(l); + + assert_se(streq(l[0], "hoge")); + assert_se(streq(l[1], "hoge2")); + assert_se(streq(l[2], "hoge3")); + assert_se(streq(l[3], "")); + assert_se(streq(l[4], "hoge5")); + assert_se(streq(l[5], "")); + assert_se(streq(l[6], "xxx")); +} + +DEFINE_TEST_MAIN(LOG_INFO); diff --git a/src/test/test-strbuf.c b/src/test/test-strbuf.c index 5c12a0597a..f69e80ca5a 100644 --- a/src/test/test-strbuf.c +++ b/src/test/test-strbuf.c @@ -2,6 +2,7 @@ #include +#include "nulstr-util.h" #include "strbuf.h" #include "string-util.h" #include "strv.h" diff --git a/src/test/test-strv.c b/src/test/test-strv.c index debb922f85..9208faafa4 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -2,7 +2,6 @@ #include "alloc-util.h" #include "escape.h" -#include "nulstr-util.h" #include "string-util.h" #include "strv.h" #include "tests.h" @@ -487,37 +486,6 @@ TEST(strv_split_newlines_full) { assert_se(strv_equal(l, (char**) input_table_retain_escape)); } -TEST(strv_split_nulstr) { - _cleanup_strv_free_ char **l = NULL; - const char nulstr[] = "str0\0str1\0str2\0str3\0"; - - l = strv_split_nulstr (nulstr); - assert_se(l); - - assert_se(streq(l[0], "str0")); - assert_se(streq(l[1], "str1")); - assert_se(streq(l[2], "str2")); - assert_se(streq(l[3], "str3")); -} - -TEST(strv_parse_nulstr) { - _cleanup_strv_free_ char **l = NULL; - const char nulstr[] = "hoge\0hoge2\0hoge3\0\0hoge5\0\0xxx"; - - l = strv_parse_nulstr(nulstr, sizeof(nulstr)-1); - assert_se(l); - puts("Parse nulstr:"); - strv_print(l); - - assert_se(streq(l[0], "hoge")); - assert_se(streq(l[1], "hoge2")); - assert_se(streq(l[2], "hoge3")); - assert_se(streq(l[3], "")); - assert_se(streq(l[4], "hoge5")); - assert_se(streq(l[5], "")); - assert_se(streq(l[6], "xxx")); -} - TEST(strv_overlap) { const char * const input_table[] = { "one", @@ -945,35 +913,6 @@ TEST(strv_extend_n) { assert_se(v[1] == NULL); } -static void test_strv_make_nulstr_one(char **l) { - _cleanup_free_ char *b = NULL, *c = NULL; - _cleanup_strv_free_ char **q = NULL; - size_t n, m; - unsigned i = 0; - - log_info("/* %s */", __func__); - - assert_se(strv_make_nulstr(l, &b, &n) >= 0); - assert_se(q = strv_parse_nulstr(b, n)); - assert_se(strv_equal(l, q)); - - assert_se(strv_make_nulstr(q, &c, &m) >= 0); - assert_se(m == n); - assert_se(memcmp(b, c, m) == 0); - - NULSTR_FOREACH(s, b) - assert_se(streq(s, l[i++])); - assert_se(i == strv_length(l)); -} - -TEST(strv_make_nulstr) { - test_strv_make_nulstr_one(NULL); - test_strv_make_nulstr_one(STRV_MAKE(NULL)); - test_strv_make_nulstr_one(STRV_MAKE("foo")); - test_strv_make_nulstr_one(STRV_MAKE("foo", "bar")); - test_strv_make_nulstr_one(STRV_MAKE("foo", "bar", "quuux")); -} - TEST(foreach_string) { const char * const t[] = { "foo", diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index d3258ec701..557492f958 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -50,6 +50,7 @@ #include "mkdir-label.h" #include "mount-util.h" #include "mountpoint-util.h" +#include "nulstr-util.h" #include "offline-passwd.h" #include "pager.h" #include "parse-argument.h" From 7f0f54050006f6fe44a81e8327a858a0aea20bc9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 21:57:28 +0100 Subject: [PATCH 2/8] nulstr-util: modernize strv_from_nulstr() a bit --- src/basic/nulstr-util.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/basic/nulstr-util.h b/src/basic/nulstr-util.h index 8c2849b111..19f4edd384 100644 --- a/src/basic/nulstr-util.h +++ b/src/basic/nulstr-util.h @@ -2,6 +2,7 @@ #pragma once #include +#include #include #include @@ -21,12 +22,15 @@ char** strv_parse_nulstr(const char *s, size_t l); char** strv_split_nulstr(const char *s); int strv_make_nulstr(char * const *l, char **p, size_t *n); -static inline int strv_from_nulstr(char ***a, const char *nulstr) { +static inline int strv_from_nulstr(char ***ret, const char *nulstr) { char **t; + assert(ret); + t = strv_split_nulstr(nulstr); if (!t) return -ENOMEM; - *a = t; + + *ret = t; return 0; } From 8ba17a319bdbcacf16b2dfe0a759d0e43971e71d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 21:59:41 +0100 Subject: [PATCH 3/8] nulstr-util: rebreak comments --- src/basic/nulstr-util.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/basic/nulstr-util.c b/src/basic/nulstr-util.c index 05154d71c2..1b6af6ffd7 100644 --- a/src/basic/nulstr-util.c +++ b/src/basic/nulstr-util.c @@ -5,16 +5,13 @@ #include "strv.h" char** strv_parse_nulstr(const char *s, size_t l) { - /* l is the length of the input data, which will be split at NULs into - * elements of the resulting strv. Hence, the number of items in the resulting strv - * will be equal to one plus the number of NUL bytes in the l bytes starting at s, - * unless s[l-1] is NUL, in which case the final empty string is not stored in - * the resulting strv, and length is equal to the number of NUL bytes. + /* l is the length of the input data, which will be split at NULs into elements of the resulting + * strv. Hence, the number of items in the resulting strv will be equal to one plus the number of NUL + * bytes in the l bytes starting at s, unless s[l-1] is NUL, in which case the final empty string is + * not stored in the resulting strv, and length is equal to the number of NUL bytes. * - * Note that contrary to a normal nulstr which cannot contain empty strings, because - * the input data is terminated by any two consequent NUL bytes, this parser accepts - * empty strings in s. - */ + * Note that contrary to a normal nulstr which cannot contain empty strings, because the input data + * is terminated by any two consequent NUL bytes, this parser accepts empty strings in s. */ size_t c = 0, i = 0; char **v; @@ -75,12 +72,10 @@ char** strv_split_nulstr(const char *s) { } int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) { - /* A valid nulstr with two NULs at the end will be created, but - * q will be the length without the two trailing NULs. Thus the output - * string is a valid nulstr and can be iterated over using NULSTR_FOREACH, - * and can also be parsed by strv_parse_nulstr as long as the length - * is provided separately. - */ + /* A valid nulstr with two NULs at the end will be created, but q will be the length without the two + * trailing NULs. Thus the output string is a valid nulstr and can be iterated over using + * NULSTR_FOREACH(), and can also be parsed by strv_parse_nulstr() as long as the length is provided + * separately. */ _cleanup_free_ char *m = NULL; size_t n = 0; From eecac5053b1aff31f43df7aa053a4561b5a8740c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 22:01:03 +0100 Subject: [PATCH 4/8] nulstr-util: use _cleanup_strv_free_() where appropriate --- src/basic/nulstr-util.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/basic/nulstr-util.c b/src/basic/nulstr-util.c index 1b6af6ffd7..e8e5629e95 100644 --- a/src/basic/nulstr-util.c +++ b/src/basic/nulstr-util.c @@ -13,8 +13,8 @@ char** strv_parse_nulstr(const char *s, size_t l) { * Note that contrary to a normal nulstr which cannot contain empty strings, because the input data * is terminated by any two consequent NUL bytes, this parser accepts empty strings in s. */ + _cleanup_strv_free_ char **v = NULL; size_t c = 0, i = 0; - char **v; assert(s || l <= 0); @@ -38,10 +38,8 @@ char** strv_parse_nulstr(const char *s, size_t l) { e = memchr(p, 0, s + l - p); v[i] = strndup(p, e ? e - p : s + l - p); - if (!v[i]) { - strv_free(v); + if (!v[i]) return NULL; - } i++; @@ -53,22 +51,20 @@ char** strv_parse_nulstr(const char *s, size_t l) { assert(i == c); - return v; + return TAKE_PTR(v); } char** strv_split_nulstr(const char *s) { - char **r = NULL; + _cleanup_strv_free_ char **r = NULL; NULSTR_FOREACH(i, s) - if (strv_extend(&r, i) < 0) { - strv_free(r); + if (strv_extend(&r, i) < 0) return NULL; - } if (!r) return strv_new(NULL); - return r; + return TAKE_PTR(r); } int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) { From 1ef970377485499b1b62e1150fec044aa852b7a1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 22:04:37 +0100 Subject: [PATCH 5/8] nulstr-util: use memdup_suffix0() where appropriate if the nulstr is not nul-terminated, we shouldn't use strndup() but memdup_suffix0(), to not trip up static analyzers which imply we are duping a string here. --- src/basic/nulstr-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/nulstr-util.c b/src/basic/nulstr-util.c index e8e5629e95..2da64fcbe9 100644 --- a/src/basic/nulstr-util.c +++ b/src/basic/nulstr-util.c @@ -37,7 +37,7 @@ char** strv_parse_nulstr(const char *s, size_t l) { e = memchr(p, 0, s + l - p); - v[i] = strndup(p, e ? e - p : s + l - p); + v[i] = memdup_suffix0(p, e ? e - p : s + l - p); if (!v[i]) return NULL; From db645f936f2b686e05524e198df00112acaff3c6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 22:07:43 +0100 Subject: [PATCH 6/8] nulstr-util: don't use 'r' for anything but integer return values --- src/basic/nulstr-util.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/basic/nulstr-util.c b/src/basic/nulstr-util.c index 2da64fcbe9..67226d208c 100644 --- a/src/basic/nulstr-util.c +++ b/src/basic/nulstr-util.c @@ -55,16 +55,18 @@ char** strv_parse_nulstr(const char *s, size_t l) { } char** strv_split_nulstr(const char *s) { - _cleanup_strv_free_ char **r = NULL; + _cleanup_strv_free_ char **l = NULL; + + /* This parses a nulstr, without specification of size, and stops at an empty string. This cannot + * parse nulstrs with embedded empty strings hence, as an empty string is an end marker. Use + * strv_parse_nulstr() above to parse a nulstr with embedded empty strings (which however requires a + * size to be specified) */ NULSTR_FOREACH(i, s) - if (strv_extend(&r, i) < 0) + if (strv_extend(&l, i) < 0) return NULL; - if (!r) - return strv_new(NULL); - - return TAKE_PTR(r); + return l ? TAKE_PTR(l) : strv_new(NULL); } int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) { From 76078ad850990fdcd42f82b7add35f73156e35bc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 23:17:12 +0100 Subject: [PATCH 7/8] nulstr-util: fix corner cases of strv_make_nulstr() Let's change the return semantics of strv_make_nulstr() so that we can properly distuingish the case where we have a no entries in the nulstr from the case where we have a single empty string in a nulstr. Previously we couldn't distuingish those, we'd in both cases return a size of zero, and a buffer with two NUL bytes. With this change, we'll still return a buffer with two NULL bytes, but for the case where no entries are defined we'll return a size of zero, and where we have two a size of one. This is a good idea, as it makes sure we can properly handle all corner cases. Nowadays the function is used by one place only: ask-password-api.c. The corner case never mattered there, since it was used to serialize passwords, and it was known that there was exactly one password, not less. But let's clean this up. This means the subtraction of the final NUL byte now happens in ask-password-api.c instead. --- src/basic/nulstr-util.c | 22 ++++++++++++++-------- src/shared/ask-password-api.c | 4 ++++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/basic/nulstr-util.c b/src/basic/nulstr-util.c index 67226d208c..44b88ca753 100644 --- a/src/basic/nulstr-util.c +++ b/src/basic/nulstr-util.c @@ -70,10 +70,15 @@ char** strv_split_nulstr(const char *s) { } int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) { - /* A valid nulstr with two NULs at the end will be created, but q will be the length without the two - * trailing NULs. Thus the output string is a valid nulstr and can be iterated over using - * NULSTR_FOREACH(), and can also be parsed by strv_parse_nulstr() as long as the length is provided - * separately. */ + /* Builds a nulstr and returns it together with the size. An extra NUL byte will be appended (⚠️ but + * not included in the size! ⚠️). This is done so that the nulstr can be used both in + * strv_parse_nulstr() and in NULSTR_FOREACH()/strv_split_nulstr() contexts, i.e. with and without a + * size parameter. In the former case we can include empty strings, in the latter case we cannot (as + * that is the end marker). + * + * When NULSTR_FOREACH()/strv_split_nulstr() is used it is often assumed that the nulstr ends in two + * NUL bytes (which it will, if not empty). To ensure that this assumption *always* holds, we'll + * return a buffer with two NUL bytes in that case, but return a size of zero. */ _cleanup_free_ char *m = NULL; size_t n = 0; @@ -94,17 +99,18 @@ int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) { } if (!m) { + /* return a buffer with an extra NUL, so that the assumption that we always have two trailing NULs holds */ m = new0(char, 2); if (!m) return -ENOMEM; - n = 1; + + n = 0; } else - /* make sure there is a second extra NUL at the end of resulting nulstr */ + /* Make sure there is a second extra NUL at the end of resulting nulstr (not counted in return size) */ m[n] = '\0'; - assert(n > 0); *ret = TAKE_PTR(m); - *ret_size = n - 1; + *ret_size = n; return 0; } diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index e0488ff194..e7db23c201 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -113,6 +113,10 @@ static int add_to_keyring(const char *keyname, AskPasswordFlags flags, char **pa if (r < 0) return r; + /* chop off the final NUL byte. We do this because we want to use the separator NUL bytes only if we + * have multiple passwords. */ + n = LESS_BY(n, (size_t) 1); + serial = add_key("user", keyname, p, n, KEY_SPEC_USER_KEYRING); if (serial == -1) return -errno; From a5a318b664461a63dfed3119f8be7c3d5d58ea1c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Nov 2022 23:26:08 +0100 Subject: [PATCH 8/8] tests: add tests for various corner cases of nulstr --- src/test/test-nulstr-util.c | 121 ++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/src/test/test-nulstr-util.c b/src/test/test-nulstr-util.c index e707416abf..a068e5f870 100644 --- a/src/test/test-nulstr-util.c +++ b/src/test/test-nulstr-util.c @@ -34,6 +34,127 @@ TEST(strv_parse_nulstr) { assert_se(streq(l[4], "hoge5")); assert_se(streq(l[5], "")); assert_se(streq(l[6], "xxx")); + strv_free(l); + + l = strv_parse_nulstr((const char[0]) {}, 0); + assert_se(l); + assert_se(strv_isempty(l)); + strv_free(l); + + l = strv_parse_nulstr((const char[1]) { 0 }, 1); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE(""))); + strv_free(l); + + l = strv_parse_nulstr((const char[1]) { 'x' }, 1); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("x"))); + strv_free(l); + + l = strv_parse_nulstr((const char[2]) { 0, 0 }, 2); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("", ""))); + strv_free(l); + + l = strv_parse_nulstr((const char[2]) { 'x', 0 }, 2); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("x"))); + strv_free(l); + + l = strv_parse_nulstr((const char[3]) { 0, 0, 0 }, 3); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("", "", ""))); + strv_free(l); + + l = strv_parse_nulstr((const char[3]) { 'x', 0, 0 }, 3); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("x", ""))); + strv_free(l); + + l = strv_parse_nulstr((const char[3]) { 0, 'x', 0 }, 3); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("", "x"))); + strv_free(l); + + l = strv_parse_nulstr((const char[3]) { 0, 0, 'x' }, 3); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("", "", "x"))); + strv_free(l); + + l = strv_parse_nulstr((const char[3]) { 'x', 'x', 0 }, 3); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("xx"))); + strv_free(l); + + l = strv_parse_nulstr((const char[3]) { 0, 'x', 'x' }, 3); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("", "xx"))); + strv_free(l); + + l = strv_parse_nulstr((const char[3]) { 'x', 0, 'x' }, 3); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("x", "x"))); + strv_free(l); + + l = strv_parse_nulstr((const char[3]) { 'x', 'x', 'x' }, 3); + assert_se(l); + assert_se(strv_equal(l, STRV_MAKE("xxx"))); +} + +static void test_strv_make_nulstr_one(char **l) { + _cleanup_free_ char *b = NULL, *c = NULL; + _cleanup_strv_free_ char **q = NULL; + size_t n, m; + unsigned i = 0; + + log_info("/* %s */", __func__); + + assert_se(strv_make_nulstr(l, &b, &n) >= 0); + assert_se(q = strv_parse_nulstr(b, n)); + assert_se(strv_equal(l, q)); + + assert_se(strv_make_nulstr(q, &c, &m) >= 0); + assert_se(memcmp_nn(b, n, c, m) == 0); + + NULSTR_FOREACH(s, b) + assert_se(streq(s, l[i++])); + assert_se(i == strv_length(l)); +} + +TEST(strv_make_nulstr) { + test_strv_make_nulstr_one(NULL); + test_strv_make_nulstr_one(STRV_MAKE(NULL)); + test_strv_make_nulstr_one(STRV_MAKE("foo")); + test_strv_make_nulstr_one(STRV_MAKE("foo", "bar")); + test_strv_make_nulstr_one(STRV_MAKE("foo", "bar", "quuux")); +} + +static void test_strv_make_nulstr_binary_one(char **l, const char *b, size_t n) { + _cleanup_strv_free_ char **z = NULL; + _cleanup_free_ char *a = NULL; + size_t m; + + assert_se(strv_make_nulstr(l, &a, &m) >= 0); + assert_se(memcmp_nn(a, m, b, n) == 0); + assert_se(z = strv_parse_nulstr(a, m)); + assert_se(strv_equal(l, z)); +} + +TEST(strv_make_nulstr_binary) { + test_strv_make_nulstr_binary_one(NULL, (const char[0]) {}, 0); + test_strv_make_nulstr_binary_one(STRV_MAKE(NULL), (const char[0]) {}, 0); + test_strv_make_nulstr_binary_one(STRV_MAKE(""), (const char[1]) { 0 }, 1); + test_strv_make_nulstr_binary_one(STRV_MAKE("", ""), (const char[2]) { 0, 0 }, 2); + test_strv_make_nulstr_binary_one(STRV_MAKE("x", ""), (const char[3]) { 'x', 0, 0 }, 3); + test_strv_make_nulstr_binary_one(STRV_MAKE("", "x"), (const char[3]) { 0, 'x', 0 }, 3); + test_strv_make_nulstr_binary_one(STRV_MAKE("", "", ""), (const char[3]) { 0, 0, 0 }, 3); + test_strv_make_nulstr_binary_one(STRV_MAKE("x", "", ""), (const char[4]) { 'x', 0, 0, 0 }, 4); + test_strv_make_nulstr_binary_one(STRV_MAKE("", "x", ""), (const char[4]) { 0, 'x', 0, 0 }, 4); + test_strv_make_nulstr_binary_one(STRV_MAKE("", "", "x"), (const char[4]) { 0, 0, 'x', 0 }, 4); + test_strv_make_nulstr_binary_one(STRV_MAKE("x", "x", ""), (const char[5]) { 'x', 0, 'x', 0, 0 }, 5); + test_strv_make_nulstr_binary_one(STRV_MAKE("", "x", "x"), (const char[5]) { 0, 'x', 0, 'x', 0 }, 5); + test_strv_make_nulstr_binary_one(STRV_MAKE("x", "", "x"), (const char[5]) { 'x', 0, 0, 'x', 0 }, 5); + test_strv_make_nulstr_binary_one(STRV_MAKE("x", "x", "x"), (const char[6]) { 'x', 0, 'x', 0, 'x', 0 }, 6); } DEFINE_TEST_MAIN(LOG_INFO);