Re: [PATCH 3/3] shorten_unambiguous_ref(): avoid sscanf()

From

Jeff King <peff@peff.net>

Date

Feb 14, 2023, 22:40 UTC

Message-ID

<Y+wN0agVK9ZQU/sT@coredump.intra.peff.net>

In-Reply-To

<xmqqwn4j3mhy.fsf@gitster.g>

On Tue, Feb 14, 2023 at 02:34:01PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > but I have a feeling that it gets more readable if we flip the break
> > conditional and the loop condition.
> 
> Yeah, the somewhoat unusual loop structure was what motivated me to
> look at its corner case.  Flipping the logic around may make it more
> straight forward.

Doing it like this is much more readable:

diff --git a/refs.c b/refs.c
index d8ce7e9ee1..725adafcd8 100644
--- a/refs.c
+++ b/refs.c
@@ -1323,12 +1323,12 @@ static const char *match_parse_rule(const char *refname, const char *rule,
 	 * in the rule. This is basically skip_prefix(), but
 	 * ending at percent in the prefix, rather than end-of-string.
 	 */
-	do {
+	while (*rule != '%') {
 		if (!*rule)
 			BUG("rev-parse rule did not have percent");
-		if (*rule == '%')
-			break;
-	} while (*refname++ == *rule++);
+		if (*refname++ != *rule++)
+			return 0;
+	}
 
 	/*
 	 * Check that we matched all the way to the "%" placeholder,

I'll hold on to that (plus an adjustment to the comment below to match,
and perhaps a test for this negative-match case) for a day or so to give
anybody else a chance to comment, and then send out a v2 tomorrow.

-Peff