diff --git a/CHANGELOG.md b/CHANGELOG.md index 86f995545..21774ca20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -127,6 +127,9 @@ support for HTTPS connections insead of OpenSSL. configuration of the server, and tools can use this to show messages about failing to communicate with the server. +* A new error code `GIT_EINVALID` indicates that an argument to a + function is invalid, or an invalid operation was requested. + * `git_diff_index_to_workdir()` and `git_diff_tree_to_index()` will now produce deltas of type `GIT_DELTA_CONFLICTED` to indicate that the index side of the delta is a conflict. @@ -267,6 +270,9 @@ support for HTTPS connections insead of OpenSSL. * `GIT_EMERGECONFLICT` is now `GIT_ECONFLICT`, which more accurately describes the nature of the error. +* It is no longer allowed to call `git_buf_grow()` on buffers + borrowing the memory they point to. + v0.22 ------ diff --git a/include/git2/blob.h b/include/git2/blob.h index c24ff7e7f..4a6d8e50a 100644 --- a/include/git2/blob.h +++ b/include/git2/blob.h @@ -107,12 +107,10 @@ GIT_EXTERN(git_off_t) git_blob_rawsize(const git_blob *blob); * The output is written into a `git_buf` which the caller must free * when done (via `git_buf_free`). * - * If no filters need to be applied, then the `out` buffer will just be - * populated with a pointer to the raw content of the blob. In that case, - * be careful to *not* free the blob until done with the buffer. To keep - * the data detached from the blob, call `git_buf_grow` on the buffer - * with a `want_size` of 0 and the buffer will be reallocated to be - * detached from the blob. + * If no filters need to be applied, then the `out` buffer will just + * be populated with a pointer to the raw content of the blob. In + * that case, be careful to *not* free the blob until done with the + * buffer or copy it into memory you own. * * @param out The git_buf to be filled in * @param blob Pointer to the blob diff --git a/include/git2/errors.h b/include/git2/errors.h index dda0f8a57..5ff0f35b5 100644 --- a/include/git2/errors.h +++ b/include/git2/errors.h @@ -46,6 +46,7 @@ typedef enum { GIT_EAPPLIED = -18, /**< Patch/merge has already been applied */ GIT_EPEEL = -19, /**< The requested peel operation is not possible */ GIT_EEOF = -20, /**< Unexpected EOF */ + GIT_EINVALID = -21, /**< Invalid operation or input */ GIT_PASSTHROUGH = -30, /**< Internal only */ GIT_ITEROVER = -31, /**< Signals end of iteration with iterator */ diff --git a/src/buffer.c b/src/buffer.c index f633c5e02..1a5809cca 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -33,7 +33,7 @@ void git_buf_init(git_buf *buf, size_t initial_size) } int git_buf_try_grow( - git_buf *buf, size_t target_size, bool mark_oom, bool preserve_external) + git_buf *buf, size_t target_size, bool mark_oom) { char *new_ptr; size_t new_size; @@ -41,6 +41,11 @@ int git_buf_try_grow( if (buf->ptr == git_buf__oom) return -1; + if (buf->asize == 0 && buf->size != 0) { + giterr_set(GITERR_INVALID, "cannot grow a borrowed buffer"); + return GIT_EINVALID; + } + if (!target_size) target_size = buf->size; @@ -82,9 +87,6 @@ int git_buf_try_grow( return -1; } - if (preserve_external && !buf->asize && buf->ptr != NULL && buf->size > 0) - memcpy(new_ptr, buf->ptr, min(buf->size, new_size)); - buf->asize = new_size; buf->ptr = new_ptr; @@ -98,7 +100,7 @@ int git_buf_try_grow( int git_buf_grow(git_buf *buffer, size_t target_size) { - return git_buf_try_grow(buffer, target_size, true, true); + return git_buf_try_grow(buffer, target_size, true); } int git_buf_grow_by(git_buf *buffer, size_t additional_size) @@ -110,7 +112,7 @@ int git_buf_grow_by(git_buf *buffer, size_t additional_size) return -1; } - return git_buf_try_grow(buffer, newsize, true, true); + return git_buf_try_grow(buffer, newsize, true); } void git_buf_free(git_buf *buf) diff --git a/src/buffer.h b/src/buffer.h index 093ed9b60..e46ee5dd7 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -59,7 +59,7 @@ extern int git_buf_grow_by(git_buf *buffer, size_t additional_size); * into the newly allocated buffer. */ extern int git_buf_try_grow( - git_buf *buf, size_t target_size, bool mark_oom, bool preserve_external); + git_buf *buf, size_t target_size, bool mark_oom); /** * Sanitizes git_buf structures provided from user input. Users of the diff --git a/src/path.c b/src/path.c index c2c90e48d..2558058dd 100644 --- a/src/path.c +++ b/src/path.c @@ -640,7 +640,7 @@ static bool _check_dir_contents( /* leave base valid even if we could not make space for subdir */ if (GIT_ADD_SIZET_OVERFLOW(&alloc_size, dir_size, sub_size) || GIT_ADD_SIZET_OVERFLOW(&alloc_size, alloc_size, 2) || - git_buf_try_grow(dir, alloc_size, false, false) < 0) + git_buf_try_grow(dir, alloc_size, false) < 0) return false; /* save excursion */ @@ -847,7 +847,7 @@ int git_path_make_relative(git_buf *path, const char *parent) /* save the offset as we might realllocate the pointer */ offset = p - path->ptr; - if (git_buf_try_grow(path, alloclen, 1, 0) < 0) + if (git_buf_try_grow(path, alloclen, 1) < 0) return -1; p = path->ptr + offset; diff --git a/tests/core/buffer.c b/tests/core/buffer.c index fef37f8f7..0e7026a9c 100644 --- a/tests/core/buffer.c +++ b/tests/core/buffer.c @@ -1153,3 +1153,16 @@ void test_core_buffer__lf_and_crlf_conversions(void) git_buf_free(&src); git_buf_free(&tgt); } + +void test_core_buffer__dont_grow_borrowed(void) +{ + const char *somestring = "blah blah"; + git_buf buf = GIT_BUF_INIT; + + git_buf_attach_notowned(&buf, somestring, strlen(somestring) + 1); + cl_assert_equal_p(somestring, buf.ptr); + cl_assert_equal_i(0, buf.asize); + cl_assert_equal_i(strlen(somestring) + 1, buf.size); + + cl_git_fail_with(GIT_EINVALID, git_buf_grow(&buf, 1024)); +}