Commit 7a3bd1e73215f8939f270092e0673d6c0f3f32af

Carlos Martín Nieto 2014-01-26T15:35:17

repository: move to use a git_buf for outputting strings Since we now export that type, we can avoid making the user guess a size.

diff --git a/include/git2/repository.h b/include/git2/repository.h
index 9f71d29..a0453da 100644
--- a/include/git2/repository.h
+++ b/include/git2/repository.h
@@ -10,6 +10,7 @@
 #include "common.h"
 #include "types.h"
 #include "oid.h"
+#include "buffer.h"
 
 /**
  * @file git2/repository.h
@@ -58,10 +59,8 @@ GIT_EXTERN(int) git_repository_wrap_odb(git_repository **out, git_odb *odb);
  * The method will automatically detect if the repository is bare
  * (if there is a repository).
  *
- * @param path_out The user allocated buffer which will
- * contain the found path.
- *
- * @param path_size repository_path size
+ * @param out A pointer to a user-allocated git_buf which will contain
+ * the found path.
  *
  * @param start_path The base path where the lookup starts.
  *
@@ -77,8 +76,7 @@ GIT_EXTERN(int) git_repository_wrap_odb(git_repository **out, git_odb *odb);
  * @return 0 or an error code
  */
 GIT_EXTERN(int) git_repository_discover(
-		char *path_out,
-		size_t path_size,
+		git_buf *out,
 		const char *start_path,
 		int across_fs,
 		const char *ceiling_dirs);
@@ -464,21 +462,11 @@ GIT_EXTERN(int) git_repository_index(git_index **out, git_repository *repo);
  * Use this function to get the contents of this file. Don't forget to
  * remove the file after you create the commit.
  *
- * If the repository message exists and there are no errors reading it, this
- * returns the bytes needed to store the message in memory (i.e. message
- * file size plus one terminating NUL byte).  That value is returned even if
- * `out` is NULL or `len` is shorter than the necessary size.
- *
- * The `out` buffer will *always* be NUL terminated, even if truncation
- * occurs.
- *
- * @param out Buffer to write data into or NULL to just read required size
- * @param len Length of `out` buffer in bytes
+ * @param out git_buf to write data into
  * @param repo Repository to read prepared message from
- * @return GIT_ENOTFOUND if no message exists, other value < 0 for other
- *         errors, or total bytes in message (may be > `len`) on success
+ * @return 0, GIT_ENOTFOUND if no message exists or an error code
  */
-GIT_EXTERN(int) git_repository_message(char *out, size_t len, git_repository *repo);
+GIT_EXTERN(int) git_repository_message(git_buf *out, git_repository *repo);
 
 /**
  * Remove git's prepared message.
diff --git a/src/repository.c b/src/repository.c
index 8645357..db66e6b 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -495,34 +495,18 @@ int git_repository_wrap_odb(git_repository **repo_out, git_odb *odb)
 }
 
 int git_repository_discover(
-	char *repository_path,
-	size_t size,
+	git_buf *out,
 	const char *start_path,
 	int across_fs,
 	const char *ceiling_dirs)
 {
-	git_buf path = GIT_BUF_INIT;
 	uint32_t flags = across_fs ? GIT_REPOSITORY_OPEN_CROSS_FS : 0;
-	int error;
-
-	assert(start_path && repository_path && size > 0);
-
-	*repository_path = '\0';
 
-	if ((error = find_repo(&path, NULL, start_path, flags, ceiling_dirs)) < 0)
-		return error != GIT_ENOTFOUND ? -1 : error;
+	assert(start_path);
 
-	if (size < (size_t)(path.size + 1)) {
-		giterr_set(GITERR_REPOSITORY,
-			"The given buffer is too small to store the discovered path");
-		git_buf_free(&path);
-		return -1;
-	}
+	git_buf_sanitize(out);
 
-	/* success: we discovered a repository */
-	git_buf_copy_cstr(repository_path, size, &path);
-	git_buf_free(&path);
-	return 0;
+	return find_repo(out, NULL, start_path, flags, ceiling_dirs);
 }
 
 static int load_config(
@@ -1732,14 +1716,13 @@ cleanup:
 	return error;
 }
 
