From 7cbc6241cf75bd8bd198273231e994711c998716 Mon Sep 17 00:00:00 2001 From: Patrick Reynolds Date: Mon, 20 Jan 2014 11:41:21 -0600 Subject: [PATCH 1/2] fix corner cases and an undefined behavior --- src/buffer.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/buffer.c b/src/buffer.c index 3283c2d4f..318fee753 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -66,8 +66,10 @@ int git_buf_try_grow( new_ptr = git__realloc(new_ptr, new_size); if (!new_ptr) { - if (mark_oom) + if (mark_oom) { + if (buf->ptr) git__free(buf->ptr); buf->ptr = git_buf__oom; + } return -1; } @@ -432,7 +434,7 @@ int git_buf_join( ssize_t offset_a = -1; /* not safe to have str_b point internally to the buffer */ - assert(str_b < buf->ptr || str_b > buf->ptr + buf->size); + assert(str_b < buf->ptr || str_b >= buf->ptr + buf->size); /* figure out if we need to insert a separator */ if (separator && strlen_a) { @@ -447,13 +449,14 @@ int git_buf_join( if (git_buf_grow(buf, strlen_a + strlen_b + need_sep + 1) < 0) return -1; + assert(buf->ptr); /* fix up internal pointers */ if (offset_a >= 0) str_a = buf->ptr + offset_a; /* do the actual copying */ - if (offset_a != 0) + if (offset_a != 0 && str_a) memmove(buf->ptr, str_a, strlen_a); if (need_sep) buf->ptr[strlen_a] = separator; From abdaf9366262ea034e092e8a9a707f5a7837ca65 Mon Sep 17 00:00:00 2001 From: Patrick Reynolds Date: Mon, 20 Jan 2014 11:42:12 -0600 Subject: [PATCH 2/2] add unit tests for git_buf_join corner cases --- tests/core/buffer.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/core/buffer.c b/tests/core/buffer.c index 11d173d49..0e7dd3d70 100644 --- a/tests/core/buffer.c +++ b/tests/core/buffer.c @@ -405,6 +405,23 @@ check_joinbuf_2( git_buf_free(&buf); } +static void +check_joinbuf_overlapped( + const char *oldval, + int ofs_a, + const char *b, + const char *expected) +{ + char sep = '/'; + git_buf buf = GIT_BUF_INIT; + + git_buf_sets(&buf, oldval); + git_buf_join(&buf, sep, buf.ptr + ofs_a, b); + cl_assert(git_buf_oom(&buf) == 0); + cl_assert_equal_s(expected, git_buf_cstr(&buf)); + git_buf_free(&buf); +} + static void check_joinbuf_n_2( const char *a, @@ -480,6 +497,20 @@ void test_core_buffer__8(void) check_joinbuf_2("/abcd/", "defg/", "/abcd/defg/"); check_joinbuf_2("/abcd/", "/defg/", "/abcd/defg/"); + check_joinbuf_overlapped("abcd", 0, "efg", "abcd/efg"); + check_joinbuf_overlapped("abcd", 1, "efg", "bcd/efg"); + check_joinbuf_overlapped("abcd", 2, "efg", "cd/efg"); + check_joinbuf_overlapped("abcd", 3, "efg", "d/efg"); + check_joinbuf_overlapped("abcd", 4, "efg", "efg"); + check_joinbuf_overlapped("abc/", 2, "efg", "c/efg"); + check_joinbuf_overlapped("abc/", 3, "efg", "/efg"); + check_joinbuf_overlapped("abc/", 4, "efg", "efg"); + check_joinbuf_overlapped("abcd", 3, "", "d/"); + check_joinbuf_overlapped("abcd", 4, "", ""); + check_joinbuf_overlapped("abc/", 2, "", "c/"); + check_joinbuf_overlapped("abc/", 3, "", "/"); + check_joinbuf_overlapped("abc/", 4, "", ""); + check_joinbuf_n_2("", "", ""); check_joinbuf_n_2("", "a", "a"); check_joinbuf_n_2("", "/a", "/a");