Add efficient git_buf join3 API

There are a few places where we need to join three strings to
assemble a path.  This adds a simple join3 function to avoid the
comparatively expensive join_n (which calls strlen on each string
twice).

5 files changed, 95 insertions, 5 deletions

diff --git a/src/buffer.c b/src/buffer.c
index a83ca8792..83960e912 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -467,6 +467,59 @@ int git_buf_join(
 	return 0;
 }
 
+int git_buf_join3(
+	git_buf *buf,
+	char separator,
+	const char *str_a,
+	const char *str_b,
+	const char *str_c)
+{
+	size_t len_a = strlen(str_a), len_b = strlen(str_b), len_c = strlen(str_c);
+	int sep_a = 0, sep_b = 0;
+	char *tgt;
+
+	/* for this function, disallow pointers into the existing buffer */
+	assert(str_a < buf->ptr || str_a >= buf->ptr + buf->size);
+	assert(str_b < buf->ptr || str_b >= buf->ptr + buf->size);
+	assert(str_c < buf->ptr || str_c >= buf->ptr + buf->size);
+
+	if (separator) {
+		if (len_a > 0) {
+			while (*str_b == separator) { str_b++; len_b--; }
+			sep_a = (str_a[len_a - 1] != separator);
+		}
+		if (len_a > 0 || len_b > 0)
+			while (*str_c == separator) { str_c++; len_c--; }
+		if (len_b > 0)
+			sep_b = (str_b[len_b - 1] != separator);
+	}
+
+	if (git_buf_grow(buf, len_a + sep_a + len_b + sep_b + len_c + 1) < 0)
+		return -1;
+
+	tgt = buf->ptr;
+
+	if (len_a) {
+		memcpy(tgt, str_a, len_a);
+		tgt += len_a;
+	}
+	if (sep_a)
+		*tgt++ = separator;
+	if (len_b) {
+		memcpy(tgt, str_b, len_b);
+		tgt += len_b;
+	}
+	if (sep_b)
+		*tgt++ = separator;
+	if (len_c)
+		memcpy(tgt, str_c, len_c);
+
+	buf->size = len_a + sep_a + len_b + sep_b + len_c;
+	buf->ptr[buf->size] = '\0';
+
+	return 0;
+}
+
 void git_buf_rtrim(git_buf *buf)
 {
 	while (buf->size > 0) {
diff --git a/src/buffer.h b/src/buffer.h
index 4c852b3cb..dba594d97 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -98,8 +98,12 @@ void git_buf_truncate(git_buf *buf, size_t len);
 void git_buf_shorten(git_buf *buf, size_t amount);
 void git_buf_rtruncate_at_char(git_buf *path, char separator);
 
+/** General join with separator */
 int git_buf_join_n(git_buf *buf, char separator, int nbuf, ...);
+/** Fast join of two strings - first may legally point into `buf` data */
 int git_buf_join(git_buf *buf, char separator, const char *str_a, const char *str_b);
+/** Fast join of three strings - cannot reference `buf` data */
+int git_buf_join3(git_buf *buf, char separator, const char *str_a, const char *str_b, const char *str_c);
 
 /**
  * Join two strings as paths, inserting a slash between as needed.
diff --git a/src/refdb_fs.c b/src/refdb_fs.c
index 9120a3e87..2550b7e26 100644
--- a/src/refdb_fs.c
+++ b/src/refdb_fs.c
@@ -1432,7 +1432,7 @@ static int create_new_reflog_file(const char *filepath)
 
 GIT_INLINE(int) retrieve_reflog_path(git_buf *path, git_repository *repo, const char *name)
 {
-	return git_buf_join_n(path, '/', 3, repo->path_repository, GIT_REFLOG_DIR, name);
+	return git_buf_join3(path, '/', repo->path_repository, GIT_REFLOG_DIR, name);
 }
 
 static int refdb_reflog_fs__ensure_log(git_refdb_backend *_backend, const char *name)
diff --git a/src/submodule.c b/src/submodule.c
index fdcc2251a..a0ce5317f 100644
--- a/src/submodule.c
+++ b/src/submodule.c
@@ -168,10 +168,11 @@ int git_submodule_lookup(
 		if (git_repository_workdir(repo)) {
 			git_buf path = GIT_BUF_INIT;
 
-			if (git_buf_joinpath(&path, git_repository_workdir(repo), name) < 0)
+			if (git_buf_join3(&path,
+					'/', git_repository_workdir(repo), name, DOT_GIT) < 0)
 				return -1;
 
-			if (git_path_contains(&path, DOT_GIT))
+			if (git_path_exists(path.ptr))
 				error = GIT_EEXISTS;
 
 			git_buf_free(&path);
@@ -328,8 +329,8 @@ int git_submodule_add_setup(
 	else if (use_gitlink) {
 		git_buf repodir = GIT_BUF_INIT;
 
-		error = git_buf_join_n(
-			&repodir, '/', 3, git_repository_path(repo), "modules", path);
+		error = git_buf_join3(
+			&repodir, '/', git_repository_path(repo), "modules", path);
 		if (error < 0)
 			goto cleanup;
 
diff --git a/tests/core/buffer.c b/tests/core/buffer.c
index 0e7dd3d70..eb1d95a95 100644
--- a/tests/core/buffer.c
+++ b/tests/core/buffer.c
@@ -599,6 +599,38 @@ void test_core_buffer__10(void)
 	git_buf_free(&a);
 }
 
+void test_core_buffer__join3(void)
+{
+	git_buf a = GIT_BUF_INIT;
+
+	cl_git_pass(git_buf_join3(&a, '/', "test", "string", "join"));
+	cl_assert_equal_s("test/string/join", a.ptr);
+	cl_git_pass(git_buf_join3(&a, '/', "test/", "string", "join"));
+	cl_assert_equal_s("test/string/join", a.ptr);
+	cl_git_pass(git_buf_join3(&a, '/', "test/", "/string", "join"));
+	cl_assert_equal_s("test/string/join", a.ptr);
+	cl_git_pass(git_buf_join3(&a, '/', "test/", "/string/", "join"));
+	cl_assert_equal_s("test/string/join", a.ptr);
+	cl_git_pass(git_buf_join3(&a, '/', "test/", "/string/", "/join"));
+	cl_assert_equal_s("test/string/join", a.ptr);
+
+	cl_git_pass(git_buf_join3(&a, '/', "", "string", "join"));
+	cl_assert_equal_s("string/join", a.ptr);
+	cl_git_pass(git_buf_join3(&a, '/', "", "string/", "join"));
+	cl_assert_equal_s("string/join", a.ptr);
+	cl_git_pass(git_buf_join3(&a, '/', "", "string/", "/join"));
+	cl_assert_equal_s("string/join", a.ptr);
+
+	cl_git_pass(git_buf_join3(&a, '/', "string", "", "join"));
+	cl_assert_equal_s("string/join", a.ptr);
+	cl_git_pass(git_buf_join3(&a, '/', "string/", "", "join"));
+	cl_assert_equal_s("string/join", a.ptr);
+	cl_git_pass(git_buf_join3(&a, '/', "string/", "", "/join"));
+	cl_assert_equal_s("string/join", a.ptr);
+
+	git_buf_free(&a);
+}
+
 void test_core_buffer__11(void)
 {
 	git_buf a = GIT_BUF_INIT;