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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 14, 2023, 21:48 UTC
Message-ID: <xmqqmt5f535i.fsf@gitster.g>
In-Reply-To: <Y+vV8Ifkj1QV7KF0@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 19 quoted lines

> +/*
> + * Check that the string refname matches a rule of the form
> + * "{prefix}%.*s{suffix}". So "foo/bar/baz" would match the rule
> + * "foo/%.*s/baz", and return the string "bar".
> + */
> +static const char *match_parse_rule(const char *refname, const char *rule,
> +				    size_t *len)
> +{
> +	/*
> +	 * Check that rule matches refname up to the first percent
> +	 * in the rule. This is basically skip_prefix(), but
> +	 * ending at percent in the prefix, rather than end-of-string.
> +	 */
> +	do {
> +		if (!*rule)
> +			BUG("rev-parse rule did not have percent");
> +		if (*rule == '%')
> +			break;
> +	} while (*refname++ == *rule++);

So, if we have refname="refs/heads/frotz" and rule="refs/%.*s", then we'll scan refname and rule to skip over their "refs/" prefix, and the next iteration, where post-increment moved the pointers to point at 'h' (at the beginning of "heads/frotz") on the refname side and '%' on the rule side, we iterate once more, notice *rule is '%', and break out of the loop. We have refname="heads/frotz" and rule="%.*s"

If we have refname="refsXheads/frotz" and rule="refs/%.*s", after skipping over "refs", refname points at 'X' while rule points at '/' and the loop needs to break. Both pointers are post-incremented, and now we have refname="heads/frotz" and rule="%.*s".

Am I reading the loop correctly? I wanted the bogus refname not to match the rule, but without peeking back refname[-1], I cannot tell the two cases apart at this point.

Show 7 quoted lines

> +	/*
> +	 * Check that we matched all the way to the "%" placeholder,
> +	 * and skip past it within the rule string. If so, "refname" at
> +	 * this point is the beginning of a potential match.
> +	 */
> +	if (!skip_prefix(rule, "%.*s", &rule))
> +		return NULL;

And we now have rule pointing at "" (i.e. "refs/%.*s" has been fully consumed). refname points at "heads/frotz".

Show 8 quoted lines

> +	/*
> +	 * And now check that our suffix (if any) matches.
> +	 */
> +	if (!strip_suffix(refname, rule, len))
> +		return NULL;
> +
> +	return refname; /* len set by strip_suffix() */
> +}

And the suffix "" is stripped and we yield "heads/frotz".