alloc-util: simplify GREEDY_REALLOC() logic by relying on malloc_usab…

…le_size()

We recently started making more use of malloc_usable_size() and rely on
it (see the string_erase() story). Given that we don't really support
sytems where malloc_usable_size() cannot be trusted beyond statistics
anyway, let's go fully in and rework GREEDY_REALLOC() on top of it:
instead of passing around and maintaining the currenly allocated size
everywhere, let's just derive it automatically from
malloc_usable_size().

I am mostly after this for the simplicity this brings. It also brings
minor efficiency improvements I guess, but things become so much nicer
to look at if we can avoid these allocation size variables everywhere.

Note that the malloc_usable_size() man page says relying on it wasn't
"good programming practice", but I think it does this for reasons that
don't apply here: the greedy realloc logic specifically doesn't rely on
the returned extra size, beyond the fact that it is equal or larger than
what was requested.

(This commit was supposed to be a quick patch btw, but apparently we use
the greedy realloc stuff quite a bit across the codebase, so this ends
up touching *a*lot* of code.)

src/analyze/analyze-security.c

@@ -2116,7 +2116,7 @@ int analyze_security(sd_bus *bus, char **units, AnalyzeSecurityFlags flags) {
                 _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
                 _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
                 _cleanup_strv_free_ char **list = NULL;
-                size_t allocated = 0, n = 0;
+                size_t n = 0;
                 char **i;
 
                 r = sd_bus_call_method(
@@ -2148,7 +2148,7 @@ int analyze_security(sd_bus *bus, char **units, AnalyzeSecurityFlags flags) {
                         if (!endswith(info.id, ".service"))
                                 continue;
 
-                        if (!GREEDY_REALLOC(list, allocated, n + 2))
+                        if (!GREEDY_REALLOC(list, n + 2))
                                 return log_oom();
 
                         copy = strdup(info.id);

src/analyze/analyze.c

@@ -342,7 +342,7 @@ static int acquire_time_data(sd_bus *bus, UnitTimes **out) {
         _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         _cleanup_(unit_times_free_arrayp) UnitTimes *unit_times = NULL;
         BootTimes *boot_times = NULL;
-        size_t allocated = 0, c = 0;
+        size_t c = 0;
         UnitInfo u;
         int r;
 
@@ -361,7 +361,7 @@ static int acquire_time_data(sd_bus *bus, UnitTimes **out) {
         while ((r = bus_parse_unit_info(reply, &u)) > 0) {
                 UnitTimes *t;
 
-                if (!GREEDY_REALLOC(unit_times, allocated, c + 2))
+                if (!GREEDY_REALLOC(unit_times, c + 2))
                         return log_oom();
 
                 unit_times[c + 1].has_data = false;

src/basic/alloc-util.c

@@ -39,74 +39,67 @@ void* memdup_suffix0(const void *p, size_t l) {
         return ret;
 }
 
-void* greedy_realloc(void **p, size_t *allocated, size_t need, size_t size) {
+void* greedy_realloc(
+                void **p,
+                size_t need,
+                size_t size) {
+
         size_t a, newalloc;
         void *q;
 
         assert(p);
-        assert(allocated);
 
-        if (*allocated >= need)
+        /* We use malloc_usable_size() for determining the current allocated size. On all systems we care
+         * about this should be safe to rely on. Should there ever arise the need to avoid relying on this we
+         * can instead locally fall back to realloc() on every call, rounded up to the next exponent of 2 or
+         * so. */
+
+        if (*p && (size == 0 || (MALLOC_SIZEOF_SAFE(*p) / size >= need)))
                 return *p;
 
         if (_unlikely_(need > SIZE_MAX/2)) /* Overflow check */
                 return NULL;
-
         newalloc = need * 2;
+
         if (size_multiply_overflow(newalloc, size))
                 return NULL;
-
         a = newalloc * size;
+
         if (a < 64) /* Allocate at least 64 bytes */
                 a = 64;
 
         q = realloc(*p, a);
         if (!q)
                 return NULL;
 
-        if (size > 0) {
-                size_t bn;
-
-                /* Adjust for the 64 byte minimum */
-                newalloc = a / size;
-
-                bn = malloc_usable_size(q) / size;
-                if (bn > newalloc) {
-                        void *qq;
-
-                        /* The actual size allocated is larger than what we asked for. Let's call realloc() again to
-                         * take possession of the extra space. This should be cheap, since libc doesn't have to move
-                         * the memory for this. */
-
-                        qq = reallocarray(q, bn, size);
-                        if (_likely_(qq)) {
-                                *p = qq;
-                                *allocated = bn;
-                                return qq;
-                        }
-                }
-        }
-
-        *p = q;
-        *allocated = newalloc;
-        return q;
+        return *p = q;
 }
 
-void* greedy_realloc0(void **p, size_t *allocated, size_t need, size_t size) {
-        size_t prev;
+void* greedy_realloc0(
+                void **p,
+                size_t need,
+                size_t size) {
+
+        size_t before, after;
         uint8_t *q;
 
         assert(p);
-        assert(allocated);
 
-        prev = *allocated;
+        before = MALLOC_SIZEOF_SAFE(*p); /* malloc_usable_size() will return 0 on NULL input, as per docs */
 
-        q = greedy_realloc(p, allocated, need, size);
+        q = greedy_realloc(p, need, size);
         if (!q)
                 return NULL;
 
-        if (*allocated > prev)
-                memzero(q + prev * size, (*allocated - prev) * size);
+        after = MALLOC_SIZEOF_SAFE(q);
+
+        if (size == 0) /* avoid division by zero */
+                before = 0;
+        else
+                before = (before / size) * size; /* Round down */
+
+        if (after > before)
+                memzero(q + before, after - before);
 
         return q;
 }

src/basic/alloc-util.h

@@ -121,14 +121,14 @@ static inline void *memdup_suffix0_multiply(const void *p, size_t size, size_t n
         return memdup_suffix0(p, size * need);
 }
 
-void* greedy_realloc(void **p, size_t *allocated, size_t need, size_t size);
-void* greedy_realloc0(void **p, size_t *allocated, size_t need, size_t size);
+void* greedy_realloc(void **p, size_t need, size_t size);
+void* greedy_realloc0(void **p, size_t need, size_t size);
 
-#define GREEDY_REALLOC(array, allocated, need)                          \
-        greedy_realloc((void**) &(array), &(allocated), (need), sizeof((array)[0]))
+#define GREEDY_REALLOC(array, need)                                     \
+        greedy_realloc((void**) &(array), (need), sizeof((array)[0]))
 
-#define GREEDY_REALLOC0(array, allocated, need)                         \
-        greedy_realloc0((void**) &(array), &(allocated), (need), sizeof((array)[0]))
+#define GREEDY_REALLOC0(array, need)                                    \
+        greedy_realloc0((void**) &(array), (need), sizeof((array)[0]))
 
 #define alloca0(n)                                      \
         ({                                              \

src/basic/btrfs-util.c

@@ -1728,7 +1728,7 @@ int btrfs_qgroup_find_parents(int fd, uint64_t qgroupid, uint64_t **ret) {
         };
 
         _cleanup_free_ uint64_t *items = NULL;
-        size_t n_items = 0, n_allocated = 0;
+        size_t n_items = 0;
         int r;
 
         assert(fd >= 0);
@@ -1775,7 +1775,7 @@ int btrfs_qgroup_find_parents(int fd, uint64_t qgroupid, uint64_t **ret) {
                         if (sh->objectid != qgroupid)
                                 continue;
 
-                        if (!GREEDY_REALLOC(items, n_allocated, n_items+1))
+                        if (!GREEDY_REALLOC(items, n_items+1))
                                 return -ENOMEM;
 
                         items[n_items++] = sh->offset;

src/basic/cap-list.c

@@ -59,7 +59,7 @@ int capability_list_length(void) {
 
 int capability_set_to_string_alloc(uint64_t set, char **s) {
         _cleanup_free_ char *str = NULL;
-        size_t allocated = 0, n = 0;
+        size_t n = 0;
 
         assert(s);
 
@@ -77,14 +77,14 @@ int capability_set_to_string_alloc(uint64_t set, char **s) {
 
                         add = strlen(p);
 
-                        if (!GREEDY_REALLOC(str, allocated, n + add + 2))
+                        if (!GREEDY_REALLOC(str, n + add + 2))
                                 return -ENOMEM;
 
                         strcpy(mempcpy(str + n, p, add), " ");
                         n += add + 1;
                 }
 
-        if (!GREEDY_REALLOC(str, allocated, n + 1))
+        if (!GREEDY_REALLOC(str, n + 1))
                 return -ENOMEM;
 
         str[n > 0 ? n - 1 : 0] = '\0'; /* truncate the last space, if it's there */

src/basic/cgroup-util.c

@@ -1817,9 +1817,9 @@ int cg_get_keyed_attribute_full(
 
 int cg_mask_to_string(CGroupMask mask, char **ret) {
         _cleanup_free_ char *s = NULL;
-        size_t n = 0, allocated = 0;
         bool space = false;
         CGroupController c;
+        size_t n = 0;
 
         assert(ret);
 
@@ -1838,7 +1838,7 @@ int cg_mask_to_string(CGroupMask mask, char **ret) {
                 k = cgroup_controller_to_string(c);
                 l = strlen(k);
 
-                if (!GREEDY_REALLOC(s, allocated, n + space + l + 1))
+                if (!GREEDY_REALLOC(s, n + space + l + 1))
                         return -ENOMEM;
 
                 if (space)

src/basic/env-file.c

@@ -20,7 +20,7 @@ static int parse_env_file_internal(
                 void *userdata,
                 int *n_pushed) {
 
-        size_t key_alloc = 0, n_key = 0, value_alloc = 0, n_value = 0, last_value_whitespace = SIZE_MAX, last_key_whitespace = SIZE_MAX;
+        size_t n_key = 0, n_value = 0, last_value_whitespace = SIZE_MAX, last_key_whitespace = SIZE_MAX;
         _cleanup_free_ char *contents = NULL, *key = NULL, *value = NULL;
         unsigned line = 1;
         char *p;
@@ -58,7 +58,7 @@ static int parse_env_file_internal(
                                 state = KEY;
                                 last_key_whitespace = SIZE_MAX;
 
-                                if (!GREEDY_REALLOC(key, key_alloc, n_key+2))
+                                if (!GREEDY_REALLOC(key, n_key+2))
                                         return -ENOMEM;
 
                                 key[n_key++] = c;
@@ -79,7 +79,7 @@ static int parse_env_file_internal(
                                 else if (last_key_whitespace == SIZE_MAX)
                                          last_key_whitespace = n_key;
 
-                                if (!GREEDY_REALLOC(key, key_alloc, n_key+2))
+                                if (!GREEDY_REALLOC(key, n_key+2))
                                         return -ENOMEM;
 
                                 key[n_key++] = c;
@@ -106,7 +106,7 @@ static int parse_env_file_internal(
 
                                 n_key = 0;
                                 value = NULL;
-                                value_alloc = n_value = 0;
+                                n_value = 0;
 
                         } else if (c == '\'')
                                 state = SINGLE_QUOTE_VALUE;
@@ -117,7 +117,7 @@ static int parse_env_file_internal(
                         else if (!strchr(WHITESPACE, c)) {
                                 state = VALUE;
 
-                                if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
+                                if (!GREEDY_REALLOC(value, n_value+2))
                                         return  -ENOMEM;
 
                                 value[n_value++] = c;
@@ -149,7 +149,7 @@ static int parse_env_file_internal(
 
                                 n_key = 0;
                                 value = NULL;
-                                value_alloc = n_value = 0;
+                                n_value = 0;
 
                         } else if (c == '\\') {
                                 state = VALUE_ESCAPE;
@@ -160,7 +160,7 @@ static int parse_env_file_internal(
                                 else if (last_value_whitespace == SIZE_MAX)
                                         last_value_whitespace = n_value;
 
-                                if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
+                                if (!GREEDY_REALLOC(value, n_value+2))
                                         return -ENOMEM;
 
                                 value[n_value++] = c;
@@ -173,7 +173,7 @@ static int parse_env_file_internal(
 
                         if (!strchr(NEWLINE, c)) {
                                 /* Escaped newlines we eat up entirely */
-                                if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
+                                if (!GREEDY_REALLOC(value, n_value+2))
                                         return -ENOMEM;
 
                                 value[n_value++] = c;
@@ -184,7 +184,7 @@ static int parse_env_file_internal(
                         if (c == '\'')
                                 state = PRE_VALUE;
                         else {
-                                if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
+                                if (!GREEDY_REALLOC(value, n_value+2))
                                         return -ENOMEM;
 
                                 value[n_value++] = c;
@@ -198,7 +198,7 @@ static int parse_env_file_internal(
                         else if (c == '\\')
                                 state = DOUBLE_QUOTE_VALUE_ESCAPE;
                         else {
-                                if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
+                                if (!GREEDY_REALLOC(value, n_value+2))
                                         return -ENOMEM;
 
                                 value[n_value++] = c;
@@ -211,13 +211,13 @@ static int parse_env_file_internal(
 
                         if (strchr(SHELL_NEED_ESCAPE, c)) {
                                 /* If this is a char that needs escaping, just unescape it. */
-                                if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
+                                if (!GREEDY_REALLOC(value, n_value+2))
                                         return -ENOMEM;
                                 value[n_value++] = c;
                         } else if (c != '\n') {
                                 /* If other char than what needs escaping, keep the "\" in place, like the
                                  * real shell does. */
-                                if (!GREEDY_REALLOC(value, value_alloc, n_value+3))
+                                if (!GREEDY_REALLOC(value, n_value+3))
                                         return -ENOMEM;
                                 value[n_value++] = '\\';
                                 value[n_value++] = c;

src/basic/extract-word.c

@@ -19,7 +19,7 @@
 
 int extract_first_word(const char **p, char **ret, const char *separators, ExtractFlags flags) {
         _cleanup_free_ char *s = NULL;
-        size_t allocated = 0, sz = 0;
+        size_t sz = 0;
         char quote = 0;                 /* 0 or ' or " */
         bool backslash = false;         /* whether we've just seen a backslash */
         char c;
@@ -42,7 +42,7 @@ int extract_first_word(const char **p, char **ret, const char *separators, Extra
          * the pointer *p at the first invalid character. */
 
         if (flags & EXTRACT_DONT_COALESCE_SEPARATORS)
-                if (!GREEDY_REALLOC(s, allocated, sz+1))
+                if (!GREEDY_REALLOC(s, sz+1))
                         return -ENOMEM;
 
         for (;; (*p)++, c = **p) {
@@ -57,15 +57,15 @@ int extract_first_word(const char **p, char **ret, const char *separators, Extra
                         /* We found a non-blank character, so we will always
                          * want to return a string (even if it is empty),
                          * allocate it here. */
-                        if (!GREEDY_REALLOC(s, allocated, sz+1))
+                        if (!GREEDY_REALLOC(s, sz+1))
                                 return -ENOMEM;
                         break;
                 }
         }
 
         for (;; (*p)++, c = **p) {
                 if (backslash) {
-                        if (!GREEDY_REALLOC(s, allocated, sz+7))
+                        if (!GREEDY_REALLOC(s, sz+7))
                                 return -ENOMEM;
 
                         if (c == 0) {
@@ -128,7 +128,7 @@ int extract_first_word(const char **p, char **ret, const char *separators, Extra
                                         backslash = true;
                                         break;
                                 } else {
-                                        if (!GREEDY_REALLOC(s, allocated, sz+2))
+                                        if (!GREEDY_REALLOC(s, sz+2))
                                                 return -ENOMEM;
 
                                         s[sz++] = c;
@@ -160,7 +160,7 @@ int extract_first_word(const char **p, char **ret, const char *separators, Extra
                                         goto finish;
 
                                 } else {
-                                        if (!GREEDY_REALLOC(s, allocated, sz+2))
+                                        if (!GREEDY_REALLOC(s, sz+2))
                                                 return -ENOMEM;
 
                                         s[sz++] = c;