Skip to content

Commit f1453c5

Browse files
author
Edward Thomson
committed
Make our overflow check look more like gcc/clang's
Make our overflow checking look more like gcc and clang's, so that we can substitute it out with the compiler instrinsics on platforms that support it. This means dropping the ability to pass `NULL` as an out parameter. As a result, the macros also get updated to reflect this as well.
1 parent 650e45f commit f1453c5

36 files changed

+352
-347
lines changed

‎src/array.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,9 @@ GIT_INLINE(void *) git_array_grow(void *_a, size_t item_size)
5151
if (a->size < 8) {
5252
new_size = 8;
5353
} else {
54-
if (GIT_ALLOC_OVERFLOW_MULTIPLY(a->size, 3 / 2))
54+
if (GIT_MULTIPLY_SIZET_OVERFLOW(&new_size, a->size, 3))
5555
goto on_oom;
56-
57-
new_size = a->size * 3 / 2;
56+
new_size /= 2;
5857
}
5958

6059
if ((new_array = git__reallocarray(a->ptr, new_size, item_size)) == NULL)

‎src/blame_git.c‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,14 @@ static void origin_decref(git_blame__origin *o)
3636
static int make_origin(git_blame__origin **out, git_commit *commit, const char *path)
3737
{
3838
git_blame__origin *o;
39-
size_t path_len = strlen(path);
39+
size_t path_len = strlen(path), alloc_len;
4040
int error = 0;
4141

42-
GITERR_CHECK_ALLOC_ADD(sizeof(*o), path_len);
43-
GITERR_CHECK_ALLOC_ADD(sizeof(*o) + path_len, 1);
44-
45-
o = git__calloc(1, sizeof(*o) + path_len + 1);
42+
GITERR_CHECK_ALLOC_ADD(&alloc_len, sizeof(*o), path_len);
43+
GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 1);
44+
o = git__calloc(1, alloc_len);
4645
GITERR_CHECK_ALLOC(o);
46+
4747
o->commit = commit;
4848
o->refcnt = 1;
4949
strcpy(o->path, path);

‎src/buf_text.c‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ int git_buf_text_puts_escaped(
1313
const char *esc_with)
1414
{
1515
const char *scan;
16-
size_t total = 0, esc_len = strlen(esc_with), count;
16+
size_t total = 0, esc_len = strlen(esc_with), count, alloclen;
1717

1818
if (!string)
1919
return 0;
@@ -29,8 +29,8 @@ int git_buf_text_puts_escaped(
2929
scan += count;
3030
}
3131

32-
GITERR_CHECK_ALLOC_ADD(total, 1);
33-
if (git_buf_grow_by(buf, total + 1) < 0)
32+
GITERR_CHECK_ALLOC_ADD(&alloclen, total, 1);
33+
if (git_buf_grow_by(buf, alloclen) < 0)
3434
return -1;
3535

3636
for (scan = string; *scan; ) {
@@ -66,6 +66,7 @@ int git_buf_text_crlf_to_lf(git_buf *tgt, const git_buf *src)
6666
const char *scan = src->ptr;
6767
const char *scan_end = src->ptr + src->size;
6868
const char *next = memchr(scan, '\r', src->size);
69+
size_t new_size;
6970
char *out;
7071

7172
assert(tgt != src);
@@ -74,8 +75,8 @@ int git_buf_text_crlf_to_lf(git_buf *tgt, const git_buf *src)
7475
return git_buf_set(tgt, src->ptr, src->size);
7576

7677
/* reduce reallocs while in the loop */
77-
GITERR_CHECK_ALLOC_ADD(src->size, 1);
78-
if (git_buf_grow(tgt, src->size + 1) < 0)
78+
GITERR_CHECK_ALLOC_ADD(&new_size, src->size, 1);
79+
if (git_buf_grow(tgt, new_size) < 0)
7980
return -1;
8081

8182
out = tgt->ptr;
@@ -113,16 +114,17 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src)
113114
const char *end = start + src->size;
114115
const char *scan = start;
115116
const char *next = memchr(scan, '\n', src->size);
117+
size_t alloclen;
116118

117119
assert(tgt != src);
118120

119121
if (!next)
120122
return git_buf_set(tgt, src->ptr, src->size);
121123

122124
/* attempt to reduce reallocs while in the loop */
123-
GITERR_CHECK_ALLOC_ADD(src->size, src->size >> 4);
124-
GITERR_CHECK_ALLOC_ADD(src->size + (src->size >> 4), 1);
125-
if (git_buf_grow(tgt, src->size + (src->size >> 4) + 1) < 0)
125+
GITERR_CHECK_ALLOC_ADD(&alloclen, src->size, src->size >> 4);
126+
GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1);
127+
if (git_buf_grow(tgt, alloclen) < 0)
126128
return -1;
127129
tgt->size = 0;
128130

