Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH v5 1/2] refs.c: optimize check_refname_component()

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:01:27

David Turner [off-list ref] writes:
 static int check_refname_component(const char *refname, int flags)
 {
 	const char *cp;
 	char last = '\0';
 
 	for (cp = refname; ; cp++) {
-		char ch = *cp;
-		if (ch == '\0' || ch == '/')
+		unsigned char ch = (unsigned char) *cp;
Hmph, this cast bothers me.  I am fine with either of these two, though.

	int ch = *cp & 0377;
        unsigned char ch = *((unsigned char *)cp);
quoted hunk
+		unsigned char disp = refname_disposition[ch];
+		switch(disp) {
+		case 1:
+			goto out;
+		case 2:
+			if (last == '.')
+				return -1; /* Refname contains "..". */
+			break;
+		case 3:
+			if (last == '@')
+				return -1; /* Refname contains "@{". */
 			break;
-		if (bad_ref_char(ch))
-			return -1; /* Illegal character in refname. */
-		if (last == '.' && ch == '.')
-			return -1; /* Refname contains "..". */
-		if (last == '@' && ch == '{')
-			return -1; /* Refname contains "@{". */
+		case 4:
+			return -1;
+		}
 		last = ch;
 	}
+out:
 	if (cp == refname)
 		return 0; /* Component has zero length. */
 	if (refname[0] == '.') {
diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh
index c289322..1571176 100755
--- a/t/t5511-refspec.sh
+++ b/t/t5511-refspec.sh
@@ -5,7 +5,6 @@ test_description='refspec parsing'
 . ./test-lib.sh
 
 test_refspec () {
-
 	kind=$1 refspec=$2 expect=$3
 	git config remote.frotz.url "." &&
 	git config --remove-section remote.frotz &&
@@ -84,4 +83,9 @@ test_refspec push 'refs/heads/*/*/for-linus:refs/remotes/mine/*' invalid
 test_refspec fetch 'refs/heads/*/for-linus:refs/remotes/mine/*'
 test_refspec push 'refs/heads/*/for-linus:refs/remotes/mine/*'
 
+good=$(echo -n '\0377')
I think we avoid "echo -n" and use "printf" to be portable across
different echo implementations.

Use of \0377, which most likely to be just half-a-character, does
not feel a particularly good example, by the way.
+test_refspec fetch "refs/heads/${good}"
+bad=$(echo -n '\011')
Likewise.
+test_refspec fetch "refs/heads/${bad}"				invalid
+
 test_done
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help