Commit c63728cd73c14093665880b26505418581d7a29a

Russell Belfer 2011-11-29T16:39:49

Make git_buf functions always maintain a valid cstr. At a tiny cost of 1 extra byte per allocation, this makes git_buf_cstr into basically a noop, which simplifies error checking when trying to convert things to use dynamic allocation. This patch also adds a new function (git_buf_copy_cstr) for copying the cstr data directly into an external buffer.

diff --git a/src/buffer.c b/src/buffer.c
index c56a755..aa66261 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -53,9 +53,10 @@ void git_buf_set(git_buf *buf, const char *data, size_t len)
 	if (len == 0 || data == NULL) {
 		git_buf_clear(buf);
 	} else {
-		ENSURE_SIZE(buf, len);
+		ENSURE_SIZE(buf, len + 1);
 		memmove(buf->ptr, data, len);
 		buf->size = len;
+		buf->ptr[buf->size] = '\0';
 	}
 }
 
@@ -66,15 +67,17 @@ void git_buf_sets(git_buf *buf, const char *string)
 
 void git_buf_putc(git_buf *buf, char c)
 {
-	ENSURE_SIZE(buf, buf->size + 1);
+	ENSURE_SIZE(buf, buf->size + 2);
 	buf->ptr[buf->size++] = c;
+	buf->ptr[buf->size] = '\0';
 }
 
 void git_buf_put(git_buf *buf, const char *data, size_t len)
 {
-	ENSURE_SIZE(buf, buf->size + len);
+	ENSURE_SIZE(buf, buf->size + len + 1);
 	memmove(buf->ptr + buf->size, data, len);
 	buf->size += len;
+	buf->ptr[buf->size] = '\0';
 }
 
 void git_buf_puts(git_buf *buf, const char *string)
@@ -111,12 +114,25 @@ void git_buf_printf(git_buf *buf, const char *format, ...)
 
 const char *git_buf_cstr(git_buf *buf)
 {
-	if (buf->size + 1 > buf->asize &&
-		git_buf_grow(buf, buf->size + 1) < GIT_SUCCESS)
-		return NULL;
+	return buf->ptr ? buf->ptr : "";
+}
 
-	buf->ptr[buf->size] = '\0';
-	return buf->ptr;
+void git_buf_copy_cstr(char *data, size_t datasize, git_buf *buf)
+{
+	size_t copylen;
+
+	assert(data && datasize);
+
+	data[0] = '\0';
+
+	if (buf->size == 0 || buf->asize <= 0)
+		return;
+
+	copylen = buf->size;
+	if (copylen > datasize - 1)
+		copylen = datasize - 1;
+	memmove(data, buf->ptr, copylen);
+	data[copylen] = '\0';
 }
 
 void git_buf_free(git_buf *buf)
@@ -132,6 +148,8 @@ void git_buf_free(git_buf *buf)
 void git_buf_clear(git_buf *buf)
 {
 	buf->size = 0;
+	if (buf->ptr)
+		*buf->ptr = '\0';
 }
 
 void git_buf_consume(git_buf *buf, const char *end)
@@ -140,6 +158,7 @@ void git_buf_consume(git_buf *buf, const char *end)
 		size_t consumed = end - buf->ptr;
 		memmove(buf->ptr, end, buf->size - consumed);
 		buf->size -= consumed;
+		buf->ptr[buf->size] = '\0';
 	}
 }
 
@@ -157,12 +176,9 @@ char *git_buf_take_cstr(git_buf *buf)
 	if (buf->ptr == NULL)
 		return NULL;
 
-	if (buf->size + 1 > buf->asize &&
-		git_buf_grow(buf, buf->size + 1) < GIT_SUCCESS)
-		return NULL;
+	assert(buf->asize > buf->size);
 
 	data = buf->ptr;
-	data[buf->size] = '\0';
 
 	buf->ptr = NULL;
 	buf->asize = 0;
@@ -199,7 +215,7 @@ void git_buf_join_n(git_buf *buf, char separator, int nbuf, ...)
 	}
 	va_end(ap);
 
-	ENSURE_SIZE(buf, buf->size + total_size);
+	ENSURE_SIZE(buf, buf->size + total_size + 1);
 
 	out = buf->ptr + buf->size;
 
@@ -235,6 +251,7 @@ void git_buf_join_n(git_buf *buf, char separator, int nbuf, ...)
 
 	/* set size based on num characters actually written */
 	buf->size = out - buf->ptr;
+	buf->ptr[buf->size] = '\0';
 }
 
 void git_buf_join(
@@ -277,7 +294,7 @@ void git_buf_join(
 
 	if (!add_size) return;
 
-	ENSURE_SIZE(buf, buf->size + add_size);
+	ENSURE_SIZE(buf, buf->size + add_size + 1);
 
 	/* concatenate strings */
 	ptr = buf->ptr + buf->size;
@@ -296,4 +313,5 @@ void git_buf_join(
 
 	/* set size based on num characters actually written */
 	buf->size = ptr - buf->ptr;
+	buf->ptr[buf->size] = '\0';
 }
diff --git a/src/buffer.h b/src/buffer.h
index 2ed9047..9e8ebd0 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -42,6 +42,7 @@ void git_buf_join(git_buf *buf, char separator, const char *str_a, const char *s
 
 const char *git_buf_cstr(git_buf *buf);
 char *git_buf_take_cstr(git_buf *buf);
+void git_buf_copy_cstr(char *data, size_t datasize, git_buf *buf);
 
 #define git_buf_PUTS(buf, str) git_buf_put(buf, str, sizeof(str) - 1)
 
diff --git a/tests-clay/core/buffer.c b/tests-clay/core/buffer.c
index bf88a30..cf6e45b 100644
--- a/tests-clay/core/buffer.c
+++ b/tests-clay/core/buffer.c
@@ -57,14 +57,13 @@ void test_core_buffer__2(void)
 
 	/* this must be safe to do */
 	git_buf_free(&buf);
-
 	cl_assert(buf.size == 0);
 	cl_assert(buf.asize == 0);
 
 	/* empty buffer should be empty string */
 	cl_assert_strequal("", git_buf_cstr(&buf));
 	cl_assert(buf.size == 0);
-	cl_assert(buf.asize > 0);
+	/* cl_assert(buf.asize == 0); -- should not assume what git_buf does */
 
 	/* free should set us back to the beginning */
 	git_buf_free(&buf);
@@ -277,15 +276,15 @@ void test_core_buffer__5(void)
 	 */
 
 	check_buf_append("abcdefgh", "/", "abcdefgh/", 9, 16);
-	check_buf_append("abcdefgh", "ijklmno", "abcdefghijklmno", 15, 24);
+	check_buf_append("abcdefgh", "ijklmno", "abcdefghijklmno", 15, 16);
 	check_buf_append("abcdefgh", "ijklmnop", "abcdefghijklmnop", 16, 24);
 	check_buf_append("0123456789", "0123456789",
 					 "01234567890123456789", 20, 24);
 	check_buf_append(REP16("x"), REP16("o"),
 					 REP16("x") REP16("o"), 32, 40);
 
-	check_buf_append(test_4096, "", test_4096, 4096, 6144);
-	check_buf_append(test_4096, test_4096, test_8192, 8192, 9216);
+	check_buf_append(test_4096, "", test_4096, 4096, 4104);
+	check_buf_append(test_4096, test_4096, test_8192, 8192, 9240);
 
 	/* check sequences of appends */
 	check_buf_append_abc("a", "b", "c",