-int git_repository_message(char *buffer, size_t len, git_repository *repo)
+int git_repository_message(git_buf *out,  git_repository *repo)
 {
-	git_buf buf = GIT_BUF_INIT, path = GIT_BUF_INIT;
+	git_buf path = GIT_BUF_INIT;
 	struct stat st;
 	int error;
 
-	if (buffer != NULL)
-		*buffer = '\0';
+	git_buf_sanitize(out);
 
 	if (git_buf_joinpath(&path, repo->path_repository, GIT_MERGE_MSG_FILE) < 0)
 		return -1;
@@ -1749,16 +1732,10 @@ int git_repository_message(char *buffer, size_t len, git_repository *repo)
 			error = GIT_ENOTFOUND;
 		giterr_set(GITERR_OS, "Could not access message file");
 	}
-	else if (buffer != NULL) {
-		error = git_futils_readbuffer(&buf, git_buf_cstr(&path));
-		git_buf_copy_cstr(buffer, len, &buf);
-	}
 
-	git_buf_free(&path);
-	git_buf_free(&buf);
+	error = git_futils_readbuffer(out, git_buf_cstr(&path));
 
-	if (!error)
-		error = (int)st.st_size + 1; /* add 1 for NUL byte */
+	git_buf_free(&path);
 
 	return error;
 }
diff --git a/tests/repo/discover.c b/tests/repo/discover.c
index f93ff24..7904b64 100644
--- a/tests/repo/discover.c
+++ b/tests/repo/discover.c
@@ -25,12 +25,13 @@
 
 static void ensure_repository_discover(const char *start_path,
                                        const char *ceiling_dirs,
-                                       const char *expected_path)
+				       git_buf *expected_path)
 {
-	char found_path[GIT_PATH_MAX];
-	cl_git_pass(git_repository_discover(found_path, sizeof(found_path), start_path, 0, ceiling_dirs));
+	git_buf found_path = GIT_BUF_INIT;
+	cl_git_pass(git_repository_discover(&found_path, start_path, 0, ceiling_dirs));
 	//across_fs is always 0 as we can't automate the filesystem change tests
-	cl_assert_equal_s(found_path, expected_path);
+	cl_assert_equal_s(found_path.ptr, expected_path->ptr);
+	git_buf_free(&found_path);
 }
 
 static void write_file(const char *path, const char *content)
@@ -69,42 +70,40 @@ static void append_ceiling_dir(git_buf *ceiling_dirs, const char *path)
 
 void test_repo_discover__0(void)
 {
-   // test discover
+	// test discover
 	git_repository *repo;
-	git_buf ceiling_dirs_buf = GIT_BUF_INIT;
+	git_buf ceiling_dirs_buf = GIT_BUF_INIT, repository_path = GIT_BUF_INIT,
+		sub_repository_path = GIT_BUF_INIT, found_path = GIT_BUF_INIT;
 	const char *ceiling_dirs;
-	char repository_path[GIT_PATH_MAX];
-	char sub_repository_path[GIT_PATH_MAX];
-	char found_path[GIT_PATH_MAX];
 	const mode_t mode = 0777;
 
 	git_futils_mkdir_r(DISCOVER_FOLDER, NULL, mode);
 	append_ceiling_dir(&ceiling_dirs_buf, TEMP_REPO_FOLDER);
 	ceiling_dirs = git_buf_cstr(&ceiling_dirs_buf);
 
-	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(repository_path, sizeof(repository_path), DISCOVER_FOLDER, 0, ceiling_dirs));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&repository_path, DISCOVER_FOLDER, 0, ceiling_dirs));
 
 	cl_git_pass(git_repository_init(&repo, DISCOVER_FOLDER, 1));
-	cl_git_pass(git_repository_discover(repository_path, sizeof(repository_path), DISCOVER_FOLDER, 0, ceiling_dirs));
+	cl_git_pass(git_repository_discover(&repository_path, DISCOVER_FOLDER, 0, ceiling_dirs));
 	git_repository_free(repo);
 
 	cl_git_pass(git_repository_init(&repo, SUB_REPOSITORY_FOLDER, 0));
 	cl_git_pass(git_futils_mkdir_r(SUB_REPOSITORY_FOLDER_SUB_SUB_SUB, NULL, mode));
