basic/strv: avoid potential UB with references to array[-1]

"""
Given an array a[N] of N elements of type T:
- Forming a pointer &a[i] (or a + i) with 0 ≤ i ≤ N is safe.
- Forming a pointer &a[i] with i < 0 or i > N causes undefined behavior.
- Dereferencing a pointer &a[i] with 0 ≤ i < N is safe.
- Dereferencing a pointer &a[i] with i < 0 or i ≥ N causes undefined behavior.
"""

As pointed by by @medhefgo, here we were forming a pointer to a[-1]. a itself
wasn't NULL, so a > 0, and a-1 was also >= 0, and this didn't seem to cause any
problems. But it's better to be formally correct, especially if we move the
code to src/fundamental/ later on and compile it differently.

Compilation shows no size change (with -O0 -g) on build/systemd, so this should
have no effect whatsoever.

diff --git a/src/basic/strv.h b/src/basic/strv.h
index 985499272f9eb00ac8c87619216589e67396a2f1..bc76a2861cb322cc33bbfe5d42cabdeb21d2e3cd 100644 (file)
--- a/src/basic/strv.h
+++ b/src/basic/strv.h
@@ -133,8 +133,8 @@ bool strv_overlap(char * const *a, char * const *b) _pure_;
                                 size_t _len = strv_length(h);           \
                                 _len > 0 ? h + _len - 1 : NULL;         \
                         });                                             \
-             i && (s = i) >= h;                                         \
-             i--)
+             (s = i);                                                   \
+             i > h ? i-- : (i = NULL))
 
 #define STRV_FOREACH_BACKWARDS(s, l)                                    \
         _STRV_FOREACH_BACKWARDS(s, l, UNIQ_T(h, UNIQ), UNIQ_T(i, UNIQ))

diff --git a/src/test/test-strv.c b/src/test/test-strv.c
index ae3e2798c2bef23bbb2852e664001b264ac93f16..edb782eb0d74a612a99d355525c2de1cd2006aa8 100644 (file)
--- a/src/test/test-strv.c
+++ b/src/test/test-strv.c
@@ -639,8 +639,13 @@ TEST(strv_foreach_backwards) {
         STRV_FOREACH_BACKWARDS(check, (char**) NULL)
                 assert_not_reached();
 
-        STRV_FOREACH_BACKWARDS(check, (char**) { NULL })
+        STRV_FOREACH_BACKWARDS(check, STRV_MAKE_EMPTY)
                 assert_not_reached();
+
+        unsigned count = 0;
+        STRV_FOREACH_BACKWARDS(check, STRV_MAKE("ONE"))
+                count++;
+        assert_se(count == 1);
 }
 
 TEST(strv_foreach_pair) {