Merge pull request #14295 from poettering/greedy-alloc-round-up

add new GREEDY_ALLOC_ROUND_UP() to make strv_extend() a bit less slow

3 files changed, 56 insertions, 12 deletions

diff --git a/src/basic/macro.h b/src/basic/macro.h
index 18e5085669..5aa7f59c0b 100644
--- a/src/basic/macro.h
+++ b/src/basic/macro.h
@@ -163,6 +163,11 @@ static inline size_t ALIGN_TO(size_t l, size_t ali) {
 
 /* align to next higher power-of-2 (except for: 0 => 0, overflow => 0) */
 static inline unsigned long ALIGN_POWER2(unsigned long u) {
+
+        /* Avoid subtraction overflow */
+        if (u == 0)
+                return 0;
+
         /* clz(0) is undefined */
         if (u == 1)
                 return 1;
@@ -174,6 +179,29 @@ static inline unsigned long ALIGN_POWER2(unsigned long u) {
         return 1UL << (sizeof(u) * 8 - __builtin_clzl(u - 1UL));
 }
 
+static inline size_t GREEDY_ALLOC_ROUND_UP(size_t l) {
+        size_t m;
+
+        /* Round up allocation sizes a bit to some reasonable, likely larger value. This is supposed to be
+         * used for cases which are likely called in an allocation loop of some form, i.e. that repetitively
+         * grow stuff, for example strv_extend() and suchlike.
+         *
+         * Note the difference to GREEDY_REALLOC() here, as this helper operates on a single size value only,
+         * and rounds up to next multiple of 2, needing no further counter.
+         *
+         * Note the benefits of direct ALIGN_POWER2() usage: type-safety for size_t, sane handling for very
+         * small (i.e. <= 2) and safe handling for very large (i.e. > SSIZE_MAX) values. */
+
+        if (l <= 2)
+                return 2; /* Never allocate less than 2 of something.  */
+
+        m = ALIGN_POWER2(l);
+        if (m == 0) /* overflow? */
+                return l;
+
+        return m;
+}
+
 #ifndef __COVERITY__
 #  define VOID_0 ((void)0)
 #else
diff --git a/src/basic/strv.c b/src/basic/strv.c
index 30fab63074..92e528940a 100644
--- a/src/basic/strv.c
+++ b/src/basic/strv.c
@@ -193,7 +193,10 @@ int strv_extend_strv(char ***a, char **b, bool filter_duplicates) {
         p = strv_length(*a);
         q = strv_length(b);
 
-        t = reallocarray(*a, p + q + 1, sizeof(char *));
+        if (p >= SIZE_MAX - q)
+                return -ENOMEM;
+
+        t = reallocarray(*a, GREEDY_ALLOC_ROUND_UP(p + q + 1), sizeof(char *));
         if (!t)
                 return -ENOMEM;
 
@@ -383,19 +386,18 @@ char *strv_join_prefix(char **l, const char *separator, const char *prefix) {
 
 int strv_push(char ***l, char *value) {
         char **c;
-        size_t n, m;
+        size_t n;
 
         if (!value)
                 return 0;
 
         n = strv_length(*l);
 
-        /* Increase and check for overflow */
-        m = n + 2;
-        if (m < n)
+        /* Check for overflow */
+        if (n > SIZE_MAX-2)
                 return -ENOMEM;
 
-        c = reallocarray(*l, m, sizeof(char*));
+        c = reallocarray(*l, GREEDY_ALLOC_ROUND_UP(n + 2), sizeof(char*));
         if (!c)
                 return -ENOMEM;
 
@@ -408,19 +410,19 @@ int strv_push(char ***l, char *value) {
 
 int strv_push_pair(char ***l, char *a, char *b) {
         char **c;
-        size_t n, m;
+        size_t n;
 
         if (!a && !b)
                 return 0;
 
         n = strv_length(*l);
 
-        /* increase and check for overflow */
-        m = n + !!a + !!b + 1;
-        if (m < n)
+        /* Check for overflow */
+        if (n > SIZE_MAX-3)
                 return -ENOMEM;
 
-        c = reallocarray(*l, m, sizeof(char*));
+        /* increase and check for overflow */
+        c = reallocarray(*l, GREEDY_ALLOC_ROUND_UP(n + !!a + !!b + 1), sizeof(char*));
         if (!c)
                 return -ENOMEM;
 
@@ -846,8 +848,10 @@ int strv_extend_n(char ***l, const char *value, size_t n) {
         /* Adds the value n times to l */
 
         k = strv_length(*l);
+        if (n >= SIZE_MAX - k)
+                return -ENOMEM;
 
-        nl = reallocarray(*l, k + n + 1, sizeof(char *));
+        nl = reallocarray(*l, GREEDY_ALLOC_ROUND_UP(k + n + 1), sizeof(char *));
         if (!nl)
                 return -ENOMEM;
 
diff --git a/src/test/test-util.c b/src/test/test-util.c
index 61725bdf08..76dd72a598 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -26,7 +26,19 @@ static void test_align_power2(void) {
         assert_se(ALIGN_POWER2(1) == 1);
         assert_se(ALIGN_POWER2(2) == 2);
         assert_se(ALIGN_POWER2(3) == 4);
+        assert_se(ALIGN_POWER2(4) == 4);
+        assert_se(ALIGN_POWER2(5) == 8);
+        assert_se(ALIGN_POWER2(6) == 8);
+        assert_se(ALIGN_POWER2(7) == 8);
+        assert_se(ALIGN_POWER2(9) == 16);
+        assert_se(ALIGN_POWER2(10) == 16);
+        assert_se(ALIGN_POWER2(11) == 16);
         assert_se(ALIGN_POWER2(12) == 16);
+        assert_se(ALIGN_POWER2(13) == 16);
+        assert_se(ALIGN_POWER2(14) == 16);
+        assert_se(ALIGN_POWER2(15) == 16);
+        assert_se(ALIGN_POWER2(16) == 16);
+        assert_se(ALIGN_POWER2(17) == 32);
 
         assert_se(ALIGN_POWER2(ULONG_MAX) == 0);
         assert_se(ALIGN_POWER2(ULONG_MAX - 1) == 0);