-	cl_git_pass(git_repository_discover(sub_repository_path, sizeof(sub_repository_path), SUB_REPOSITORY_FOLDER, 0, ceiling_dirs));
+	cl_git_pass(git_repository_discover(&sub_repository_path, SUB_REPOSITORY_FOLDER, 0, ceiling_dirs));
 
 	cl_git_pass(git_futils_mkdir_r(SUB_REPOSITORY_FOLDER_SUB_SUB_SUB, NULL, mode));
-	ensure_repository_discover(SUB_REPOSITORY_FOLDER_SUB, ceiling_dirs, sub_repository_path);
-	ensure_repository_discover(SUB_REPOSITORY_FOLDER_SUB_SUB, ceiling_dirs, sub_repository_path);
-	ensure_repository_discover(SUB_REPOSITORY_FOLDER_SUB_SUB_SUB, ceiling_dirs, sub_repository_path);
+	ensure_repository_discover(SUB_REPOSITORY_FOLDER_SUB, ceiling_dirs, &sub_repository_path);
+	ensure_repository_discover(SUB_REPOSITORY_FOLDER_SUB_SUB, ceiling_dirs, &sub_repository_path);
+	ensure_repository_discover(SUB_REPOSITORY_FOLDER_SUB_SUB_SUB, ceiling_dirs, &sub_repository_path);
 
 	cl_git_pass(git_futils_mkdir_r(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB_SUB, NULL, mode));
 	write_file(REPOSITORY_ALTERNATE_FOLDER "/" DOT_GIT, "gitdir: ../" SUB_REPOSITORY_FOLDER_NAME "/" DOT_GIT);
 	write_file(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB "/" DOT_GIT, "gitdir: ../../../" SUB_REPOSITORY_FOLDER_NAME "/" DOT_GIT);
 	write_file(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB_SUB "/" DOT_GIT, "gitdir: ../../../../");
-	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER, ceiling_dirs, sub_repository_path);
-	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB, ceiling_dirs, sub_repository_path);
-	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB, ceiling_dirs, sub_repository_path);
-	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB_SUB, ceiling_dirs, repository_path);
+	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER, ceiling_dirs, &sub_repository_path);
+	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB, ceiling_dirs, &sub_repository_path);
+	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB, ceiling_dirs, &sub_repository_path);
+	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB_SUB, ceiling_dirs, &repository_path);
 
 	cl_git_pass(git_futils_mkdir_r(ALTERNATE_MALFORMED_FOLDER1, NULL, mode));
 	write_file(ALTERNATE_MALFORMED_FOLDER1 "/" DOT_GIT, "Anything but not gitdir:");
@@ -114,29 +113,31 @@ void test_repo_discover__0(void)
 	write_file(ALTERNATE_MALFORMED_FOLDER3 "/" DOT_GIT, "gitdir: \n\n\n");
 	cl_git_pass(git_futils_mkdir_r(ALTERNATE_NOT_FOUND_FOLDER, NULL, mode));
 	write_file(ALTERNATE_NOT_FOUND_FOLDER "/" DOT_GIT, "gitdir: a_repository_that_surely_does_not_exist");
-	cl_git_fail(git_repository_discover(found_path, sizeof(found_path), ALTERNATE_MALFORMED_FOLDER1, 0, ceiling_dirs));
-	cl_git_fail(git_repository_discover(found_path, sizeof(found_path), ALTERNATE_MALFORMED_FOLDER2, 0, ceiling_dirs));
-	cl_git_fail(git_repository_discover(found_path, sizeof(found_path), ALTERNATE_MALFORMED_FOLDER3, 0, ceiling_dirs));
-	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(found_path, sizeof(found_path), ALTERNATE_NOT_FOUND_FOLDER, 0, ceiling_dirs));
+	cl_git_fail(git_repository_discover(&found_path, ALTERNATE_MALFORMED_FOLDER1, 0, ceiling_dirs));
+	cl_git_fail(git_repository_discover(&found_path, ALTERNATE_MALFORMED_FOLDER2, 0, ceiling_dirs));
+	cl_git_fail(git_repository_discover(&found_path, ALTERNATE_MALFORMED_FOLDER3, 0, ceiling_dirs));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, ALTERNATE_NOT_FOUND_FOLDER, 0, ceiling_dirs));
 
 	append_ceiling_dir(&ceiling_dirs_buf, SUB_REPOSITORY_FOLDER);
 	ceiling_dirs = git_buf_cstr(&ceiling_dirs_buf);
 
 	//this must pass as ceiling_directories cannot predent the current
 	//working directory to be checked
