Commit 458cea5c5b820f9766cb5ba4c3d89830592d655b

Edward Thomson 2017-06-08T14:22:24

Merge pull request #4255 from pks-t/pks/buffer-grow-errors Buffer growing cleanups

diff --git a/src/buffer.c b/src/buffer.c
index fdb732d..6dfcbfb 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -18,18 +18,19 @@ char git_buf__initbuf[1];
 char git_buf__oom[1];
 
 #define ENSURE_SIZE(b, d) \
-	if ((d) > buf->asize && git_buf_grow(b, (d)) < 0)\
+	if ((d) > (b)->asize && git_buf_grow((b), (d)) < 0)\
 		return -1;
 
 
-void git_buf_init(git_buf *buf, size_t initial_size)
+int git_buf_init(git_buf *buf, size_t initial_size)
 {
 	buf->asize = 0;
 	buf->size = 0;
 	buf->ptr = git_buf__initbuf;
 
-	if (initial_size)
-		git_buf_grow(buf, initial_size);
+	ENSURE_SIZE(buf, initial_size);
+
+	return 0;
 }
 
 int git_buf_try_grow(
@@ -577,7 +578,7 @@ char *git_buf_detach(git_buf *buf)
 	return data;
 }
 
-void git_buf_attach(git_buf *buf, char *ptr, size_t asize)
+int git_buf_attach(git_buf *buf, char *ptr, size_t asize)
 {
 	git_buf_free(buf);
 
@@ -588,9 +589,10 @@ void git_buf_attach(git_buf *buf, char *ptr, size_t asize)
 			buf->asize = (asize < buf->size) ? buf->size + 1 : asize;
 		else /* pass 0 to fall back on strlen + 1 */
 			buf->asize = buf->size + 1;
-	} else {
-		git_buf_grow(buf, asize);
 	}
+
+	ENSURE_SIZE(buf, asize);
+	return 0;
 }
 
 void git_buf_attach_notowned(git_buf *buf, const char *ptr, size_t size)
@@ -724,9 +726,7 @@ int git_buf_join(
 	GITERR_CHECK_ALLOC_ADD(&alloc_len, strlen_a, strlen_b);
 	GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, need_sep);
 	GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 1);
-	if (git_buf_grow(buf, alloc_len) < 0)
-		return -1;
-	assert(buf->ptr);
+	ENSURE_SIZE(buf, alloc_len);
 
 	/* fix up internal pointers */
 	if (offset_a >= 0)
@@ -780,8 +780,7 @@ int git_buf_join3(
 	GITERR_CHECK_ALLOC_ADD(&len_total, len_total, sep_b);
 	GITERR_CHECK_ALLOC_ADD(&len_total, len_total, len_c);
 	GITERR_CHECK_ALLOC_ADD(&len_total, len_total, 1);
-	if (git_buf_grow(buf, len_total) < 0)
-		return -1;
+	ENSURE_SIZE(buf, len_total);
 
 	tgt = buf->ptr;
 
diff --git a/src/buffer.h b/src/buffer.h
index a76b2d7..b0aece4 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -34,7 +34,7 @@ GIT_INLINE(bool) git_buf_is_allocated(const git_buf *buf)
  * For the cases where GIT_BUF_INIT cannot be used to do static
  * initialization.
  */
-extern void git_buf_init(git_buf *buf, size_t initial_size);
+extern int git_buf_init(git_buf *buf, size_t initial_size);
 
 /**
  * Resize the buffer allocation to make more space.
@@ -73,7 +73,7 @@ extern void git_buf_sanitize(git_buf *buf);
 
 extern void git_buf_swap(git_buf *buf_a, git_buf *buf_b);
 extern char *git_buf_detach(git_buf *buf);
-extern void git_buf_attach(git_buf *buf, char *ptr, size_t asize);
+extern int git_buf_attach(git_buf *buf, char *ptr, size_t asize);
 
 /* Populates a `git_buf` where the contents are not "owned" by the
  * buffer, and calls to `git_buf_free` will not free the given buf.
diff --git a/src/config_file.c b/src/config_file.c
index 2302d33..e15d57b 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -1267,7 +1267,7 @@ static const char *escaped = "\n\t\b\"\\";
 /* Escape the values to write them to the file */
 static char *escape_value(const char *ptr)
 {
-	git_buf buf = GIT_BUF_INIT;
+	git_buf buf;
 	size_t len;
 	const char *esc;
 
@@ -1277,7 +1277,8 @@ static char *escape_value(const char *ptr)
 	if (!len)
 		return git__calloc(1, sizeof(char));
 
-	git_buf_grow(&buf, len);
+	if (git_buf_init(&buf, len) < 0)
+		return NULL;
 
 	while (*ptr != '\0') {
 		if ((esc = strchr(escaped, *ptr)) != NULL) {
diff --git a/src/pack.c b/src/pack.c
index 60b757e..f8d0dc9 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -312,7 +312,7 @@ static int pack_index_open(struct git_pack_file *p)
 {
 	int error = 0;
 	size_t name_len;
-	git_buf idx_name = GIT_BUF_INIT;
+	git_buf idx_name;
 
 	if (p->index_version > -1)
 		return 0;
@@ -320,11 +320,13 @@ static int pack_index_open(struct git_pack_file *p)
 	name_len = strlen(p->pack_name);
 	assert(name_len > strlen(".pack")); /* checked by git_pack_file alloc */
 
-	git_buf_grow(&idx_name, name_len);
+	if (git_buf_init(&idx_name, name_len) < 0)
+		return -1;
+
 	git_buf_put(&idx_name, p->pack_name, name_len - strlen(".pack"));
 	git_buf_puts(&idx_name, ".idx");
 	if (git_buf_oom(&idx_name)) {
-		giterr_set_oom();
+		git_buf_free(&idx_name);
 		return -1;
 	}
 
diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c
index e8e8480..fb504c9 100644
--- a/src/transports/winhttp.c
+++ b/src/transports/winhttp.c
@@ -429,7 +429,6 @@ static int winhttp_stream_connect(winhttp_stream *s)
 			git_buf_printf(&processed_url, ":%s", t->proxy_connection_data.port);
 
 		if (git_buf_oom(&processed_url)) {
-			giterr_set_oom();
 			error = -1;
 			goto on_error;
 		}