From: Jeff King <peff@peff.net>
Date: Mon, 27 Oct 2025 14:29:02 GMT
Subject: Re: [PATCH] match_pathname(): give fnmatch one char of prefix context
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:

> 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