diff options
| -rw-r--r-- | src/buffer.c | 21 | ||||
| -rw-r--r-- | tests/core/buffer.c | 42 |
2 files changed, 53 insertions, 10 deletions
diff --git a/src/buffer.c b/src/buffer.c index 51fb48a45..61cf9675b 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -18,7 +18,8 @@ char git_buf__initbuf[1]; char git_buf__oom[1]; #define ENSURE_SIZE(b, d) \ - if ((d) > (b)->asize && git_buf_grow((b), (d)) < 0)\ + if ((b)->ptr == git_buf__oom || \ + ((d) > (b)->asize && git_buf_grow((b), (d)) < 0))\ return -1; @@ -58,20 +59,26 @@ int git_buf_try_grow( new_ptr = NULL; } else { new_size = buf->asize; + /* + * Grow the allocated buffer by 1.5 to allow + * re-use of memory holes resulting from the + * realloc. If this is still too small, then just + * use the target size. + */ + if ((new_size = (new_size << 1) - (new_size >> 1)) < target_size) + new_size = target_size; new_ptr = buf->ptr; } - /* grow the buffer size by 1.5, until it's big enough - * to fit our target size */ - while (new_size < target_size) - new_size = (new_size << 1) - (new_size >> 1); - /* round allocation up to multiple of 8 */ new_size = (new_size + 7) & ~7; if (new_size < buf->size) { - if (mark_oom) + if (mark_oom) { + if (buf->ptr && buf->ptr != git_buf__initbuf) + git__free(buf->ptr); buf->ptr = git_buf__oom; + } git_error_set_oom(); return -1; diff --git a/tests/core/buffer.c b/tests/core/buffer.c index 58c98cb28..4e895afd9 100644 --- a/tests/core/buffer.c +++ b/tests/core/buffer.c @@ -231,9 +231,9 @@ check_buf_append( git_buf_puts(&tgt, data_b); cl_assert(git_buf_oom(&tgt) == 0); cl_assert_equal_s(expected_data, git_buf_cstr(&tgt)); - cl_assert(tgt.size == expected_size); + cl_assert_equal_i(tgt.size, expected_size); if (expected_asize > 0) - cl_assert(tgt.asize == expected_asize); + cl_assert_equal_i(tgt.asize, expected_asize); git_buf_dispose(&tgt); } @@ -308,7 +308,7 @@ void test_core_buffer__5(void) REP16("x") REP16("o"), 32, 40); check_buf_append(test_4096, "", test_4096, 4096, 4104); - check_buf_append(test_4096, test_4096, test_8192, 8192, 9240); + check_buf_append(test_4096, test_4096, test_8192, 8192, 8200); /* check sequences of appends */ check_buf_append_abc("a", "b", "c", @@ -1204,3 +1204,39 @@ void test_core_buffer__dont_grow_borrowed(void) cl_git_fail_with(GIT_EINVALID, git_buf_grow(&buf, 1024)); } + +void test_core_buffer__dont_hit_infinite_loop_when_resizing(void) +{ + git_buf buf = GIT_BUF_INIT; + + cl_git_pass(git_buf_puts(&buf, "foobar")); + /* + * We do not care whether this succeeds or fails, which + * would depend on platform-specific allocation + * semantics. We only want to know that the function + * actually returns. + */ + (void)git_buf_try_grow(&buf, SIZE_MAX, true); + + git_buf_dispose(&buf); +} + +void test_core_buffer__avoid_printing_into_oom_buffer(void) +{ + git_buf buf = GIT_BUF_INIT; + + /* Emulate OOM situation with a previous allocation */ + buf.asize = 8; + buf.ptr = git_buf__oom; + + /* + * Print the same string again. As the buffer still has + * an `asize` of 8 due to the previous print, + * `ENSURE_SIZE` would not try to reallocate the array at + * all. As it didn't explicitly check for `git_buf__oom` + * in earlier versions, this would've resulted in it + * returning successfully and thus `git_buf_puts` would + * just print into the `git_buf__oom` array. + */ + cl_git_fail(git_buf_puts(&buf, "foobar")); +} |