Optimized of git_buf_join.

This streamlines git_buf_join and removes the join-append behavior,
opting instead for a very compact join-replace of the git_buf contents.
The unit tests had to be updated to remove the join-append tests and
have a bunch more exhaustive tests added.

5 files changed, 66 insertions, 65 deletions

diff --git a/src/buffer.c b/src/buffer.c
index 3dff813e3..b90dd29c5 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -282,58 +282,24 @@ void git_buf_join(
 	const char *str_a,
 	const char *str_b)
 {
-	size_t add_size = 0;
-	size_t sep_a = 0;
-	size_t strlen_a = 0;
-	size_t sep_b = 0;
-	size_t strlen_b = 0;
-	char *ptr;
-
-	/* calculate string lengths and need for added separators */
-	if (str_a) {
-		while (*str_a == separator) { sep_a = 1; str_a++; }
-		strlen_a = strlen(str_a);
+	size_t strlen_a = strlen(str_a);
+	size_t strlen_b = strlen(str_b);
+	int need_sep = 0;
+
+	/* figure out if we need to insert a separator */
+	if (separator && strlen_a) {
+		while (*str_b == separator) { str_b++; strlen_b--; }
+		if (str_a[strlen_a - 1] != separator)
+			need_sep = 1;
 	}
-	if (str_b) {
-		while (*str_b == separator) { sep_b = 1; str_b++; }
-		strlen_b = strlen(str_b);
-	}
-	if (buf->size > 0) {
-		if (buf->ptr[buf->size - 1] == separator) {
-			sep_a = 0;
-			if (!strlen_a) sep_b = 0;
-		} else if (!strlen_a)
-			sep_b = (str_b != NULL);
-	}
-	if (strlen_a > 0) {
-		if (str_a[strlen_a - 1] == separator)
-			sep_b = 0;
-		else if (str_b)
-			sep_b = 1;
-	}
-
-	add_size = sep_a + strlen_a + sep_b + strlen_b;
 
-	if (!add_size) return;
+	ENSURE_SIZE(buf, strlen_a + strlen_b + need_sep + 1);
 
-	ENSURE_SIZE(buf, buf->size + add_size + 1);
+	memmove(buf->ptr, str_a, strlen_a);
+	if (need_sep)
+		buf->ptr[strlen_a] = separator;
+	memmove(buf->ptr + strlen_a + need_sep, str_b, strlen_b);
 
-	/* concatenate strings */
-	ptr = buf->ptr + buf->size;
-	if (sep_a)
-		*ptr++ = separator;
-	if (strlen_a) {
-		memmove(ptr, str_a, strlen_a);
-		ptr += strlen_a;
-	}
-	if (sep_b)
-		*ptr++ = separator;
-	if (strlen_b) {
-		memmove(ptr, str_b, strlen_b);
-		ptr += strlen_b;
-	}
-
-	/* set size based on num characters actually written */
-	buf->size = ptr - buf->ptr;
+	buf->size = strlen_a + strlen_b + need_sep;
 	buf->ptr[buf->size] = '\0';
 }
diff --git a/tests-clay/clay.h b/tests-clay/clay.h
index e75ef856e..b3aae467c 100644
--- a/tests-clay/clay.h
+++ b/tests-clay/clay.h
@@ -92,6 +92,7 @@ extern void test_core_buffer__5(void);
 extern void test_core_buffer__6(void);
 extern void test_core_buffer__7(void);
 extern void test_core_buffer__8(void);
+extern void test_core_buffer__9(void);
 extern void test_core_dirent__dont_traverse_dot(void);
 extern void test_core_dirent__dont_traverse_empty_folders(void);
 extern void test_core_dirent__traverse_slash_terminated_folder(void);
diff --git a/tests-clay/clay_libgit2.h b/tests-clay/clay_libgit2.h
index 2eedc61d2..6b14b7d6e 100644
--- a/tests-clay/clay_libgit2.h
+++ b/tests-clay/clay_libgit2.h
@@ -29,15 +29,16 @@
  * Wrapper for string comparison that knows about nulls.
  */
 #define cl_assert_strequal(a,b) \
-	cl_assert_strequal_internal(a,b,__FILE__,__LINE__)
+	cl_assert_strequal_internal(a,b,__FILE__,__LINE__,"string mismatch: " #a " != " #b)
 
-GIT_INLINE(void) cl_assert_strequal_internal(const char *a, const char *b, const char *file, int line)
+GIT_INLINE(void) cl_assert_strequal_internal(
+	const char *a, const char *b, const char *file, int line, const char *err)
 {
 	int match = (a == NULL || b == NULL) ? (a == b) : (strcmp(a, b) == 0);
 	if (!match) {
 		char buf[4096];
 		snprintf(buf, 4096, "'%s' != '%s'", a, b);
-		clay__assert(0, file, line, buf, "Strings do not match", 1);
+		clay__assert(0, file, line, buf, err, 1);
 	}
 }
 
diff --git a/tests-clay/clay_main.c b/tests-clay/clay_main.c
index 65c56de8d..409ce7643 100644
--- a/tests-clay/clay_main.c
+++ b/tests-clay/clay_main.c
@@ -145,7 +145,8 @@ static const struct clay_func _clay_cb_core_buffer[] = {
 	{"5", &test_core_buffer__5},
 	{"6", &test_core_buffer__6},
 	{"7", &test_core_buffer__7},
-	{"8", &test_core_buffer__8}
+	{"8", &test_core_buffer__8},
+	{"9", &test_core_buffer__9}
 };
 static const struct clay_func _clay_cb_core_dirent[] = {
     {"dont_traverse_dot", &test_core_dirent__dont_traverse_dot},
@@ -326,7 +327,7 @@ static const struct clay_suite _clay_suites[] = {
         "core::buffer",
         {NULL, NULL},
         {NULL, NULL},
-        _clay_cb_core_buffer, 9
+        _clay_cb_core_buffer, 10
     },
 	{
         "core::dirent",
@@ -493,7 +494,7 @@ static const struct clay_suite _clay_suites[] = {
 };
 
 static size_t _clay_suite_count = 34;
-static size_t _clay_callback_count = 112;
+static size_t _clay_callback_count = 113;
 
 /* Core test functions */
 static void
diff --git a/tests-clay/core/buffer.c b/tests-clay/core/buffer.c
index acde8c0a7..7d25b8179 100644
--- a/tests-clay/core/buffer.c
+++ b/tests-clay/core/buffer.c
@@ -374,20 +374,10 @@ check_joinbuf_2(
 	char sep = '/';
 	git_buf buf = GIT_BUF_INIT;
 
-	/* first validate join from nothing */
 	git_buf_join(&buf, sep, a, b);
 	cl_assert(git_buf_oom(&buf) == 0);
 	cl_assert_strequal(expected, git_buf_cstr(&buf));
 	git_buf_free(&buf);
-
-	/* next validate join-append */
-	git_buf_sets(&buf, a);
-	cl_assert(git_buf_oom(&buf) == 0);
-	git_buf_join(&buf, sep, NULL, b);
-	cl_assert(git_buf_oom(&buf) == 0);
-	cl_assert_strequal(expected, git_buf_cstr(&buf));
-
-	git_buf_free(&buf);
 }
 
 static void
@@ -490,3 +480,45 @@ void test_core_buffer__8(void)
 	check_joinbuf_n_4(";abcd;", ";efgh;", ";ijkl;", ";mnop;", ";abcd;efgh;ijkl;mnop;");
 }
 
+void test_core_buffer__9(void)
+{
+	git_buf buf = GIT_BUF_INIT;
+
+	/* just some exhaustive tests of various separator placement */
+	char *a[] = { "", "-", "a-", "-a", "-a-" };
+	char *b[] = { "", "-", "b-", "-b", "-b-" };
+	char sep[] = { 0, '-', '/' };
+	char *expect_null[] = { "",    "-",     "a-",     "-a",     "-a-",
+							"-",   "--",    "a--",    "-a-",    "-a--",
+							"b-",  "-b-",   "a-b-",   "-ab-",   "-a-b-",
+							"-b",  "--b",   "a--b",   "-a-b",   "-a--b",
+							"-b-", "--b-",  "a--b-",  "-a-b-",  "-a--b-" };
+	char *expect_dash[] = { "",    "-",     "a-",     "-a-",    "-a-",
+							"-",   "-",     "a-",     "-a-",    "-a-",
+							"b-",  "-b-",   "a-b-",   "-a-b-",  "-a-b-",
+							"-b",  "-b",    "a-b",    "-a-b",   "-a-b",
+							"-b-", "-b-",   "a-b-",   "-a-b-",  "-a-b-" };
+	char *expect_slas[] = { "",    "-/",    "a-/",    "-a/",    "-a-/",
+							"-",   "-/-",   "a-/-",   "-a/-",   "-a-/-",
+							"b-",  "-/b-",  "a-/b-",  "-a/b-",  "-a-/b-",
+							"-b",  "-/-b",  "a-/-b",  "-a/-b",  "-a-/-b",
+							"-b-", "-/-b-", "a-/-b-", "-a/-b-", "-a-/-b-" };
+	char **expect_values[] = { expect_null, expect_dash, expect_slas };
+	char separator, **expect;
+	unsigned int s, i, j;
+
+	for (s = 0; s < sizeof(sep) / sizeof(char); ++s) {
+		separator = sep[s];
+		expect = expect_values[s];
+
+		for (j = 0; j < sizeof(b) / sizeof(char*); ++j) {
+			for (i = 0; i < sizeof(a) / sizeof(char*); ++i) {
+				git_buf_join(&buf, separator, a[i], b[j]);
+				cl_assert_strequal(*expect, buf.ptr);
+				expect++;
+			}
+		}
+	}
+
+	git_buf_free(&buf);
+}