Add two string git_buf_join and tweak input error checking.

This commit addresses two of the comments:
* renamed existing n-input git_buf_join to git_buf_join_n
* added new git_buf_join that always takes two inputs
* moved some parameter error checking to asserts
* extended unit tests to cover new version of git_buf_join

src/buffer.c

@@ -121,14 +121,14 @@ const char *git_buf_cstr(git_buf *buf)
 
 void git_buf_free(git_buf *buf)
 {
-	if (buf) {
+	assert(buf);
-		if (buf->ptr) {
+
-			git__free(buf->ptr);
+	if (buf->ptr) {
-			buf->ptr = NULL;
+		git__free(buf->ptr);
-		}
+		buf->ptr = NULL;
-		buf->asize = 0;
-		buf->size = 0;
 	}
+	buf->asize = 0;
+	buf->size = 0;
 }
 
 void git_buf_clear(git_buf *buf)
@@ -173,7 +173,7 @@ char *git_buf_take_cstr(git_buf *buf)
 	return data;
 }
 
-void git_buf_join(git_buf *buf, char separator, int nbuf, ...)
+void git_buf_join_n(git_buf *buf, char separator, int nbuf, ...)
 {
 	/* Make two passes to avoid multiple reallocation */
 
@@ -239,3 +239,63 @@ void git_buf_join(git_buf *buf, char separator, int nbuf, ...)
 	buf->size = out - buf->ptr;
 }
 
