[PATCH 1/7] shorten_unambiguous_ref(): Allow shortening refs/remotes/origin/HEAD to origin

[Johan Herland <johan@herland.net>]

When expanding shorthand refs to full ref names (e.g. in dwim_ref()),
we use the ref_rev_parse_rules list of expansion patterns. This list
allows "origin" to be expanded into "refs/remotes/origin/HEAD", by
using the "refs/remotes/%.*s/HEAD" pattern from that list.

shorten_unambiguous_ref() exists to provide the reverse operation:
turning a full ref name into a shorter (but still unambiguous) name.
It does so by matching the given refname against each pattern from
the ref_rev_parse_rules list (in reverse), and extracting the short-
hand name from the matching rule.

However, when given "refs/remotes/origin/HEAD" it fails to shorten it
into "origin", because we misuse the sscanf() function when matching
"refs/remotes/origin/HEAD" against "refs/remotes/%.*s/HEAD": We end
up calling sscanf like this:

  sscanf("refs/remotes/origin/HEAD", "refs/remotes/%s/HEAD", short_name)

In this case, sscanf() will match the initial "refs/remotes/" part, and
then match the remainder of the refname against the "%s", and place it
("origin/HEAD") into short_name. The part of the pattern following the
"%s" format is never verified, because sscanf() apparently does not
need to do that (it has performed the one expected format extraction,
and will return 1 correspondingly; see [1] for more details).

This patch replaces the misuse of sscanf() with a fairly simple function
that manually matches the refname against patterns, and extracts the
shorthand name.

Also a testcase verifying "refs/remotes/origin/HEAD" -> "origin" has
been added.

[1]: If we assume that sscanf() does not do a verification pass prior
to format extraction, there is AFAICS _no_ way for sscanf() - having
already done one or more format extractions - to indicate to its caller
that the input fails to match the trailing part of the format string.
In other words, AFAICS, the scanf() family of function will only verify
matching input up to and including the last format specifier in the
format string. Any data following the last format specifier will not be
verified. Yet another reason to consider the scanf functions harmful...

Cc: Bert Wesarg <bert.wesarg@googlemail.com>
Signed-off-by: Johan Herland <johan@herland.net>
---
 refs.c                  | 82 +++++++++++++++++++------------------------------
 t/t6300-for-each-ref.sh | 12 ++++++++
 2 files changed, 43 insertions(+), 51 deletions(-)

diff --git a/refs.c b/refs.c
index d17931a..7231f54 100644
--- a/refs.c
+++ b/refs.c
@@ -2945,80 +2945,60 @@ struct ref *find_ref_by_name(const struct ref *list, const char *name)
 	return NULL;
 }
 
-/*
- * generate a format suitable for scanf from a ref_rev_parse_rules
- * rule, that is replace the "%.*s" spec with a "%s" spec
- */
-static void gen_scanf_fmt(char *scanf_fmt, const char *rule)
+int shorten_ref(const char *refname, const char *pattern, char *short_name)
 {
-	char *spec;
-
-	spec = strstr(rule, "%.*s");
-	if (!spec || strstr(spec + 4, "%.*s"))
-		die("invalid rule in ref_rev_parse_rules: %s", rule);
-
-	/* copy all until spec */
-	strncpy(scanf_fmt, rule, spec - rule);
-	scanf_fmt[spec - rule] = '\0';
-	/* copy new spec */
-	strcat(scanf_fmt, "%s");
-	/* copy remaining rule */
-	strcat(scanf_fmt, spec + 4);
-
-	return;
+	/*
+	 * pattern must be of the form "[pre]%.*s[post]". Check if refname
+	 * starts with "[pre]" and ends with "[post]". If so, write the
+	 * middle part into short_name, and return the number of chars
+	 * written (not counting the added NUL-terminator). Otherwise,
+	 * if refname does not match pattern, return 0.
+	 */
+	size_t pre_len, post_start, post_len, match_len;
+	size_t ref_len = strlen(refname);
+	char *sep = strstr(pattern, "%.*s");
+	if (!sep || strstr(sep + 4, "%.*s"))
+		die("invalid pattern in ref_rev_parse_rules: %s", pattern);
+	pre_len = sep - pattern;
+	post_start = pre_len + 4;
+	post_len = strlen(pattern + post_start);
+	if (pre_len + post_len >= ref_len)
+		return 0; /* refname too short */
+	match_len = ref_len - (pre_len + post_len);
+	if (strncmp(refname, pattern, pre_len) ||
+	    strncmp(refname + ref_len - post_len, pattern + post_start, post_len))
+		return 0; /* refname does not match */
+	memcpy(short_name, refname + pre_len, match_len);
+	short_name[match_len] = '\0';
+	return match_len;
 }
 
 char *shorten_unambiguous_ref(const char *refname, int strict)
 {
 	int i;
-	static char **scanf_fmts;
-	static int nr_rules;
 	char *short_name;
 
-	/* pre generate scanf formats from ref_rev_parse_rules[] */
-	if (!nr_rules) {
-		size_t total_len = 0;
-
-		/* the rule list is NULL terminated, count them first */
-		for (; ref_rev_parse_rules[nr_rules]; nr_rules++)
-			/* no +1 because strlen("%s") < strlen("%.*s") */
-			total_len += strlen(ref_rev_parse_rules[nr_rules]);
-
-		scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);
-
-		total_len = 0;
-		for (i = 0; i < nr_rules; i++) {
-			scanf_fmts[i] = (char *)&scanf_fmts[nr_rules]
-					+ total_len;
-			gen_scanf_fmt(scanf_fmts[i], ref_rev_parse_rules[i]);
-			total_len += strlen(ref_rev_parse_rules[i]);
-		}
-	}
-
-	/* bail out if there are no rules */
-	if (!nr_rules)
-		return xstrdup(refname);
-
 	/* buffer for scanf result, at most refname must fit */
 	short_name = xstrdup(refname);
 
 	/* skip first rule, it will always match */
-	for (i = nr_rules - 1; i > 0 ; --i) {
+	for (i = ARRAY_SIZE(ref_rev_parse_rules) - 1; i > 0 ; --i) {
 		int j;
 		int rules_to_fail = i;
 		int short_name_len;
 
-		if (1 != sscanf(refname, scanf_fmts[i], short_name))
+		if (!ref_rev_parse_rules[i] ||
+		    !(short_name_len = shorten_ref(refname,
+						   ref_rev_parse_rules[i],
+						   short_name)))
 			continue;
 
-		short_name_len = strlen(short_name);
-
 		/*
 		 * in strict mode, all (except the matched one) rules
 		 * must fail to resolve to a valid non-ambiguous ref
 		 */
 		if (strict)
-			rules_to_fail = nr_rules;
+			rules_to_fail = ARRAY_SIZE(ref_rev_parse_rules);
 
 		/*
 		 * check if the short name resolves to a valid ref,
diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 752f5cb..57e3109 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -466,4 +466,16 @@ test_expect_success 'Verify sort with multiple keys' '
 		refs/tags/bogo refs/tags/master > actual &&
 	test_cmp expected actual
 '
+
+cat >expected <<\EOF
+origin
+origin/master
+EOF
+
+test_expect_success 'Check refs/remotes/origin/HEAD shortens to origin' '
+	git remote set-head origin master &&
+	git for-each-ref --format="%(refname:short)" refs/remotes >actual &&
+	test_cmp expected actual
+'
+
 test_done
-- 
1.8.1.3.704.g33f7d4f