Re: [PATCH] match_pathname(): give fnmatch one char of prefix context

From: Jeff King <peff@peff.net>
Date: Oct 27, 2025, 14:29 UTC
Message-ID: <20251027142902.GB2758515@coredump.intra.peff.net>
In-Reply-To: <xmqq7bwhmecb.fsf@gitster.g>

On Sun, Oct 26, 2025 at 04:29:40PM -0700, Junio C Hamano wrote:

Show 13 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > I also wonder how expensive that memcmp() is. ;) Obviously not very, but
> > if the point is that we are trying to save fnmatch from looking at that
> > one extra character, we already pinching pennies in a mostly
> > un-measurable way.
> 
> I added the "limit to known bad case" in the illustration not for
> performance but for correctness.  This was because just like we
> weren't convinced that the "**/" may be the only case that breaks
> the existing optimization, I was worried if stepping back by one
> byte may somehow make a pattern that should not match mistakenly
> match.

Hmm, I hadn't really considered that. This is mostly hand-waving, but it feels unlikely to cause issues because we're giving fnmatch() more context, and never less. We know that the stepped-back character matches in the name and the pattern. So we are asking "o*bar" to match "obar" instead of "*bar" to match "bar", which seems like it should never be a bad thing unless there is a bug in fnmatch.

BTW, there was another bug mentioned in that original issue around backslash handling. I didn't investigate it at all, though. It didn't look like it would be related to this optimization, so I think we can just consider this fix independently.

-Peff