Re: [PATCH v2 03/12] refs: stop using mkpath() in refname_match()

[Junio C Hamano <gitster@pobox.com>]

Jon Simons <jon@jonsimons.org> writes:

> +/*
> + * 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".
> + */

So would "foo/bar/bee/baz" also match and return "bar/bee".  Is that
expected?

> +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.
> +	 * We can bail immediately if not, but otherwise we leave "rule" at the
> +	 * %-placeholder, and "refname" at the start of the potential matched
> +	 * name.
> +	 */
> +	while (*rule != '%') {
> +		if (!*rule)
> +			BUG("rev-parse rule did not have percent");
> +		if (*refname++ != *rule++)
> +			return NULL;
> +	}
> +
> +	/*
> +	 * Check that our "%" is the expected placeholder. This assumes there
> +	 * are no other percents (placeholder or quoted) in the string, but
> +	 * that is sufficient for our rev-parse rules.
> +	 */
> +	if (!skip_prefix(rule, "%.*s", &rule))
> +		return NULL;
> +
> +	/*
> +	 * And now check that our suffix (if any) matches.
> +	 */
> +	if (!strip_suffix(refname, rule, len))
> +		return NULL;
> +
> +	return refname; /* len set by strip_suffix() */
> +}
> +
>  /*
>   * Is it possible that the caller meant full_name with abbrev_name?
>   * If so return a non-zero value to signal "yes"; the magnitude of
> @@ -662,12 +700,18 @@ static const char *ref_rev_parse_rules[] = {
>  int refname_match(const char *abbrev_name, const char *full_name)
>  {
>  	const char **p;
> -	const int abbrev_name_len = strlen(abbrev_name);
> +	const size_t abbrev_name_len = strlen(abbrev_name);
>  	const int num_rules = NUM_REV_PARSE_RULES;
>  
> -	for (p = ref_rev_parse_rules; *p; p++)
> -		if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name)))
> +	for (p = ref_rev_parse_rules; *p; p++) {
> +		size_t short_name_len;
> +		const char *short_name = match_parse_rule(full_name, *p,
> +							  &short_name_len);
> +
> +		if (short_name && short_name_len == abbrev_name_len &&
> +		    !memcmp(short_name, abbrev_name, abbrev_name_len))
>  			return &ref_rev_parse_rules[num_rules] - p;
> +	}

OK, this is the crux of the change, I guess.  Including the change
to use correct type "int"->"size_t", this looks very good.