SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
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.)
This commit is contained in:
Lennart Poettering
2021-05-19 16:42:37 +02:00
parent 99480504d4
commit 319a4f4bc4
127 changed files with 658 additions and 688 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/alloc-util.c
+31 -38
View File
@@ -39,24 +39,32 @@ 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;
@@ -64,49 +72,34 @@ void* greedy_realloc(void **p, size_t *allocated, size_t need, size_t size) {
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
+6 -6
View File
@@ -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
+2 -2
View File
@@ -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
+3 -3
View File
@@ -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
+2 -2
View File
@@ -1817,9 +1817,9 @@ done:
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
+12 -12
View File
@@ -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
+6 -6
View File
@@ -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,7 +57,7 @@ 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;
}
@@ -65,7 +65,7 @@ int extract_first_word(const char **p, char **ret, const char *separators, Extra
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;
src/basic/fileio.c
+3 -3
View File
@@ -1212,8 +1212,8 @@ static EndOfLineMarker categorize_eol(char c, ReadLineFlags flags) {
DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(FILE*, funlockfile, NULL);
int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
size_t n = 0, allocated = 0, count = 0;
_cleanup_free_ char *buffer = NULL;
size_t n = 0, count = 0;
int r;
assert(f);
@@ -1243,7 +1243,7 @@ int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
* If a line shall be skipped ret may be initialized as NULL. */
if (ret) {
if (!GREEDY_REALLOC(buffer, allocated, 1))
if (!GREEDY_REALLOC(buffer, 1))
return -ENOMEM;
}
@@ -1315,7 +1315,7 @@ int read_line_full(FILE *f, size_t limit, ReadLineFlags flags, char **ret) {
}
if (ret) {
if (!GREEDY_REALLOC(buffer, allocated, n + 2))
if (!GREEDY_REALLOC(buffer, n + 2))
return -ENOMEM;
buffer[n] = c;
src/basic/fs-util.c
+3 -3
View File
@@ -549,10 +549,10 @@ int mkfifoat_atomic(int dirfd, const char *path, mode_t mode) {
}
int get_files_in_directory(const char *path, char ***list) {
_cleanup_strv_free_ char **l = NULL;
_cleanup_closedir_ DIR *d = NULL;
struct dirent *de;
size_t bufsize = 0, n = 0;
_cleanup_strv_free_ char **l = NULL;
size_t n = 0;
assert(path);
@@ -572,7 +572,7 @@ int get_files_in_directory(const char *path, char ***list) {
if (list) {
/* one extra slot is needed for the terminating NULL */
if (!GREEDY_REALLOC(l, bufsize, n + 2))
if (!GREEDY_REALLOC(l, n + 2))
return -ENOMEM;
l[n] = strdup(de->d_name);
src/basic/hashmap.c
+6 -6
View File
@@ -230,7 +230,7 @@ struct Set {
typedef struct CacheMem {
const void **ptr;
size_t n_populated, n_allocated;
size_t n_populated;
bool active:1;
} CacheMem;
@@ -1897,10 +1897,10 @@ int set_put_strsplit(Set *s, const char *v, const char *separators, ExtractFlags
}
/* expand the cachemem if needed, return true if newly (re)activated. */
static int cachemem_maintain(CacheMem *mem, unsigned size) {
static int cachemem_maintain(CacheMem *mem, size_t size) {
assert(mem);
if (!GREEDY_REALLOC(mem->ptr, mem->n_allocated, size)) {
if (!GREEDY_REALLOC(mem->ptr, size)) {
if (size > 0)
return -ENOMEM;
}
@@ -1915,7 +1915,7 @@ static int cachemem_maintain(CacheMem *mem, unsigned size) {
int iterated_cache_get(IteratedCache *cache, const void ***res_keys, const void ***res_values, unsigned *res_n_entries) {
bool sync_keys = false, sync_values = false;
unsigned size;
size_t size;
int r;
assert(cache);
@@ -1988,8 +1988,8 @@ IteratedCache* iterated_cache_free(IteratedCache *cache) {
}
int set_strjoin(Set *s, const char *separator, bool wrap_with_separator, char **ret) {
size_t separator_len, allocated = 0, len = 0;
_cleanup_free_ char *str = NULL;
size_t separator_len, len = 0;
const char *value;
bool first;
@@ -2013,7 +2013,7 @@ int set_strjoin(Set *s, const char *separator, bool wrap_with_separator, char **
if (l == 0)
continue;
if (!GREEDY_REALLOC(str, allocated, len + l + (first ? 0 : separator_len) + (wrap_with_separator ? separator_len : 0) + 1))
if (!GREEDY_REALLOC(str, len + l + (first ? 0 : separator_len) + (wrap_with_separator ? separator_len : 0) + 1))
return -ENOMEM;
if (separator_len > 0 && !first) {
src/basic/io-util.c
+1 -2
View File
@@ -288,7 +288,6 @@ void iovw_free_contents(struct iovec_wrapper *iovw, bool free_vectors) {
iovw->iovec = mfree(iovw->iovec);
iovw->count = 0;
iovw->size_bytes = 0;
}
struct iovec_wrapper *iovw_free_free(struct iovec_wrapper *iovw) {
@@ -307,7 +306,7 @@ int iovw_put(struct iovec_wrapper *iovw, void *data, size_t len) {
if (iovw->count >= IOV_MAX)
return -E2BIG;
if (!GREEDY_REALLOC(iovw->iovec, iovw->size_bytes, iovw->count + 1))
if (!GREEDY_REALLOC(iovw->iovec, iovw->count + 1))
return -ENOMEM;
iovw->iovec[iovw->count++] = IOVEC_MAKE(data, len);
src/basic/io-util.h
-1
View File
@@ -85,7 +85,6 @@ char* set_iovec_string_field_free(struct iovec *iovec, size_t *n_iovec, const ch
struct iovec_wrapper {
struct iovec *iovec;
size_t count;
size_t size_bytes;
};
struct iovec_wrapper *iovw_new(void);
src/basic/process-util.c
+2 -2
View File
@@ -601,7 +601,7 @@ int get_process_root(pid_t pid, char **root) {
int get_process_environ(pid_t pid, char **env) {
_cleanup_fclose_ FILE *f = NULL;
_cleanup_free_ char *outcome = NULL;
size_t allocated = 0, sz = 0;
size_t sz = 0;
const char *p;
int r;
@@ -622,7 +622,7 @@ int get_process_environ(pid_t pid, char **env) {
if (sz >= ENVIRONMENT_BLOCK_MAX)
return -ENOBUFS;
if (!GREEDY_REALLOC(outcome, allocated, sz + 5))
if (!GREEDY_REALLOC(outcome, sz + 5))
return -ENOMEM;
r = safe_fgetc(f, &c);
src/basic/string-util.c
+3 -3
View File
@@ -522,7 +522,7 @@ char* strshorten(char *s, size_t l) {
}
char *strreplace(const char *text, const char *old_string, const char *new_string) {
size_t l, old_len, new_len, allocated = 0;
size_t l, old_len, new_len;
char *t, *ret = NULL;
const char *f;
@@ -536,7 +536,7 @@ char *strreplace(const char *text, const char *old_string, const char *new_strin
new_len = strlen(new_string);
l = strlen(text);
if (!GREEDY_REALLOC(ret, allocated, l+1))
if (!GREEDY_REALLOC(ret, l+1))
return NULL;
f = text;
@@ -552,7 +552,7 @@ char *strreplace(const char *text, const char *old_string, const char *new_strin
d = t - ret;
nl = l - old_len + new_len;
if (!GREEDY_REALLOC(ret, allocated, nl + 1))
if (!GREEDY_REALLOC(ret, nl + 1))
return mfree(ret);
l = nl;
src/basic/strv.c
+6 -6
View File
@@ -266,7 +266,7 @@ int strv_split_newlines_full(char ***ret, const char *s, ExtractFlags flags) {
int strv_split_full(char ***t, const char *s, const char *separators, ExtractFlags flags) {
_cleanup_strv_free_ char **l = NULL;
size_t n = 0, allocated = 0;
size_t n = 0;
int r;
assert(t);
@@ -281,7 +281,7 @@ int strv_split_full(char ***t, const char *s, const char *separators, ExtractFla
if (r == 0)
break;
if (!GREEDY_REALLOC(l, allocated, n + 2))
if (!GREEDY_REALLOC(l, n + 2))
return -ENOMEM;
l[n++] = TAKE_PTR(word);
@@ -302,7 +302,7 @@ int strv_split_full(char ***t, const char *s, const char *separators, ExtractFla
int strv_split_colon_pairs(char ***t, const char *s) {
_cleanup_strv_free_ char **l = NULL;
size_t n = 0, allocated = 0;
size_t n = 0;
int r;
assert(t);
@@ -332,7 +332,7 @@ int strv_split_colon_pairs(char ***t, const char *s) {
if (!second_or_empty)
return -ENOMEM;
if (!GREEDY_REALLOC(l, allocated, n + 3))
if (!GREEDY_REALLOC(l, n + 3))
return -ENOMEM;
l[n++] = TAKE_PTR(first);
@@ -708,9 +708,9 @@ int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) {
* is provided separately.
*/
size_t n_allocated = 0, n = 0;
_cleanup_free_ char *m = NULL;
char * const *i;
size_t n = 0;
assert(ret);
assert(ret_size);
@@ -720,7 +720,7 @@ int strv_make_nulstr(char * const *l, char **ret, size_t *ret_size) {
z = strlen(*i);
if (!GREEDY_REALLOC(m, n_allocated, n + z + 2))
if (!GREEDY_REALLOC(m, n + z + 2))
return -ENOMEM;
memcpy(m + n, *i, z + 1);
src/basic/time-util.c
+2 -3
View File
@@ -1247,7 +1247,7 @@ int parse_nsec(const char *t, nsec_t *nsec) {
int get_timezones(char ***ret) {
_cleanup_fclose_ FILE *f = NULL;
_cleanup_strv_free_ char **zones = NULL;
size_t n_zones = 0, n_allocated = 0;
size_t n_zones = 0;
int r;
assert(ret);
@@ -1256,7 +1256,6 @@ int get_timezones(char ***ret) {
if (!zones)
return -ENOMEM;
n_allocated = 2;
n_zones = 1;
f = fopen("/usr/share/zoneinfo/zone1970.tab", "re");
@@ -1294,7 +1293,7 @@ int get_timezones(char ***ret) {
if (!w)
return -ENOMEM;
if (!GREEDY_REALLOC(zones, n_allocated, n_zones + 2))
if (!GREEDY_REALLOC(zones, n_zones + 2))
return -ENOMEM;
zones[n_zones++] = TAKE_PTR(w);