Re: [PATCH 4/6] dir.c::match_pathname(): pay attention to the length of string parameters

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

Jeff King <peff@peff.net> writes:

> This function takes two counted strings: a <pattern, patternlen> pair
> and a <pathname, pathlen> pair. But we end up feeding the result to
> fnmatch, which expects NUL-terminated strings.
>
> We can fix this by calling the fnmatch_icase_mem function, which
> handles re-allocating into a NUL-terminated string if necessary.
>
> While we're at it, we can avoid even calling fnmatch in some cases. In
> addition to patternlen, we get "prefix", the size of the pattern that
> contains no wildcard characters. We do a straight match of the prefix
> part first, and then use fnmatch to cover the rest. But if there are
> no wildcards in the pattern at all, we do not even need to call
> fnmatch; we would simply be comparing two empty strings.

It is not a new issue, but it would be nicer to have a comment to
warn people that this part needs to be adjusted when they want to
add support for using FNM_PERIOD in our codebase.

Other than that, looks sane to me.  Thanks.

>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> New in this iteration.
>
>  dir.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/dir.c b/dir.c
> index cc4ce8b..3ad44c3 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -624,11 +624,22 @@ int match_pathname(const char *pathname, int pathlen,
>  		if (strncmp_icase(pattern, name, prefix))
>  			return 0;
>  		pattern += prefix;
> +		patternlen -= prefix;
>  		name    += prefix;
>  		namelen -= prefix;
> +
> +		/*
> +		 * If the whole pattern did not have a wildcard,
> +		 * then our prefix match is all we need; we
> +		 * do not need to call fnmatch at all.
> +		 */
> +		if (!patternlen && !namelen)
> +			return 1;
>  	}
>  
> -	return fnmatch_icase(pattern, name, FNM_PATHNAME) == 0;
> +	return fnmatch_icase_mem(pattern, patternlen,
> +				 name, namelen,
> +				 FNM_PATHNAME) == 0;
>  }
>  
>  /* Scan the list and let the last match determine the fate.