basic/strv: add an extra NUL after strings in strv_make_nulstr

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 <jean-sebastien@bour.name>
Fixes #3689.

diff --git a/src/basic/strv.c b/src/basic/strv.c
index e0e2d1ebbeaa483d7c3ad749ef887e5538d6e4cd..98d6f610674091d0ee541525eac48e810af5c135 100644 (file)
--- 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 a86b0db5544c89f5a1425107cf2199a9973f7b2a..65151b19a6ed7b2ac7db4f7572ac6e593b0ddf03 100644 (file)
--- 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 91265c9cbab759d257394655eab9f6ef09afb3a7..841a36782f4f6a080195b518a447943b45cc01af 100644 (file)
--- 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));
 }