Re: [PATCH 4/6] dir.c::match_pathname(): pay attention to the length of string parameters
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:34
Jeff King [off-list ref] 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.
quoted hunk
Signed-off-by: Jeff King <redacted> --- 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.