From b25d87c9cdef0bc09007c756d7e52f15eb4622d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 26 Jan 2014 16:03:37 +0100 Subject: [PATCH] branch: move to git_buf when outputting newly-allocated strings Internally we already did everything with git_bufs, so this is just exposing those functions with public names. --- include/git2/branch.h | 31 +++------- src/branch.c | 98 ++++++++---------------------- tests/clone/empty.c | 12 ++-- tests/refs/branches/remote.c | 39 ++++-------- tests/refs/branches/upstreamname.c | 10 +-- 5 files changed, 56 insertions(+), 134 deletions(-) diff --git a/include/git2/branch.h b/include/git2/branch.h index 44d6fd9c3..851de290a 100644 --- a/include/git2/branch.h +++ b/include/git2/branch.h @@ -200,25 +200,20 @@ GIT_EXTERN(int) git_branch_set_upstream(git_reference *branch, const char *upstr * Return the name of the reference supporting the remote tracking branch, * given the name of a local branch reference. * - * @param tracking_branch_name_out The user-allocated buffer which will be - * filled with the name of the reference. Pass NULL if you just want to - * get the needed size of the name of the reference as the output value. - * - * @param buffer_size Size of the `out` buffer in bytes. + * @param out Pointer to the user-allocated git_buf which will be + * filled with the name of the reference. * * @param repo the repository where the branches live * - * @param canonical_branch_name name of the local branch. + * @param refname reference name of the local branch. * - * @return number of characters in the reference name - * including the trailing NUL byte; GIT_ENOTFOUND when no remote tracking - * reference exists, otherwise an error code. + * @return 0, GIT_ENOTFOUND when no remote tracking reference exists, + * otherwise an error code. */ GIT_EXTERN(int) git_branch_upstream_name( - char *tracking_branch_name_out, - size_t buffer_size, + git_buf *out, git_repository *repo, - const char *canonical_branch_name); + const char *refname); /** * Determine if the current local branch is pointed at by HEAD. @@ -234,25 +229,19 @@ GIT_EXTERN(int) git_branch_is_head( /** * Return the name of remote that the remote tracking branch belongs to. * - * @param remote_name_out The user-allocated buffer which will be - * filled with the name of the remote. Pass NULL if you just want to - * get the needed size of the name of the remote as the output value. - * - * @param buffer_size Size of the `out` buffer in bytes. + * @param out Pointer to the user-allocated git_buf which will be filled iwth the name of the remote. * * @param repo The repository where the branch lives. * * @param canonical_branch_name name of the remote tracking branch. * - * @return Number of characters in the reference name - * including the trailing NUL byte; GIT_ENOTFOUND + * @return 0, GIT_ENOTFOUND * when no remote matching remote was found, * GIT_EAMBIGUOUS when the branch maps to several remotes, * otherwise an error code. */ GIT_EXTERN(int) git_branch_remote_name( - char *remote_name_out, - size_t buffer_size, + git_buf *out, git_repository *repo, const char *canonical_branch_name); diff --git a/src/branch.c b/src/branch.c index 3b9aa0d20..a702c661e 100644 --- a/src/branch.c +++ b/src/branch.c @@ -286,10 +286,10 @@ static int retrieve_upstream_configuration( return error; } -int git_branch_upstream__name( - git_buf *tracking_name, +int git_branch_upstream_name( + git_buf *out, git_repository *repo, - const char *canonical_branch_name) + const char *refname) { const char *remote_name, *merge_name; git_buf buf = GIT_BUF_INIT; @@ -297,22 +297,24 @@ int git_branch_upstream__name( git_remote *remote = NULL; const git_refspec *refspec; - assert(tracking_name && canonical_branch_name); + assert(out && refname); - if (!git_reference__is_branch(canonical_branch_name)) - return not_a_local_branch(canonical_branch_name); + git_buf_sanitize(out); + + if (!git_reference__is_branch(refname)) + return not_a_local_branch(refname); if ((error = retrieve_upstream_configuration( - &remote_name, repo, canonical_branch_name, "branch.%s.remote")) < 0) + &remote_name, repo, refname, "branch.%s.remote")) < 0) goto cleanup; if ((error = retrieve_upstream_configuration( - &merge_name, repo, canonical_branch_name, "branch.%s.merge")) < 0) + &merge_name, repo, refname, "branch.%s.merge")) < 0) goto cleanup; if (!*remote_name || !*merge_name) { giterr_set(GITERR_REFERENCE, - "branch '%s' does not have an upstream", canonical_branch_name); + "branch '%s' does not have an upstream", refname); error = GIT_ENOTFOUND; goto cleanup; } @@ -333,7 +335,7 @@ int git_branch_upstream__name( if (git_buf_sets(&buf, merge_name) < 0) goto cleanup; - error = git_buf_set(tracking_name, git_buf_cstr(&buf), git_buf_len(&buf)); + error = git_buf_set(out, git_buf_cstr(&buf), git_buf_len(&buf)); cleanup: git_remote_free(remote); @@ -341,7 +343,7 @@ cleanup: return error; } -static int remote_name(git_buf *buf, git_repository *repo, const char *canonical_branch_name) +int git_branch_remote_name(git_buf *buf, git_repository *repo, const char *refname) { git_strarray remote_list = {0}; size_t i; @@ -350,12 +352,14 @@ static int remote_name(git_buf *buf, git_repository *repo, const char *canonical int error = 0; char *remote_name = NULL; - assert(buf && repo && canonical_branch_name); + assert(buf && repo && refname); + + git_buf_sanitize(buf); /* Verify that this is a remote branch */ - if (!git_reference__is_remote(canonical_branch_name)) { + if (!git_reference__is_remote(refname)) { giterr_set(GITERR_INVALID, "Reference '%s' is not a remote branch.", - canonical_branch_name); + refname); error = GIT_ERROR; goto cleanup; } @@ -369,7 +373,7 @@ static int remote_name(git_buf *buf, git_repository *repo, const char *canonical if ((error = git_remote_load(&remote, repo, remote_list.strings[i])) < 0) continue; - fetchspec = git_remote__matching_dst_refspec(remote, canonical_branch_name); + fetchspec = git_remote__matching_dst_refspec(remote, refname); if (fetchspec) { /* If we have not already set out yet, then set * it to the matching remote name. Otherwise @@ -381,7 +385,7 @@ static int remote_name(git_buf *buf, git_repository *repo, const char *canonical git_remote_free(remote); giterr_set(GITERR_REFERENCE, - "Reference '%s' is ambiguous", canonical_branch_name); + "Reference '%s' is ambiguous", refname); error = GIT_EAMBIGUOUS; goto cleanup; } @@ -395,68 +399,18 @@ static int remote_name(git_buf *buf, git_repository *repo, const char *canonical error = git_buf_puts(buf, remote_name); } else { giterr_set(GITERR_REFERENCE, - "Could not determine remote for '%s'", canonical_branch_name); + "Could not determine remote for '%s'", refname); error = GIT_ENOTFOUND; } cleanup: + if (error < 0) + git_buf_free(buf); + git_strarray_free(&remote_list); return error; } -int git_branch_remote_name(char *buffer, size_t buffer_len, git_repository *repo, const char *refname) -{ - int ret; - git_buf buf = GIT_BUF_INIT; - - if ((ret = remote_name(&buf, repo, refname)) < 0) - return ret; - - if (buffer) - git_buf_copy_cstr(buffer, buffer_len, &buf); - - ret = (int)git_buf_len(&buf) + 1; - git_buf_free(&buf); - - return ret; -} - -int git_branch_upstream_name( - char *tracking_branch_name_out, - size_t buffer_size, - git_repository *repo, - const char *canonical_branch_name) -{ - git_buf buf = GIT_BUF_INIT; - int error; - - assert(canonical_branch_name); - - if (tracking_branch_name_out && buffer_size) - *tracking_branch_name_out = '\0'; - - if ((error = git_branch_upstream__name( - &buf, repo, canonical_branch_name)) < 0) - goto cleanup; - - if (tracking_branch_name_out && buf.size + 1 > buffer_size) { /* +1 for NUL byte */ - giterr_set( - GITERR_INVALID, - "Buffer too short to hold the tracked reference name."); - error = -1; - goto cleanup; - } - - if (tracking_branch_name_out) - git_buf_copy_cstr(tracking_branch_name_out, buffer_size, &buf); - - error = (int)buf.size + 1; - -cleanup: - git_buf_free(&buf); - return (int)error; -} - int git_branch_upstream( git_reference **tracking_out, git_reference *branch) @@ -464,7 +418,7 @@ int git_branch_upstream( int error; git_buf tracking_name = GIT_BUF_INIT; - if ((error = git_branch_upstream__name(&tracking_name, + if ((error = git_branch_upstream_name(&tracking_name, git_reference_owner(branch), git_reference_name(branch))) < 0) return error; @@ -547,7 +501,7 @@ int git_branch_set_upstream(git_reference *branch, const char *upstream_name) if (local) git_buf_puts(&value, "."); else - remote_name(&value, repo, git_reference_name(upstream)); + git_branch_remote_name(&value, repo, git_reference_name(upstream)); if (git_buf_printf(&key, "branch.%s.remote", shortname) < 0) goto on_error; diff --git a/tests/clone/empty.c b/tests/clone/empty.c index 6d19244cc..78aef7dae 100644 --- a/tests/clone/empty.c +++ b/tests/clone/empty.c @@ -38,7 +38,7 @@ void test_clone_empty__can_clone_an_empty_local_repo_barely(void) char *local_name = "refs/heads/master"; const char *expected_tracked_branch_name = "refs/remotes/origin/master"; const char *expected_remote_name = "origin"; - char buffer[1024]; + git_buf buf = GIT_BUF_INIT; git_reference *ref; cl_set_cleanup(&cleanup_repository, "./empty"); @@ -50,16 +50,14 @@ void test_clone_empty__can_clone_an_empty_local_repo_barely(void) cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&ref, g_repo_cloned, local_name)); /* ...one can still retrieve the name of the remote tracking reference */ - cl_assert_equal_i((int)strlen(expected_tracked_branch_name) + 1, - git_branch_upstream_name(buffer, 1024, g_repo_cloned, local_name)); + cl_git_pass(git_branch_upstream_name(&buf, g_repo_cloned, local_name)); - cl_assert_equal_s(expected_tracked_branch_name, buffer); + cl_assert_equal_s(expected_tracked_branch_name, buf.ptr); /* ...and the name of the remote... */ - cl_assert_equal_i((int)strlen(expected_remote_name) + 1, - git_branch_remote_name(buffer, 1024, g_repo_cloned, expected_tracked_branch_name)); + cl_git_pass(git_branch_remote_name(&buf, g_repo_cloned, expected_tracked_branch_name)); - cl_assert_equal_s(expected_remote_name, buffer); + cl_assert_equal_s(expected_remote_name, buf.ptr); /* ...even when the remote HEAD is unborn as well */ cl_assert_equal_i(GIT_ENOTFOUND, git_reference_lookup(&ref, g_repo_cloned, diff --git a/tests/refs/branches/remote.c b/tests/refs/branches/remote.c index c110adb33..bac088454 100644 --- a/tests/refs/branches/remote.c +++ b/tests/refs/branches/remote.c @@ -21,53 +21,40 @@ void test_refs_branches_remote__cleanup(void) void test_refs_branches_remote__can_get_remote_for_branch(void) { - char remotename[1024] = {0}; + git_buf remotename = {0}; - cl_assert_equal_i(expected_remote_name_length, - git_branch_remote_name(NULL, 0, g_repo, remote_tracking_branch_name)); + cl_git_pass(git_branch_remote_name(&remotename, g_repo, remote_tracking_branch_name)); - cl_assert_equal_i(expected_remote_name_length, - git_branch_remote_name(remotename, expected_remote_name_length, g_repo, - remote_tracking_branch_name)); - - cl_assert_equal_s("test", remotename); -} - -void test_refs_branches_remote__insufficient_buffer_returns_error(void) -{ - char remotename[1024] = {0}; - - cl_assert_equal_i(expected_remote_name_length, - git_branch_remote_name(NULL, 0, g_repo, remote_tracking_branch_name)); - - cl_git_fail_with(git_branch_remote_name(remotename, - expected_remote_name_length - 1, g_repo, remote_tracking_branch_name), - expected_remote_name_length); + cl_assert_equal_s("test", remotename.ptr); + git_buf_free(&remotename); } void test_refs_branches_remote__no_matching_remote_returns_error(void) { const char *unknown = "refs/remotes/nonexistent/master"; + git_buf buf; giterr_clear(); - cl_git_fail_with(git_branch_remote_name( - NULL, 0, g_repo, unknown), GIT_ENOTFOUND); + memset(&buf, 0, sizeof(git_buf)); + cl_git_fail_with(git_branch_remote_name(&buf, g_repo, unknown), GIT_ENOTFOUND); cl_assert(giterr_last() != NULL); } void test_refs_branches_remote__local_remote_returns_error(void) { const char *local = "refs/heads/master"; + git_buf buf; giterr_clear(); - cl_git_fail_with(git_branch_remote_name( - NULL, 0, g_repo, local), GIT_ERROR); + memset(&buf, 0, sizeof(git_buf)); + cl_git_fail_with(git_branch_remote_name(&buf, g_repo, local), GIT_ERROR); cl_assert(giterr_last() != NULL); } void test_refs_branches_remote__ambiguous_remote_returns_error(void) { git_remote *remote; + git_buf buf; /* Create the remote */ cl_git_pass(git_remote_create(&remote, g_repo, "addtest", "http://github.com/libgit2/libgit2")); @@ -80,7 +67,7 @@ void test_refs_branches_remote__ambiguous_remote_returns_error(void) git_remote_free(remote); giterr_clear(); - cl_git_fail_with(git_branch_remote_name(NULL, 0, g_repo, - remote_tracking_branch_name), GIT_EAMBIGUOUS); + memset(&buf, 0, sizeof(git_buf)); + cl_git_fail_with(git_branch_remote_name(&buf, g_repo, remote_tracking_branch_name), GIT_EAMBIGUOUS); cl_assert(giterr_last() != NULL); } diff --git a/tests/refs/branches/upstreamname.c b/tests/refs/branches/upstreamname.c index f05607d44..d30002e08 100644 --- a/tests/refs/branches/upstreamname.c +++ b/tests/refs/branches/upstreamname.c @@ -21,7 +21,7 @@ void test_refs_branches_upstreamname__cleanup(void) void test_refs_branches_upstreamname__can_retrieve_the_remote_tracking_reference_name_of_a_local_branch(void) { - cl_git_pass(git_branch_upstream__name( + cl_git_pass(git_branch_upstream_name( &upstream_name, repo, "refs/heads/master")); cl_assert_equal_s("refs/remotes/test/master", git_buf_cstr(&upstream_name)); @@ -29,14 +29,8 @@ void test_refs_branches_upstreamname__can_retrieve_the_remote_tracking_reference void test_refs_branches_upstreamname__can_retrieve_the_local_upstream_reference_name_of_a_local_branch(void) { - cl_git_pass(git_branch_upstream__name( + cl_git_pass(git_branch_upstream_name( &upstream_name, repo, "refs/heads/track-local")); cl_assert_equal_s("refs/heads/master", git_buf_cstr(&upstream_name)); } - -void test_refs_branches_upstreamname__can_return_the_size_of_thelocal_upstream_reference_name_of_a_local_branch(void) -{ - cl_assert_equal_i((int)strlen("refs/heads/master") + 1, - git_branch_upstream_name(NULL, 0, repo, "refs/heads/track-local")); -}