refs: error checking in internal name validation

Move `git_reference__is_valid_name` to `git_reference__name_is_valid`,
which returns errors and sets an out boolean parameter.

3 files changed, 64 insertions, 36 deletions

diff --git a/src/refs.c b/src/refs.c
index 51635a9e4..bb83d9b3c 100644
--- a/src/refs.c
+++ b/src/refs.c
@@ -1287,19 +1287,32 @@ cleanup:
 	return error;
 }
 
-int git_reference__is_valid_name(const char *refname, unsigned int flags)
+int git_reference__name_is_valid(
+	int *valid,
+	const char *refname,
+	unsigned int flags)
 {
-	if (git_reference__normalize_name(NULL, refname, flags) < 0) {
-		git_error_clear();
-		return false;
-	}
+	int error;
 
-	return true;
+	*valid = 0;
+
+	error = git_reference__normalize_name(NULL, refname, flags);
+
+	if (!error)
+		*valid = 1;
+	else if (error == GIT_EINVALIDSPEC)
+		error = 0;
+
+	return error;
 }
 
 int git_reference_is_valid_name(const char *refname)
 {
-	return git_reference__is_valid_name(refname, GIT_REFERENCE_FORMAT_ALLOW_ONELEVEL);
+	int valid = 0;
+
+	git_reference__name_is_valid(&valid, refname, GIT_REFERENCE_FORMAT_ALLOW_ONELEVEL);
+
+	return valid;
 }
 
 const char *git_reference__shorthand(const char *name)
diff --git a/src/refs.h b/src/refs.h
index e0ee03b0e..376a512f8 100644
--- a/src/refs.h
+++ b/src/refs.h
@@ -85,7 +85,7 @@ git_reference *git_reference__realloc(git_reference **ptr_to_ref, const char *na
 
 int git_reference__normalize_name(git_buf *buf, const char *name, unsigned int flags);
 int git_reference__update_terminal(git_repository *repo, const char *ref_name, const git_oid *oid, const git_signature *sig, const char *log_message);
-int git_reference__is_valid_name(const char *refname, unsigned int flags);
+int git_reference__name_is_valid(int *valid, const char *name, unsigned int flags);
 int git_reference__is_branch(const char *ref_name);
 int git_reference__is_remote(const char *ref_name);
 int git_reference__is_tag(const char *ref_name);
diff --git a/src/refspec.c b/src/refspec.c
index 854240a84..95b2830f8 100644
--- a/src/refspec.c
+++ b/src/refspec.c
@@ -21,7 +21,8 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 	size_t llen;
 	int is_glob = 0;
 	const char *lhs, *rhs;
-	int flags;
+	int valid = 0;
+	unsigned int flags;
 
 	assert(refspec && input);
 
@@ -75,57 +76,69 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 
 	if (is_fetch) {
 		/*
-			* LHS
-			* - empty is allowed; it means HEAD.
-			* - otherwise it must be a valid looking ref.
-			*/
+		 * LHS
+		 * - empty is allowed; it means HEAD.
+		 * - otherwise it must be a valid looking ref.
+		 */
 		if (!*refspec->src)
 			; /* empty is ok */
-		else if (!git_reference__is_valid_name(refspec->src, flags))
+		else if (git_reference__name_is_valid(&valid, refspec->src, flags) < 0)
+			goto on_error;
+		else if (!valid)
 			goto invalid;
+
 		/*
-			* RHS
-			* - missing is ok, and is same as empty.
-			* - empty is ok; it means not to store.
-			* - otherwise it must be a valid looking ref.
-			*/
+		 * RHS
+		 * - missing is ok, and is same as empty.
+		 * - empty is ok; it means not to store.
+		 * - otherwise it must be a valid looking ref.
+		 */
 		if (!refspec->dst)
 			; /* ok */
 		else if (!*refspec->dst)
 			; /* ok */
-		else if (!git_reference__is_valid_name(refspec->dst, flags))
+		else if (git_reference__name_is_valid(&valid, refspec->dst, flags) < 0)
+			goto on_error;
+		else if (!valid)
 			goto invalid;
 	} else {
 		/*
-			* LHS
-			* - empty is allowed; it means delete.
-			* - when wildcarded, it must be a valid looking ref.
-			* - otherwise, it must be an extended SHA-1, but
-			*   there is no existing way to validate this.
-			*/
+		 * LHS
+		 * - empty is allowed; it means delete.
+		 * - when wildcarded, it must be a valid looking ref.
+		 * - otherwise, it must be an extended SHA-1, but
+		 *   there is no existing way to validate this.
+		 */
 		if (!*refspec->src)
 			; /* empty is ok */
 		else if (is_glob) {
-			if (!git_reference__is_valid_name(refspec->src, flags))
+			if (git_reference__name_is_valid(&valid, refspec->src, flags) < 0)
+				goto on_error;
+			else if (!valid)
 				goto invalid;
 		}
 		else {
 			; /* anything goes, for now */
 		}
+
 		/*
-			* RHS
-			* - missing is allowed, but LHS then must be a
-			*   valid looking ref.
-			* - empty is not allowed.
-			* - otherwise it must be a valid looking ref.
-			*/
+		 * RHS
+		 * - missing is allowed, but LHS then must be a
+		 *   valid looking ref.
+		 * - empty is not allowed.
+		 * - otherwise it must be a valid looking ref.
+		 */
 		if (!refspec->dst) {
-			if (!git_reference__is_valid_name(refspec->src, flags))
+			if (git_reference__name_is_valid(&valid, refspec->src, flags) < 0)
+				goto on_error;
+			else if (!valid)
 				goto invalid;
 		} else if (!*refspec->dst) {
 			goto invalid;
 		} else {
-			if (!git_reference__is_valid_name(refspec->dst, flags))
+			if (git_reference__name_is_valid(&valid, refspec->dst, flags) < 0)
+				goto on_error;
+			else if (!valid)
 				goto invalid;
 		}
 
@@ -141,10 +154,12 @@ int git_refspec__parse(git_refspec *refspec, const char *input, bool is_fetch)
 
 	return 0;
 
- invalid:
+invalid:
         git_error_set(
                 GIT_ERROR_INVALID,
                 "'%s' is not a valid refspec.", input);
+
+on_error:
         git_refspec__dispose(refspec);
 	return -1;
 }