@@ -135,8 +137,8 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src)
135137
return GIT_PASSTHROUGH;
136138
}
137139

138-
GITERR_CHECK_ALLOC_ADD(copylen, 3);
139-
if (git_buf_grow_by(tgt, copylen + 3) < 0)
140+
GITERR_CHECK_ALLOC_ADD(&alloclen, copylen, 3);
141+
if (git_buf_grow_by(tgt, alloclen) < 0)
140142
return -1;
141143

142144
if (next > scan) {

‎src/buffer.c‎

Lines changed: 76 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -103,13 +103,14 @@ int git_buf_grow(git_buf *buffer, size_t target_size)
103103

104104
int git_buf_grow_by(git_buf *buffer, size_t additional_size)
105105
{
106-
if (GIT_ALLOC_OVERFLOW_ADD(buffer->size, additional_size)) {
106+
size_t newsize;
107+
108+
if (GIT_ADD_SIZET_OVERFLOW(&newsize, buffer->size, additional_size)) {
107109
buffer->ptr = git_buf__oom;
108110
return -1;
109111
}
110112

111-
return git_buf_try_grow(
112-
buffer, buffer->size + additional_size, true, true);
113+
return git_buf_try_grow(buffer, newsize, true, true);
113114
}
114115

115116
void git_buf_free(git_buf *buf)
@@ -146,12 +147,14 @@ void git_buf_clear(git_buf *buf)
146147

147148
int git_buf_set(git_buf *buf, const void *data, size_t len)
148149
{
150+
size_t alloclen;
151+
149152
if (len == 0 || data == NULL) {
150153
git_buf_clear(buf);
151154
} else {
152155
if (data != buf->ptr) {
153-
GITERR_CHECK_ALLOC_ADD(len, 1);
154-
ENSURE_SIZE(buf, len + 1);
156+
GITERR_CHECK_ALLOC_ADD(&alloclen, len, 1);
157+
ENSURE_SIZE(buf, alloclen);
155158
memmove(buf->ptr, data, len);
156159
}
157160

@@ -180,18 +183,20 @@ int git_buf_sets(git_buf *buf, const char *string)
180183

181184
int git_buf_putc(git_buf *buf, char c)
182185
{
183-
GITERR_CHECK_ALLOC_ADD(buf->size, 2);
184-
ENSURE_SIZE(buf, buf->size + 2);
186+
size_t new_size;
187+
GITERR_CHECK_ALLOC_ADD(&new_size, buf->size, 2);
188+
ENSURE_SIZE(buf, new_size);
185189
buf->ptr[buf->size++] = c;
186190
buf->ptr[buf->size] = '\0';
187191
return 0;
188192
}
189193

190194
int git_buf_putcn(git_buf *buf, char c, size_t len)
191195
{
192-
GITERR_CHECK_ALLOC_ADD(buf->size, len);
193-
GITERR_CHECK_ALLOC_ADD(buf->size + len, 1);
194-
ENSURE_SIZE(buf, buf->size + len + 1);
196+
size_t new_size;
197+
GITERR_CHECK_ALLOC_ADD(&new_size, buf->size, len);
198+
GITERR_CHECK_ALLOC_ADD(&new_size, new_size, 1);
199+
ENSURE_SIZE(buf, new_size);
195200
memset(buf->ptr + buf->size, c, len);
196201
buf->size += len;
197202
buf->ptr[buf->size] = '\0';
@@ -201,10 +206,13 @@ int git_buf_putcn(git_buf *buf, char c, size_t len)
201206
int git_buf_put(git_buf *buf, const char *data, size_t len)
202207
{
203208
if (len) {
209+
size_t new_size;
210+
204211
assert(data);
205-
GITERR_CHECK_ALLOC_ADD(buf->size, len);
206-
GITERR_CHECK_ALLOC_ADD(buf->size + len, 1);
207-
ENSURE_SIZE(buf, buf->size + len + 1);
212+
213+
GITERR_CHECK_ALLOC_ADD(&new_size, buf->size, len);
214+
GITERR_CHECK_ALLOC_ADD(&new_size, new_size, 1);
215+
ENSURE_SIZE(buf, new_size);
208216
memmove(buf->ptr + buf->size, data, len);
209217
buf->size += len;
210218
buf->ptr[buf->size] = '\0';
@@ -226,12 +234,13 @@ int git_buf_encode_base64(git_buf *buf, const char *data, size_t len)
226234
size_t extra = len % 3;
227235
uint8_t *write, a, b, c;
228236
const uint8_t *read = (const uint8_t *)data;
229-
size_t blocks = (len / 3) + !!extra;
237+
size_t blocks = (len / 3) + !!extra, alloclen;
238+
239+
GITERR_CHECK_ALLOC_ADD(&blocks, blocks, 1);
240+
GITERR_CHECK_ALLOC_MULTIPLY(&alloclen, blocks, 4);
241+
GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, buf->size);
230242

231-
GITERR_CHECK_ALLOC_MULTIPLY(blocks, 4);
232-
GITERR_CHECK_ALLOC_ADD(buf->size, 4 * blocks);
233-
GITERR_CHECK_ALLOC_ADD(buf->size + 4 * blocks, 1);
234-
ENSURE_SIZE(buf, buf->size + 4 * blocks + 1);
243+
ENSURE_SIZE(buf, alloclen);
235244
write = (uint8_t *)&buf->ptr[buf->size];
236245

237246
/* convert each run of 3 bytes into 4 output bytes */
@@ -282,12 +291,12 @@ int git_buf_decode_base64(git_buf *buf, const char *base64, size_t len)
282291
{
283292
size_t i;
284293
int8_t a, b, c, d;
285-
size_t orig_size = buf->size;
294+
size_t orig_size = buf->size, new_size;
286295

287296
assert(len % 4 == 0);
288-
GITERR_CHECK_ALLOC_ADD(buf->size, len / 4 * 3);
289-
GITERR_CHECK_ALLOC_ADD(buf->size + (len / 4 * 3), 1);
290-
ENSURE_SIZE(buf, buf->size + (len / 4 * 3) + 1);
297+
GITERR_CHECK_ALLOC_ADD(&new_size, (len / 4 * 3), buf->size);
298+
GITERR_CHECK_ALLOC_ADD(&new_size, new_size, 1);
299+
ENSURE_SIZE(buf, new_size);
291300

292301
for (i = 0; i < len; i += 4) {
293302
if ((a = BASE64_DECODE_VALUE(base64[i])) < 0 ||
@@ -315,12 +324,13 @@ static const char b85str[] =
315324

316325
int git_buf_encode_base85(git_buf *buf, const char *data, size_t len)
317326
{
318-
size_t blocks = (len / 4) + !!(len % 4);
327+
size_t blocks = (len / 4) + !!(len % 4), alloclen;
319328

320-
GITERR_CHECK_ALLOC_MULTIPLY(blocks, 5);
321-
GITERR_CHECK_ALLOC_ADD(buf->size, 5 * blocks);
322-
GITERR_CHECK_ALLOC_ADD(buf->size + 5 * blocks, 1);
323-
ENSURE_SIZE(buf, buf->size + blocks * 5 + 1);
329+
GITERR_CHECK_ALLOC_MULTIPLY(&alloclen, blocks, 5);
330+
GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, buf->size);
331+
GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1);
332+
333+
ENSURE_SIZE(buf, alloclen);
324334

325335
while (len) {
326336
uint32_t acc = 0;
@@ -353,15 +363,11 @@ int git_buf_encode_base85(git_buf *buf, const char *data, size_t len)
353363

354364
int git_buf_vprintf(git_buf *buf, const char *format, va_list ap)
355365
{
356-
size_t expected_size = strlen(format);
366+
size_t expected_size, new_size;
357367
int len;
358368

359-
GITERR_CHECK_ALLOC_MULTIPLY(expected_size, 2);
360-
expected_size *= 2;
361-
362-
GITERR_CHECK_ALLOC_ADD(expected_size, buf->size);
363-
expected_size += buf->size;
364-
369+
GITERR_CHECK_ALLOC_MULTIPLY(&expected_size, strlen(format), 2);
370+
GITERR_CHECK_ALLOC_ADD(&expected_size, expected_size, buf->size);
365371
ENSURE_SIZE(buf, expected_size);
366372

367373
while (1) {
@@ -387,9 +393,9 @@ int git_buf_vprintf(git_buf *buf, const char *format, va_list ap)
387393
break;
388394
}
389395

390-
GITERR_CHECK_ALLOC_ADD(buf->size, len);
391-
GITERR_CHECK_ALLOC_ADD(buf->size + len, 1);
392-
ENSURE_SIZE(buf, buf->size + len + 1);
396+
GITERR_CHECK_ALLOC_ADD(&new_size, buf->size, len);
397+
GITERR_CHECK_ALLOC_ADD(&new_size, new_size, 1);
398+
ENSURE_SIZE(buf, new_size);
393399
}
394400

395401
return 0;
@@ -516,18 +522,20 @@ int git_buf_join_n(git_buf *buf, char separator, int nbuf, ...)
516522
continue;
517523

518524
segment_len = strlen(segment);
519-
total_size += segment_len;
525+
526+
GITERR_CHECK_ALLOC_ADD(&total_size, total_size, segment_len);
527+
520528
if (segment_len == 0 || segment[segment_len - 1] != separator)
521-
++total_size; /* space for separator */
529+
GITERR_CHECK_ALLOC_ADD(&total_size, total_size, 1);
522530
}
523531
va_end(ap);
524532

525533
/* expand buffer if needed */
526534
if (total_size == 0)
527535
return 0;
528536

529-
GITERR_CHECK_ALLOC_ADD(total_size, 1);
530-
if (git_buf_grow_by(buf, total_size + 1) < 0)
537+
GITERR_CHECK_ALLOC_ADD(&total_size, total_size, 1);
538+
if (git_buf_grow_by(buf, total_size) < 0)
531539
return -1;
532540

533541
out = buf->ptr + buf->size;
@@ -588,6 +596,7 @@ int git_buf_join(
588596
{
589597
size_t strlen_a = str_a ? strlen(str_a) : 0;
590598
size_t strlen_b = strlen(str_b);
599+
size_t alloc_len;
591600
int need_sep = 0;
592601
ssize_t offset_a = -1;
593602

@@ -605,10 +614,10 @@ int git_buf_join(
605614
if (str_a >= buf->ptr && str_a < buf->ptr + buf->size)
606615
offset_a = str_a - buf->ptr;
607616

608-
GITERR_CHECK_ALLOC_ADD(strlen_a, strlen_b);
609-
GITERR_CHECK_ALLOC_ADD(strlen_a + strlen_b, need_sep);
610-
GITERR_CHECK_ALLOC_ADD(strlen_a + strlen_b + need_sep, 1);
611-
if (git_buf_grow(buf, strlen_a + strlen_b + need_sep + 1) < 0)
617+
GITERR_CHECK_ALLOC_ADD(&alloc_len, strlen_a, strlen_b);
618+
GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, need_sep);
619+
GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 1);
620+
if (git_buf_grow(buf, alloc_len) < 0)
612621
return -1;
613622
assert(buf->ptr);
614623

@@ -636,7 +645,10 @@ int git_buf_join3(
636645
const char *str_b,
637646
const char *str_c)
638647
{
639-
size_t len_a = strlen(str_a), len_b = strlen(str_b), len_c = strlen(str_c);
648+
size_t len_a = strlen(str_a),
649+
len_b = strlen(str_b),
650+
len_c = strlen(str_c),
651+
len_total;
640652
int sep_a = 0, sep_b = 0;
641653
char *tgt;
642654

@@ -656,12 +668,12 @@ int git_buf_join3(
656668
sep_b = (str_b[len_b - 1] != separator);
657669
}
658670

659-
GITERR_CHECK_ALLOC_ADD(len_a, sep_a);
660-
GITERR_CHECK_ALLOC_ADD(len_a + sep_a, len_b);
661-
GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b, sep_b);
662-
GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b + sep_b, len_c);
663-
GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b + sep_b + len_c, 1);
664-
if (git_buf_grow(buf, len_a + sep_a + len_b + sep_b + len_c + 1) < 0)
671+
GITERR_CHECK_ALLOC_ADD(&len_total, len_a, sep_a);
672+
GITERR_CHECK_ALLOC_ADD(&len_total, len_total, len_b);
673+
GITERR_CHECK_ALLOC_ADD(&len_total, len_total, sep_b);
674+
GITERR_CHECK_ALLOC_ADD(&len_total, len_total, len_c);
675+
GITERR_CHECK_ALLOC_ADD(&len_total, len_total, 1);
676+
if (git_buf_grow(buf, len_total) < 0)
665677
return -1;
666678

667679
tgt = buf->ptr;
@@ -714,28 +726,25 @@ int git_buf_splice(
714726
const char *data,
715727
size_t nb_to_insert)
716728
{
717-
size_t new_size;
729+
char *splice_loc;
730+
size_t new_size, alloc_size;
718731

719-
assert(buf &&
720-
where <= git_buf_len(buf) &&
721-
where + nb_to_remove <= git_buf_len(buf));
732+
assert(buf && where <= buf->size && nb_to_remove <= buf->size - where);
733+
734+
splice_loc = buf->ptr + where;
722735

723736
/* Ported from git.git
724737
* https://github.com/git/git/blob/16eed7c/strbuf.c#L159-176
725738
*/
726-
new_size = buf->size - nb_to_remove;
727-
728-
GITERR_CHECK_ALLOC_ADD(new_size, nb_to_insert);
729-
new_size += nb_to_insert;
730-
731-
GITERR_CHECK_ALLOC_ADD(new_size, 1);
732-
ENSURE_SIZE(buf, new_size + 1);
739+
GITERR_CHECK_ALLOC_ADD(&new_size, (buf->size - nb_to_remove), nb_to_insert);
740+
GITERR_CHECK_ALLOC_ADD(&alloc_size, new_size, 1);
741+
ENSURE_SIZE(buf, alloc_size);
733742

734-
memmove(buf->ptr + where + nb_to_insert,
735-
buf->ptr + where + nb_to_remove,
736-
buf->size - where - nb_to_remove);
743+
memmove(splice_loc + nb_to_insert,
744+
splice_loc + nb_to_remove,
745+
buf->size - where - nb_to_remove);
737746

738-
memcpy(buf->ptr + where, data, nb_to_insert);
747+
memcpy(splice_loc, data, nb_to_insert);
739748

740749
buf->size = new_size;
741750
buf->ptr[buf->size] = '\0';

0 commit comments

Comments
 (0)