From f41794d0360b8e3a705db58df14957ceb37c6f27 Mon Sep 17 00:00:00 2001 From: Jean-Sébastien Bour Date: Sat, 9 Jul 2016 02:38:00 +0200 Subject: basic/strv: exhibit strv_make_nulstr missing final NUL char (systemd/systemd#3689) --- src/test/test-strv.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/test-strv.c b/src/test/test-strv.c index f7a1217df7..91265c9cba 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -647,7 +647,9 @@ static void test_strv_extend_n(void) { static void test_strv_make_nulstr_one(char **l) { _cleanup_free_ char *b = NULL, *c = NULL; _cleanup_strv_free_ char **q = NULL; + const char *s = NULL; size_t n, m; + unsigned i = 0; assert_se(strv_make_nulstr(l, &b, &n) >= 0); assert_se(q = strv_parse_nulstr(b, n)); @@ -656,6 +658,11 @@ static void test_strv_make_nulstr_one(char **l) { 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)); } static void test_strv_make_nulstr(void) { -- cgit v1.2.3-54-g00ecf From b60df13b39c0237f9cb1114076464d2431e6bee5 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Sun, 17 Jul 2016 15:25:01 -0400 Subject: basic/strv: add an extra NUL after strings in strv_make_nulstr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit strv_make_nulstr was creating a nulstr which was not a valid nulstr, because it was missing the terminating NUL. This didn't cause any issues, because strv_parse_nulstr correctly parsed the result, using the separately specified length. But it's confusing to have something called nulstr which really isn't. It is likely that somebody will try to use strv_make_nulstr() in some other place, incorrectly. This patch changes strv_parse_nulstr() to produce a valid nulstr, and changes the output length parameter to be the minimum number of bytes which can be later on parsed by strv_parse_nulstr(). This allows the only user in ask-password-api to be slightly simplified. Based-on-patch-by: Jean-Sébastien Bour Fixes #3689. --- src/basic/strv.c | 29 +++++++++++++++++++++++++---- src/shared/ask-password-api.c | 6 +----- src/test/test-strv.c | 3 +-- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/basic/strv.c b/src/basic/strv.c index e0e2d1ebbe..98d6f61067 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -638,6 +638,17 @@ char **strv_remove(char **l, const char *s) { } 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. + */ + const char *p; unsigned c = 0, i = 0; char **v; @@ -700,6 +711,13 @@ char **strv_split_nulstr(const char *s) { } int strv_make_nulstr(char **l, char **p, size_t *q) { + /* 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. + */ + size_t n_allocated = 0, n = 0; _cleanup_free_ char *m = NULL; char **i; @@ -712,7 +730,7 @@ int strv_make_nulstr(char **l, char **p, size_t *q) { z = strlen(*i); - if (!GREEDY_REALLOC(m, n_allocated, n + z + 1)) + if (!GREEDY_REALLOC(m, n_allocated, n + z + 2)) return -ENOMEM; memcpy(m + n, *i, z + 1); @@ -723,11 +741,14 @@ int strv_make_nulstr(char **l, char **p, size_t *q) { m = new0(char, 1); if (!m) return -ENOMEM; - n = 0; - } + n = 1; + } else + /* make sure there is a second extra NUL at the end of resulting nulstr */ + m[n] = '\0'; + assert(n > 0); *p = m; - *q = n; + *q = n - 1; m = NULL; diff --git a/src/shared/ask-password-api.c b/src/shared/ask-password-api.c index a86b0db554..65151b19a6 100644 --- a/src/shared/ask-password-api.c +++ b/src/shared/ask-password-api.c @@ -139,11 +139,7 @@ static int add_to_keyring(const char *keyname, AskPasswordFlags flags, char **pa if (r < 0) return r; - /* Truncate trailing NUL */ - assert(n > 0); - assert(p[n-1] == 0); - - serial = add_key("user", keyname, p, n-1, KEY_SPEC_USER_KEYRING); + serial = add_key("user", keyname, p, n, KEY_SPEC_USER_KEYRING); memory_erase(p, n); if (serial == -1) return -errno; diff --git a/src/test/test-strv.c b/src/test/test-strv.c index 91265c9cba..841a36782f 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -659,9 +659,8 @@ static void test_strv_make_nulstr_one(char **l) { assert_se(m == n); assert_se(memcmp(b, c, m) == 0); - NULSTR_FOREACH(s, b) { + NULSTR_FOREACH(s, b) assert_se(streq(s, l[i++])); - } assert_se(i == strv_length(l)); } -- cgit v1.2.3-54-g00ecf