-	cl_git_pass(git_repository_discover(found_path, sizeof(found_path), SUB_REPOSITORY_FOLDER, 0, ceiling_dirs));
-	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(found_path, sizeof(found_path), SUB_REPOSITORY_FOLDER_SUB, 0, ceiling_dirs));
-	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(found_path, sizeof(found_path), SUB_REPOSITORY_FOLDER_SUB_SUB, 0, ceiling_dirs));
-	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(found_path, sizeof(found_path), SUB_REPOSITORY_FOLDER_SUB_SUB_SUB, 0, ceiling_dirs));
+	cl_git_pass(git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER, 0, ceiling_dirs));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER_SUB, 0, ceiling_dirs));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER_SUB_SUB, 0, ceiling_dirs));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_discover(&found_path, SUB_REPOSITORY_FOLDER_SUB_SUB_SUB, 0, ceiling_dirs));
 
 	//.gitfile redirection should not be affected by ceiling directories
-	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER, ceiling_dirs, sub_repository_path);
-	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB, ceiling_dirs, sub_repository_path);
-	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB, ceiling_dirs, sub_repository_path);
-	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB_SUB, ceiling_dirs, repository_path);
+	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER, ceiling_dirs, &sub_repository_path);
+	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB, ceiling_dirs, &sub_repository_path);
+	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB, ceiling_dirs, &sub_repository_path);
+	ensure_repository_discover(REPOSITORY_ALTERNATE_FOLDER_SUB_SUB_SUB, ceiling_dirs, &repository_path);
 
 	cl_git_pass(git_futils_rmdir_r(TEMP_REPO_FOLDER, NULL, GIT_RMDIR_REMOVE_FILES));
 	git_repository_free(repo);
 	git_buf_free(&ceiling_dirs_buf);
+	git_buf_free(&repository_path);
+	git_buf_free(&sub_repository_path);
 }
 
diff --git a/tests/repo/message.c b/tests/repo/message.c
index 629d40c..57e8e5f 100644
--- a/tests/repo/message.c
+++ b/tests/repo/message.c
@@ -5,48 +5,37 @@
 
 static git_repository *_repo;
 static git_buf _path;
-static char *_actual;
+static git_buf _actual;
 
 void test_repo_message__initialize(void)
 {
         _repo = cl_git_sandbox_init("testrepo.git");
+	git_buf_init(&_actual, 0);
 }
 
 void test_repo_message__cleanup(void)
 {
         cl_git_sandbox_cleanup();
 	git_buf_free(&_path);
-	git__free(_actual);
-	_actual = NULL;
+	git_buf_free(&_actual);
 }
 
 void test_repo_message__none(void)
 {
-	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_message(NULL, 0, _repo));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_message(&_actual, _repo));
 }
 
 void test_repo_message__message(void)
 {
 	const char expected[] = "Test\n\nThis is a test of the emergency broadcast system\n";
-	ssize_t len;
 
 	cl_git_pass(git_buf_joinpath(&_path, git_repository_path(_repo), "MERGE_MSG"));
 	cl_git_mkfile(git_buf_cstr(&_path), expected);
 
-	len = git_repository_message(NULL, 0, _repo);
-	cl_assert(len > 0);
-
-	_actual = git__malloc(len + 1);
-	cl_assert(_actual != NULL);
-
-	/* Test non truncation */
-	cl_assert(git_repository_message(_actual, len, _repo) > 0);
+	cl_git_pass(git_repository_message(&_actual, _repo));
 	cl_assert_equal_s(expected, _actual);
-
-	/* Test truncation and that trailing NUL is inserted */
-	cl_assert(git_repository_message(_actual, 6, _repo) > 0);
-	cl_assert_equal_s("Test\n", _actual);
+	git_buf_free(&_actual);
 
 	cl_git_pass(p_unlink(git_buf_cstr(&_path)));
-	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_message(NULL, 0, _repo));
+	cl_assert_equal_i(GIT_ENOTFOUND, git_repository_message(&_actual, _repo));
 }