Commit b52bb4d4b2548d0c7f00de0eab5073cc8fe9e899

Edward Thomson 2020-10-11T13:20:52

refs: use git_reference_name_is_valid

diff --git a/src/refs.c b/src/refs.c
index da12d02..90b9b06 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -239,7 +239,7 @@ int git_reference_lookup_resolved(
 
 int git_reference_dwim(git_reference **out, git_repository *repo, const char *refname)
 {
-	int error = 0, i;
+	int error = 0, i, valid;
 	bool fallbackmode = true, foundvalid = false;
 	git_reference *ref;
 	git_buf refnamebuf = GIT_BUF_INIT, name = GIT_BUF_INIT;
@@ -265,10 +265,11 @@ int git_reference_dwim(git_reference **out, git_repository *repo, const char *re
 
 		git_buf_clear(&refnamebuf);
 
-		if ((error = git_buf_printf(&refnamebuf, formatters[i], git_buf_cstr(&name))) < 0)
+		if ((error = git_buf_printf(&refnamebuf, formatters[i], git_buf_cstr(&name))) < 0 ||
+		    (error = git_reference_name_is_valid(&valid, git_buf_cstr(&refnamebuf))) < 0)
 			goto cleanup;
 
-		if (!git_reference_is_valid_name(git_buf_cstr(&refnamebuf))) {
+		if (!valid) {
 			error = GIT_EINVALIDSPEC;
 			continue;
 		}
diff --git a/src/remote.c b/src/remote.c
index 51e99dc..b69526c 100644
--- a/src/remote.c
+++ b/src/remote.c
@@ -1362,7 +1362,7 @@ static int update_tips_for_spec(
 		git_vector *refs,
 		const char *log_message)
 {
-	int error = 0, autotag;
+	int error = 0, autotag, valid;
 	unsigned int i = 0;
 	git_buf refname = GIT_BUF_INIT;
 	git_oid old;
@@ -1390,7 +1390,10 @@ static int update_tips_for_spec(
 		git_buf_clear(&refname);
 
 		/* Ignore malformed ref names (which also saves us from tag^{} */
-		if (!git_reference_is_valid_name(head->name))
+		if (git_reference_name_is_valid(&valid, head->name) < 0)
+			goto on_error;
+
+		if (!valid)
 			continue;
 
 		/* If we have a tag, see if the auto-follow rules say to update it */
@@ -1499,6 +1502,7 @@ static int next_head(const git_remote *remote, git_vector *refs,
 	git_remote_head *head;
 	git_refspec *spec, *passive_spec;
 	size_t i, j, k;
+	int valid;
 
 	active = &remote->active_refspecs;
 	passive = &remote->passive_refspecs;
@@ -1510,7 +1514,10 @@ static int next_head(const git_remote *remote, git_vector *refs,
 	for (; i < refs->length; i++) {
 		head = git_vector_get(refs, i);
 
-		if (!git_reference_is_valid_name(head->name))
+		if (git_reference_name_is_valid(&valid, head->name) < 0)
+			return -1;
+
+		if (!valid)
 			continue;
 
 		for (; j < active->length; j++) {
diff --git a/src/repository.c b/src/repository.c
index 513dbd6..f0d4b06 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -2359,7 +2359,7 @@ int git_repository_initialbranch(git_buf *out, git_repository *repo)
 	git_config *config;
 	git_config_entry *entry = NULL;
 	const char *branch;
-	int error;
+	int valid, error;
 
 	if ((error = git_repository_config__weakptr(&config, repo)) < 0)
 		return error;
@@ -2375,10 +2375,11 @@ int git_repository_initialbranch(git_buf *out, git_repository *repo)
 	}
 
 	if ((error = git_buf_puts(out, GIT_REFS_HEADS_DIR)) < 0 ||
-	    (error = git_buf_puts(out, branch)) < 0)
+	    (error = git_buf_puts(out, branch)) < 0 ||
+	    (error = git_reference_name_is_valid(&valid, out->ptr)) < 0)
 	    goto done;
 
-	if (!git_reference_is_valid_name(out->ptr)) {
+	if (!valid) {
 		git_error_set(GIT_ERROR_INVALID, "the value of init.defaultBranch is not a valid reference name");
 		error = -1;
 	}
diff --git a/tests/online/push_util.c b/tests/online/push_util.c
index fe9af2c..b39f781 100644
--- a/tests/online/push_util.c
+++ b/tests/online/push_util.c
@@ -58,12 +58,14 @@ int record_update_tips_cb(const char *refname, const git_oid *a, const git_oid *
 int create_deletion_refspecs(git_vector *out, const git_remote_head **heads, size_t heads_len)
 {
 	git_buf del_spec = GIT_BUF_INIT;
+	int valid;
 	size_t i;
 
 	for (i = 0; i < heads_len; i++) {
 		const git_remote_head *head = heads[i];
 		/* Ignore malformed ref names (which also saves us from tag^{} */
-		if (!git_reference_is_valid_name(head->name))
+		cl_git_pass(git_reference_name_is_valid(&valid, head->name));
+		if (!valid)
 			return 0;
 
 		/* Create a refspec that deletes a branch in the remote */
diff --git a/tests/refs/isvalidname.c b/tests/refs/isvalidname.c
index 65c70ba..063f0f7 100644
--- a/tests/refs/isvalidname.c
+++ b/tests/refs/isvalidname.c
@@ -1,31 +1,38 @@
 #include "clar_libgit2.h"
 
+static bool is_valid_name(const char *name)
+{
+	int valid;
+	cl_git_pass(git_reference_name_is_valid(&valid, name));
+	return valid;
+}
+
 void test_refs_isvalidname__can_detect_invalid_formats(void)
 {
-	cl_assert_equal_i(false, git_reference_is_valid_name("refs/tags/0.17.0^{}"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("TWO/LEVELS"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("ONE.LEVEL"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("HEAD/"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("NO_TRAILING_UNDERSCORE_"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("_NO_LEADING_UNDERSCORE"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("HEAD/aa"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("lower_case"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("/stupid/name/master"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("/"));
-	cl_assert_equal_i(false, git_reference_is_valid_name("//"));
-	cl_assert_equal_i(false, git_reference_is_valid_name(""));
-	cl_assert_equal_i(false, git_reference_is_valid_name("refs/heads/sub.lock/webmatrix"));
+	cl_assert_equal_i(false, is_valid_name("refs/tags/0.17.0^{}"));
+	cl_assert_equal_i(false, is_valid_name("TWO/LEVELS"));
+	cl_assert_equal_i(false, is_valid_name("ONE.LEVEL"));
+	cl_assert_equal_i(false, is_valid_name("HEAD/"));
+	cl_assert_equal_i(false, is_valid_name("NO_TRAILING_UNDERSCORE_"));
+	cl_assert_equal_i(false, is_valid_name("_NO_LEADING_UNDERSCORE"));
+	cl_assert_equal_i(false, is_valid_name("HEAD/aa"));
+	cl_assert_equal_i(false, is_valid_name("lower_case"));
+	cl_assert_equal_i(false, is_valid_name("/stupid/name/master"));
+	cl_assert_equal_i(false, is_valid_name("/"));
+	cl_assert_equal_i(false, is_valid_name("//"));
+	cl_assert_equal_i(false, is_valid_name(""));
+	cl_assert_equal_i(false, is_valid_name("refs/heads/sub.lock/webmatrix"));
 }
 
 void test_refs_isvalidname__wont_hopefully_choke_on_valid_formats(void)
 {
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/tags/0.17.0"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/LEVELS"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("HEAD"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("ONE_LEVEL"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/stash"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/remotes/origin/bim_with_3d@11296"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/master{yesterday"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/master}yesterday"));
-	cl_assert_equal_i(true, git_reference_is_valid_name("refs/master{yesterday}"));
+	cl_assert_equal_i(true, is_valid_name("refs/tags/0.17.0"));
+	cl_assert_equal_i(true, is_valid_name("refs/LEVELS"));
+	cl_assert_equal_i(true, is_valid_name("HEAD"));
+	cl_assert_equal_i(true, is_valid_name("ONE_LEVEL"));
+	cl_assert_equal_i(true, is_valid_name("refs/stash"));
+	cl_assert_equal_i(true, is_valid_name("refs/remotes/origin/bim_with_3d@11296"));
+	cl_assert_equal_i(true, is_valid_name("refs/master{yesterday"));
+	cl_assert_equal_i(true, is_valid_name("refs/master}yesterday"));
+	cl_assert_equal_i(true, is_valid_name("refs/master{yesterday}"));
 }