Commit 4aa664ae3953d99c2ae4cd769f02818bc122eebc

Edward Thomson 2015-02-10T23:55:07

git_buf_grow_by: increase buf asize incrementally Introduce `git_buf_grow_by` to incrementally increase the size of a `git_buf`, performing an overflow calculation on the growth.

diff --git a/src/buf_text.c b/src/buf_text.c
index ace54d7..08b86f4 100644
--- a/src/buf_text.c
+++ b/src/buf_text.c
@@ -29,9 +29,8 @@ int git_buf_text_puts_escaped(
 		scan += count;
 	}
 
-	GITERR_CHECK_ALLOC_ADD(buf->size, total);
-	GITERR_CHECK_ALLOC_ADD(buf->size + total, 1);
-	if (git_buf_grow(buf, buf->size + total + 1) < 0)
+	GITERR_CHECK_ALLOC_ADD(total, 1);
+	if (git_buf_grow_by(buf, total + 1) < 0)
 		return -1;
 
 	for (scan = string; *scan; ) {
@@ -129,7 +128,6 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src)
 
 	for (; next; scan = next + 1, next = memchr(scan, '\n', end - scan)) {
 		size_t copylen = next - scan;
-		size_t needsize;
 
 		/* if we find mixed line endings, bail */
 		if (next > start && next[-1] == '\r') {
@@ -137,11 +135,8 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src)
 			return GIT_PASSTHROUGH;
 		}
 
-		GITERR_CHECK_ALLOC_ADD(tgt->size, copylen);
-		GITERR_CHECK_ALLOC_ADD(tgt->size + copylen, 3);
-		needsize = tgt->size + copylen + 3;
-
-		if (tgt->asize < needsize && git_buf_grow(tgt, needsize) < 0)
+		GITERR_CHECK_ALLOC_ADD(copylen, 3);
+		if (git_buf_grow_by(tgt, copylen + 3) < 0)
 			return -1;
 
 		if (next > scan) {
diff --git a/src/buffer.c b/src/buffer.c
index ee8bba4..8098bb1 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -101,6 +101,18 @@ int git_buf_grow(git_buf *buffer, size_t target_size)
 	return git_buf_try_grow(buffer, target_size, true, true);
 }
 
+int git_buf_grow_by(git_buf *buffer, size_t additional_size)
+{
+	if (GIT_ALLOC_OVERFLOW_ADD(buffer->size, additional_size)) {
+		buffer->ptr = git_buf__oom;
+		giterr_set_oom();
+		return -1;
+	}
+
+	return git_buf_try_grow(
+		buffer, buffer->size + additional_size, true, true);	
+}
+
 void git_buf_free(git_buf *buf)
 {
 	if (!buf) return;
@@ -515,9 +527,8 @@ int git_buf_join_n(git_buf *buf, char separator, int nbuf, ...)
 	if (total_size == 0)
 		return 0;
 
-	GITERR_CHECK_ALLOC_ADD(buf->size, total_size);
-	GITERR_CHECK_ALLOC_ADD(buf->size + total_size, 1);
-	if (git_buf_grow(buf, buf->size + total_size + 1) < 0)
+	GITERR_CHECK_ALLOC_ADD(total_size, 1);
+	if (git_buf_grow_by(buf, total_size + 1) < 0)
 		return -1;
 
 	out = buf->ptr + buf->size;
diff --git a/src/buffer.h b/src/buffer.h
index 8ee4b53..52342e3 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -37,6 +37,18 @@ GIT_INLINE(bool) git_buf_is_allocated(const git_buf *buf)
 extern void git_buf_init(git_buf *buf, size_t initial_size);
 
 /**
+ * Resize the buffer allocation to make more space.
+ *
+ * This will attempt to grow the buffer to accommodate the additional size.
+ * It is similar to `git_buf_grow`, but performs the new size calculation,
+ * checking for overflow.
+ *
+ * Like `git_buf_grow`, if this is a user-supplied buffer, this will allocate
+ * a new buffer.
+ */
+extern int git_buf_grow_by(git_buf *buffer, size_t additional_size);
+
+/**
  * Attempt to grow the buffer to hold at least `target_size` bytes.
  *
  * If the allocation fails, this will return an error.  If `mark_oom` is true,
diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c
index 2f83e0d..ad81ad1 100644
--- a/src/transports/smart_pkt.c
+++ b/src/transports/smart_pkt.c
@@ -513,7 +513,7 @@ static int buffer_want_with_caps(const git_remote_head *head, transport_smart_ca
 	len = (unsigned int)
 		(strlen("XXXXwant ") + GIT_OID_HEXSZ + 1 /* NUL */ +
 		 git_buf_len(&str) + 1 /* LF */);
-	git_buf_grow(buf, git_buf_len(buf) + len);
+	git_buf_grow_by(buf, len);
 	git_oid_fmt(oid, &head->oid);
 	git_buf_printf(buf, "%04xwant %s %s\n", len, oid, git_buf_cstr(&str));
 	git_buf_free(&str);
diff --git a/src/zstream.c b/src/zstream.c
index 06660e9..2130bc3 100644
--- a/src/zstream.c
+++ b/src/zstream.c
@@ -134,8 +134,7 @@ int git_zstream_deflatebuf(git_buf *out, const void *in, size_t in_len)
 	while (!git_zstream_done(&zs)) {
 		size_t step = git_zstream_suggest_output_len(&zs), written;
 
-		GITERR_CHECK_ALLOC_ADD(out->size, step);
-		if ((error = git_buf_grow(out, out->size + step)) < 0)
+		if ((error = git_buf_grow_by(out, step)) < 0)
 			goto done;
 
 		written = out->asize - out->size;
diff --git a/tests/buf/basic.c b/tests/buf/basic.c
index d558757..dcc0916 100644
--- a/tests/buf/basic.c
+++ b/tests/buf/basic.c
@@ -15,6 +15,26 @@ void test_buf_basic__resize(void)
 	git_buf_free(&buf1);
 }
 
+void test_buf_basic__resize_incremental(void)
+{
+	git_buf buf1 = GIT_BUF_INIT;
+
+	/* Presently, asking for 6 bytes will round up to 8. */
+	cl_git_pass(git_buf_puts(&buf1, "Hello"));
+	cl_assert_equal_i(5, buf1.size);
+	cl_assert_equal_i(8, buf1.asize);
+
+	/* Ensure an additional byte does not realloc. */
+	cl_git_pass(git_buf_grow_by(&buf1, 1));
+	cl_assert_equal_i(5, buf1.size);
+	cl_assert_equal_i(8, buf1.asize);
+
+	/* But requesting many does. */
+	cl_git_pass(git_buf_grow_by(&buf1, 16));
+	cl_assert_equal_i(5, buf1.size);
+	cl_assert(buf1.asize > 8);
+}
+
 void test_buf_basic__printf(void)
 {
 	git_buf buf2 = GIT_BUF_INIT;
diff --git a/tests/buf/oom.c b/tests/buf/oom.c
index 709439a..b9fd29c 100644
--- a/tests/buf/oom.c
+++ b/tests/buf/oom.c
@@ -29,3 +29,13 @@ void test_buf_oom__grow(void)
 
 	git_buf_free(&buf);
 }
+
+void test_buf_oom__grow_by(void)
+{
+	git_buf buf = GIT_BUF_INIT;
+
+	buf.size = SIZE_MAX-10;
+
+	cl_assert(git_buf_grow_by(&buf, 50) == -1);
+	cl_assert(git_buf_oom(&buf));
+}