From 7a3bd1e73215f8939f270092e0673d6c0f3f32af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 26 Jan 2014 15:35:17 +0100 Subject: [PATCH] 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. --- include/git2/repository.h | 26 +++++----------- src/repository.c | 41 ++++++------------------- tests/repo/discover.c | 63 ++++++++++++++++++++------------------- tests/repo/message.c | 25 +++++----------- 4 files changed, 55 insertions(+), 100 deletions(-) diff --git a/include/git2/repository.h b/include/git2/repository.h index 9f71d2959..a0453da5c 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 8645357b8..db66e6bd5 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); + assert(start_path); - *repository_path = '\0'; + git_buf_sanitize(out); - if ((error = find_repo(&path, NULL, start_path, flags, ceiling_dirs)) < 0) - return error != GIT_ENOTFOUND ? -1 : error; - - 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; - } - - /* 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); - } + + error = git_futils_readbuffer(out, git_buf_cstr(&path)); git_buf_free(&path); - git_buf_free(&buf); - - if (!error) - error = (int)st.st_size + 1; /* add 1 for NUL byte */ return error; } diff --git a/tests/repo/discover.c b/tests/repo/discover.c index f93ff2462..7904b6496 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 629d40c12..57e8e5f4d 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)); }