Skip to content

Commit 319a4f4

Browse files
poetteringpoettering
committed
alloc-util: simplify GREEDY_REALLOC() logic by relying on malloc_usable_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.)
1 parent 9948050 commit 319a4f4

File tree

127 files changed

+657
-687
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

127 files changed

+657
-687
lines changed

‎src/analyze/analyze-security.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2116,7 +2116,7 @@ int analyze_security(sd_bus *bus, char **units, AnalyzeSecurityFlags flags) {
21162116
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
21172117
_cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
21182118
_cleanup_strv_free_ char **list = NULL;
2119-
size_t allocated = 0, n = 0;
2119+
size_t n = 0;
21202120
char **i;
21212121

21222122
r = sd_bus_call_method(
@@ -2148,7 +2148,7 @@ int analyze_security(sd_bus *bus, char **units, AnalyzeSecurityFlags flags) {
21482148
if (!endswith(info.id, ".service"))
21492149
continue;
21502150

2151-
if (!GREEDY_REALLOC(list, allocated, n + 2))
2151+
if (!GREEDY_REALLOC(list, n + 2))
21522152
return log_oom();
21532153

21542154
copy = strdup(info.id);

‎src/analyze/analyze.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ static int acquire_time_data(sd_bus *bus, UnitTimes **out) {
342342
_cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
343343
_cleanup_(unit_times_free_arrayp) UnitTimes *unit_times = NULL;
344344
BootTimes *boot_times = NULL;
345-
size_t allocated = 0, c = 0;
345+
size_t c = 0;
346346
UnitInfo u;
347347
int r;
348348

@@ -361,7 +361,7 @@ static int acquire_time_data(sd_bus *bus, UnitTimes **out) {
361361
while ((r = bus_parse_unit_info(reply, &u)) > 0) {
362362
UnitTimes *t;
363363

364-
if (!GREEDY_REALLOC(unit_times, allocated, c + 2))
364+
if (!GREEDY_REALLOC(unit_times, c + 2))
365365
return log_oom();
366366

367367
unit_times[c + 1].has_data = false;

‎src/basic/alloc-util.c‎

Lines changed: 31 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -39,74 +39,67 @@ void* memdup_suffix0(const void *p, size_t l) {
3939
return ret;
4040
}
4141

42-
void* greedy_realloc(void **p, size_t *allocated, size_t need, size_t size) {
42+
void* greedy_realloc(
43+
void **p,
44+
size_t need,
45+
size_t size) {
46+
4347
size_t a, newalloc;
4448
void *q;
4549

4650
assert(p);
47-
assert(allocated);
4851

49-
if (*allocated >= need)
52+
/* We use malloc_usable_size() for determining the current allocated size. On all systems we care
53+
* about this should be safe to rely on. Should there ever arise the need to avoid relying on this we
54+
* can instead locally fall back to realloc() on every call, rounded up to the next exponent of 2 or
55+
* so. */
56+
57+
if (*p && (size == 0 || (MALLOC_SIZEOF_SAFE(*p) / size >= need)))
5058
return *p;
5159

5260
if (_unlikely_(need > SIZE_MAX/2)) /* Overflow check */
5361
return NULL;
54-
5562
newalloc = need * 2;
63+
5664
if (size_multiply_overflow(newalloc, size))
5765
return NULL;
58-
5966
a = newalloc * size;
67+
6068
if (a < 64) /* Allocate at least 64 bytes */
6169
a = 64;
6270

6371
q = realloc(*p, a);
6472
if (!q)
6573
return NULL;
6674

67-
if (size > 0) {
68-
size_t bn;
69-
70-
/* Adjust for the 64 byte minimum */
71-
newalloc = a / size;
72-
73-
bn = malloc_usable_size(q) / size;
74-
if (bn > newalloc) {
75-
void *qq;
76-
77-
/* The actual size allocated is larger than what we asked for. Let's call realloc() again to
78-
* take possession of the extra space. This should be cheap, since libc doesn't have to move
79-
* the memory for this. */
80-
81-
qq = reallocarray(q, bn, size);
82-
if (_likely_(qq)) {
83-
*p = qq;
84-
*allocated = bn;
85-
return qq;
86-
}
87-
}
88-
}
89-
90-
*p = q;
91-
*allocated = newalloc;
92-
return q;
75+
return *p = q;
9376
}
9477

95-
void* greedy_realloc0(void **p, size_t *allocated, size_t need, size_t size) {
96-
size_t prev;
78+
void* greedy_realloc0(
79+
void **p,
80+
size_t need,
81+
size_t size) {
82+
83+
size_t before, after;
9784
uint8_t *q;
9885

9986
assert(p);
100-
assert(allocated);
10187

102-
prev = *allocated;
88+
before = MALLOC_SIZEOF_SAFE(*p); /* malloc_usable_size() will return 0 on NULL input, as per docs */
10389

104-
q = greedy_realloc(p, allocated, need, size);
90+
q = greedy_realloc(p, need, size);
10591
if (!q)
10692
return NULL;
10793

108-
if (*allocated > prev)
109-
memzero(q + prev * size, (*allocated - prev) * size);
94+
after = MALLOC_SIZEOF_SAFE(q);
95+
96+
if (size == 0) /* avoid division by zero */
97+
before = 0;
98+
else
99+
before = (before / size) * size; /* Round down */
100+
101+
if (after > before)
102+
memzero(q + before, after - before);
110103

111104
return q;
112105
}

‎src/basic/alloc-util.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,14 @@ static inline void *memdup_suffix0_multiply(const void *p, size_t size, size_t n
121121
return memdup_suffix0(p, size * need);
122122
}
123123

124-
void* greedy_realloc(void **p, size_t *allocated, size_t need, size_t size);
125-
void* greedy_realloc0(void **p, size_t *allocated, size_t need, size_t size);
124+
void* greedy_realloc(void **p, size_t need, size_t size);
125+
void* greedy_realloc0(void **p, size_t need, size_t size);
126126

127-
#define GREEDY_REALLOC(array, allocated, need) \
128-
greedy_realloc((void**) &(array), &(allocated), (need), sizeof((array)[0]))
127+
#define GREEDY_REALLOC(array, need) \
128+
greedy_realloc((void**) &(array), (need), sizeof((array)[0]))
129129

130-
#define GREEDY_REALLOC0(array, allocated, need) \
131-
greedy_realloc0((void**) &(array), &(allocated), (need), sizeof((array)[0]))
130+
#define GREEDY_REALLOC0(array, need) \
131+
greedy_realloc0((void**) &(array), (need), sizeof((array)[0]))
132132

133133
#define alloca0(n) \
134134
({ \

‎src/basic/btrfs-util.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1728,7 +1728,7 @@ int btrfs_qgroup_find_parents(int fd, uint64_t qgroupid, uint64_t **ret) {
17281728
};
17291729

17301730
_cleanup_free_ uint64_t *items = NULL;
1731-
size_t n_items = 0, n_allocated = 0;
1731+
size_t n_items = 0;
17321732
int r;
17331733

17341734
assert(fd >= 0);
@@ -1775,7 +1775,7 @@ int btrfs_qgroup_find_parents(int fd, uint64_t qgroupid, uint64_t **ret) {
17751775
if (sh->objectid != qgroupid)
17761776
continue;
17771777

1778-
if (!GREEDY_REALLOC(items, n_allocated, n_items+1))
1778+
if (!GREEDY_REALLOC(items, n_items+1))
17791779
return -ENOMEM;
17801780

17811781
items[n_items++] = sh->offset;

‎src/basic/cap-list.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ int capability_list_length(void) {
5959

6060
int capability_set_to_string_alloc(uint64_t set, char **s) {
6161
_cleanup_free_ char *str = NULL;
62-
size_t allocated = 0, n = 0;
62+
size_t n = 0;
6363

6464
assert(s);
6565

@@ -77,14 +77,14 @@ int capability_set_to_string_alloc(uint64_t set, char **s) {
7777

7878
add = strlen(p);
7979

80-
if (!GREEDY_REALLOC(str, allocated, n + add + 2))
80+
if (!GREEDY_REALLOC(str, n + add + 2))
8181
return -ENOMEM;
8282

8383
strcpy(mempcpy(str + n, p, add), " ");
8484
n += add + 1;
8585
}
8686

87-
if (!GREEDY_REALLOC(str, allocated, n + 1))
87+
if (!GREEDY_REALLOC(str, n + 1))
8888
return -ENOMEM;
8989

9090
str[n > 0 ? n - 1 : 0] = '\0'; /* truncate the last space, if it's there */

‎src/basic/cgroup-util.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1817,9 +1817,9 @@ int cg_get_keyed_attribute_full(
18171817

18181818
int cg_mask_to_string(CGroupMask mask, char **ret) {
18191819
_cleanup_free_ char *s = NULL;
1820-
size_t n = 0, allocated = 0;
18211820
bool space = false;
18221821
CGroupController c;
1822+
size_t n = 0;
18231823

18241824
assert(ret);
18251825

@@ -1838,7 +1838,7 @@ int cg_mask_to_string(CGroupMask mask, char **ret) {
18381838
k = cgroup_controller_to_string(c);
18391839
l = strlen(k);
18401840

1841-
if (!GREEDY_REALLOC(s, allocated, n + space + l + 1))
1841+
if (!GREEDY_REALLOC(s, n + space + l + 1))
18421842
return -ENOMEM;
18431843

18441844
if (space)

‎src/basic/env-file.c‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ static int parse_env_file_internal(
2020
void *userdata,
2121
int *n_pushed) {
2222

23-
size_t key_alloc = 0, n_key = 0, value_alloc = 0, n_value = 0, last_value_whitespace = SIZE_MAX, last_key_whitespace = SIZE_MAX;
23+
size_t n_key = 0, n_value = 0, last_value_whitespace = SIZE_MAX, last_key_whitespace = SIZE_MAX;
2424
_cleanup_free_ char *contents = NULL, *key = NULL, *value = NULL;
2525
unsigned line = 1;
2626
char *p;
@@ -58,7 +58,7 @@ static int parse_env_file_internal(
5858
state = KEY;
5959
last_key_whitespace = SIZE_MAX;
6060

61-
if (!GREEDY_REALLOC(key, key_alloc, n_key+2))
61+
if (!GREEDY_REALLOC(key, n_key+2))
6262
return -ENOMEM;
6363

6464
key[n_key++] = c;
@@ -79,7 +79,7 @@ static int parse_env_file_internal(
7979
else if (last_key_whitespace == SIZE_MAX)
8080
last_key_whitespace = n_key;
8181

82-
if (!GREEDY_REALLOC(key, key_alloc, n_key+2))
82+
if (!GREEDY_REALLOC(key, n_key+2))
8383
return -ENOMEM;
8484

8585
key[n_key++] = c;
@@ -106,7 +106,7 @@ static int parse_env_file_internal(
106106

107107
n_key = 0;
108108
value = NULL;
109-
value_alloc = n_value = 0;
109+
n_value = 0;
110110

111111
} else if (c == '\'')
112112
state = SINGLE_QUOTE_VALUE;
@@ -117,7 +117,7 @@ static int parse_env_file_internal(
117117
else if (!strchr(WHITESPACE, c)) {
118118
state = VALUE;
119119

120-
if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
120+
if (!GREEDY_REALLOC(value, n_value+2))
121121
return -ENOMEM;
122122

123123
value[n_value++] = c;
@@ -149,7 +149,7 @@ static int parse_env_file_internal(
149149

150150
n_key = 0;
151151
value = NULL;
152-
value_alloc = n_value = 0;
152+
n_value = 0;
153153

154154
} else if (c == '\\') {
155155
state = VALUE_ESCAPE;
@@ -160,7 +160,7 @@ static int parse_env_file_internal(
160160
else if (last_value_whitespace == SIZE_MAX)
161161
last_value_whitespace = n_value;
162162

163-
if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
163+
if (!GREEDY_REALLOC(value, n_value+2))
164164
return -ENOMEM;
165165

166166
value[n_value++] = c;
@@ -173,7 +173,7 @@ static int parse_env_file_internal(
173173

174174
if (!strchr(NEWLINE, c)) {
175175
/* Escaped newlines we eat up entirely */
176-
if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
176+
if (!GREEDY_REALLOC(value, n_value+2))
177177
return -ENOMEM;
178178

179179
value[n_value++] = c;
@@ -184,7 +184,7 @@ static int parse_env_file_internal(
184184
if (c == '\'')
185185
state = PRE_VALUE;
186186
else {
187-
if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
187+
if (!GREEDY_REALLOC(value, n_value+2))
188188
return -ENOMEM;
189189

190190
value[n_value++] = c;
@@ -198,7 +198,7 @@ static int parse_env_file_internal(
198198
else if (c == '\\')
199199
state = DOUBLE_QUOTE_VALUE_ESCAPE;
200200
else {
201-
if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
201+
if (!GREEDY_REALLOC(value, n_value+2))
202202
return -ENOMEM;
203203

204204
value[n_value++] = c;
@@ -211,13 +211,13 @@ static int parse_env_file_internal(
211211

212212
if (strchr(SHELL_NEED_ESCAPE, c)) {
213213
/* If this is a char that needs escaping, just unescape it. */
214-
if (!GREEDY_REALLOC(value, value_alloc, n_value+2))
214+
if (!GREEDY_REALLOC(value, n_value+2))
215215
return -ENOMEM;
216216
value[n_value++] = c;
217217
} else if (c != '\n') {
218218
/* If other char than what needs escaping, keep the "\" in place, like the
219219
* real shell does. */
220-
if (!GREEDY_REALLOC(value, value_alloc, n_value+3))
220+
if (!GREEDY_REALLOC(value, n_value+3))
221221
return -ENOMEM;
222222
value[n_value++] = '\\';
223223
value[n_value++] = c;

‎src/basic/extract-word.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919

2020
int extract_first_word(const char **p, char **ret, const char *separators, ExtractFlags flags) {
2121
_cleanup_free_ char *s = NULL;
22-
size_t allocated = 0, sz = 0;
22+
size_t sz = 0;
2323
char quote = 0; /* 0 or ' or " */
2424
bool backslash = false; /* whether we've just seen a backslash */
2525
char c;
@@ -42,7 +42,7 @@ int extract_first_word(const char **p, char **ret, const char *separators, Extra
4242
* the pointer *p at the first invalid character. */
4343

4444
if (flags & EXTRACT_DONT_COALESCE_SEPARATORS)
45-
if (!GREEDY_REALLOC(s, allocated, sz+1))
45+
if (!GREEDY_REALLOC(s, sz+1))
4646
return -ENOMEM;
4747

4848
for (;; (*p)++, c = **p) {
@@ -57,15 +57,15 @@ int extract_first_word(const char **p, char **ret, const char *separators, Extra
5757
/* We found a non-blank character, so we will always
5858
* want to return a string (even if it is empty),
5959
* allocate it here. */
60-
if (!GREEDY_REALLOC(s, allocated, sz+1))
60+
if (!GREEDY_REALLOC(s, sz+1))
6161
return -ENOMEM;
6262
break;
6363
}
6464
}
6565

6666
for (;; (*p)++, c = **p) {
6767
if (backslash) {
68-
if (!GREEDY_REALLOC(s, allocated, sz+7))
68+
if (!GREEDY_REALLOC(s, sz+7))
6969
return -ENOMEM;
7070

7171
if (c == 0) {
@@ -128,7 +128,7 @@ int extract_first_word(const char **p, char **ret, const char *separators, Extra
128128
backslash = true;
129129
break;
130130
} else {
131-
if (!GREEDY_REALLOC(s, allocated, sz+2))
131+
if (!GREEDY_REALLOC(s, sz+2))
132132
return -ENOMEM;
133133

134134
s[sz++] = c;
@@ -160,7 +160,7 @@ int extract_first_word(const char **p, char **ret, const char *separators, Extra
160160
goto finish;
161161

162162
} else {
163-
if (!GREEDY_REALLOC(s, allocated, sz+2))
163+
if (!GREEDY_REALLOC(s, sz+2))
164164
return -ENOMEM;
165165

166166
s[sz++] = c;

0 commit comments

Comments
 (0)