Merge pull request #3246 from libgit2/cmn/dont-grow-borrowed Don't allow growing borrowed buffers
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 86f9955..21774ca 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 c24ff7e..4a6d8e5 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 dda0f8a..5ff0f35 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 f633c5e..1a5809c 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 093ed9b..e46ee5d 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 c2c90e4..2558058 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 fef37f8..0e7026a 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));
+}