Re: [PATCH] exclude: fix a bug in prefix comparison optimization

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 14, 2012, 17:36 UTC
Message-ID: <7v8vb9yn09.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1350214522-3242-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

Show 7 quoted lines

> When "namelen" becomes zero at this stage, we have matched the fixed
> part, but whether it actually matches the pattern still depends on the
> pattern in "exclude". As demonstrated in t3001, path "three/a.3"
> exists and it matches the "three/a.3" part in pattern "three/a.3[abc]",
> but that does not mean a true match.
>
> Don't be too optimistic and let fnmatch() do the job.

Yeah, the existing code is correct _only_ if the pattern part can match an empty string (e.g. "three/a.3*") and this is a correct fix.

With your "teach attr.c match the same optimization as dir.c" series, you would need something like this

diff --git i/attr.c w/attr.c
index 6d39406..528e935 100644
--- i/attr.c
+++ w/attr.c
@@ -710,7 +710,7 @@ static int path_matches(const char *pathname, int pathlen,
 	 * if the non-wildcard part is longer than the remaining
 	 * pathname, surely it cannot match.
 	 */
-	if (!namelen || prefix > namelen)
+	if (prefix > namelen)
 		return 0;
 	if (baselen != 0)
 		baselen++;

Comparing the corresponding code in dir.c, there is no "compare the
literal prefix part with strcmp() before doing the fnmatch()"
optimization.  Intended?

(warning: I haven't had my caffeine yet)