+void git_buf_join(
+	git_buf *buf,
+	char separator,
+	const char *str_a,
+	const char *str_b)
+{
+	int add_size = 0;
+	int sep_a = 0;
+	int strlen_a = 0;
+	int sep_b = 0;
+	int 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);
+	}
+	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, buf->size + add_size);
+
+	/* 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;
+}

src/buffer.h

@@ -37,7 +37,8 @@ void git_buf_puts(git_buf *buf, const char *string);
 void git_buf_printf(git_buf *buf, const char *format, ...) GIT_FORMAT_PRINTF(2, 3);
 void git_buf_clear(git_buf *buf);
 void git_buf_consume(git_buf *buf, const char *end);
-void git_buf_join(git_buf *buf, char separator, int nbuf, ...);
+void git_buf_join_n(git_buf *buf, char separator, int nbuf, ...);
+void git_buf_join(git_buf *buf, char separator, const char *str_a, const char *str_b);
 
 const char *git_buf_cstr(git_buf *buf);
 char *git_buf_take_cstr(git_buf *buf);

tests-clay/core/buffer.c

@@ -352,7 +352,32 @@ void test_core_buffer__7(void)
 
 
 static void
-check_joinbuf(
+check_joinbuf_2(
+	const char *a,
+	const char *b,
+	const char *expected)
+{
+	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
+check_joinbuf_n_2(
 	const char *a,
 	const char *b,
 	const char *expected)
@@ -363,7 +388,7 @@ check_joinbuf(
 	git_buf_sets(&buf, a);
 	cl_assert(git_buf_oom(&buf) == 0);
 
-	git_buf_join(&buf, sep, 1, b);
+	git_buf_join_n(&buf, sep, 1, b);
 	cl_assert(git_buf_oom(&buf) == 0);
 	cl_assert_strequal(expected, git_buf_cstr(&buf));
 
@@ -371,7 +396,7 @@ check_joinbuf(
 }
 
 static void
-check_joinbuf_n(
+check_joinbuf_n_4(
 	const char *a,
 	const char *b,
 	const char *c,
@@ -380,7 +405,7 @@ check_joinbuf_n(
 {
 	char sep = ';';
 	git_buf buf = GIT_BUF_INIT;
-	git_buf_join(&buf, sep, 4, a, b, c, d);
+	git_buf_join_n(&buf, sep, 4, a, b, c, d);
 	cl_assert(git_buf_oom(&buf) == 0);
 	cl_assert_strequal(expected, git_buf_cstr(&buf));
 	git_buf_free(&buf);
@@ -391,45 +416,63 @@ void test_core_buffer__8(void)
 {
 	git_buf a = GIT_BUF_INIT;
 
-	git_buf_join(&a, '/', 1, "foo");
+	git_buf_join_n(&a, '/', 1, "foo");
 	cl_assert(git_buf_oom(&a) == 0);
 	cl_assert_strequal("foo", git_buf_cstr(&a));
 
-	git_buf_join(&a, '/', 1, "bar");
+	git_buf_join_n(&a, '/', 1, "bar");
 	cl_assert(git_buf_oom(&a) == 0);
 	cl_assert_strequal("foo/bar", git_buf_cstr(&a));
 
-	git_buf_join(&a, '/', 1, "baz");
+	git_buf_join_n(&a, '/', 1, "baz");
 	cl_assert(git_buf_oom(&a) == 0);
 	cl_assert_strequal("foo/bar/baz", git_buf_cstr(&a));
 
 	git_buf_free(&a);
 
-	check_joinbuf("", "", "");
+	check_joinbuf_2("", "", "");
-	check_joinbuf("", "a", "a");
+	check_joinbuf_2("", "a", "a");
-	check_joinbuf("", "/a", "/a");
+	check_joinbuf_2("", "/a", "/a");
-	check_joinbuf("a", "", "a/");
+	check_joinbuf_2("a", "", "a/");
-	check_joinbuf("a", "/", "a/");
+	check_joinbuf_2("a", "/", "a/");
-	check_joinbuf("a", "b", "a/b");
+	check_joinbuf_2("a", "b", "a/b");
-	check_joinbuf("/", "a", "/a");
+	check_joinbuf_2("/", "a", "/a");
-	check_joinbuf("/", "", "/");
+	check_joinbuf_2("/", "", "/");
-	check_joinbuf("/a", "/b", "/a/b");
+	check_joinbuf_2("/a", "/b", "/a/b");
-	check_joinbuf("/a", "/b/", "/a/b/");
+	check_joinbuf_2("/a", "/b/", "/a/b/");
-	check_joinbuf("/a/", "b/", "/a/b/");
+	check_joinbuf_2("/a/", "b/", "/a/b/");
-	check_joinbuf("/a/", "/b/", "/a/b/");
+	check_joinbuf_2("/a/", "/b/", "/a/b/");
-	check_joinbuf("/abcd", "/defg", "/abcd/defg");
+	check_joinbuf_2("/a/", "//b/", "/a/b/");
-	check_joinbuf("/abcd", "/defg/", "/abcd/defg/");
+	check_joinbuf_2("/abcd", "/defg", "/abcd/defg");
-	check_joinbuf("/abcd/", "defg/", "/abcd/defg/");
+	check_joinbuf_2("/abcd", "/defg/", "/abcd/defg/");
-	check_joinbuf("/abcd/", "/defg/", "/abcd/defg/");
+	check_joinbuf_2("/abcd/", "defg/", "/abcd/defg/");
+	check_joinbuf_2("/abcd/", "/defg/", "/abcd/defg/");
 
-	check_joinbuf_n("", "", "", "", "");
+	check_joinbuf_n_2("", "", "");
-	check_joinbuf_n("", "a", "", "", "a;");
+	check_joinbuf_n_2("", "a", "a");
-	check_joinbuf_n("a", "", "", "", "a;");
+	check_joinbuf_n_2("", "/a", "/a");
-	check_joinbuf_n("", "", "", "a", "a");
+	check_joinbuf_n_2("a", "", "a/");
-	check_joinbuf_n("a", "b", "", ";c;d;", "a;b;c;d;");
+	check_joinbuf_n_2("a", "/", "a/");
-	check_joinbuf_n("a", "b", "", ";c;d", "a;b;c;d");
+	check_joinbuf_n_2("a", "b", "a/b");
-	check_joinbuf_n("abcd", "efgh", "ijkl", "mnop", "abcd;efgh;ijkl;mnop");
+	check_joinbuf_n_2("/", "a", "/a");
-	check_joinbuf_n("abcd;", "efgh;", "ijkl;", "mnop;", "abcd;efgh;ijkl;mnop;");
+	check_joinbuf_n_2("/", "", "/");
-	check_joinbuf_n(";abcd;", ";efgh;", ";ijkl;", ";mnop;", ";abcd;efgh;ijkl;mnop;");
+	check_joinbuf_n_2("/a", "/b", "/a/b");
+	check_joinbuf_n_2("/a", "/b/", "/a/b/");
+	check_joinbuf_n_2("/a/", "b/", "/a/b/");
+	check_joinbuf_n_2("/a/", "/b/", "/a/b/");
+	check_joinbuf_n_2("/abcd", "/defg", "/abcd/defg");
+	check_joinbuf_n_2("/abcd", "/defg/", "/abcd/defg/");
+	check_joinbuf_n_2("/abcd/", "defg/", "/abcd/defg/");
+	check_joinbuf_n_2("/abcd/", "/defg/", "/abcd/defg/");
+
+	check_joinbuf_n_4("", "", "", "", "");
+	check_joinbuf_n_4("", "a", "", "", "a;");
+	check_joinbuf_n_4("a", "", "", "", "a;");
+	check_joinbuf_n_4("", "", "", "a", "a");
+	check_joinbuf_n_4("a", "b", "", ";c;d;", "a;b;c;d;");
+	check_joinbuf_n_4("a", "b", "", ";c;d", "a;b;c;d");
+	check_joinbuf_n_4("abcd", "efgh", "ijkl", "mnop", "abcd;efgh;ijkl;mnop");
+	check_joinbuf_n_4("abcd;", "efgh;", "ijkl;", "mnop;", "abcd;efgh;ijkl;mnop;");
+	check_joinbuf_n_4(";abcd;", ";efgh;", ";ijkl;", ";mnop;", ";abcd;efgh;ijkl;mnop;");
 }