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

On Fri, Mar 29, 2013 at 09:44:32AM -0700, Junio C Hamano wrote:

Duy Nguyen [off-list ref] writes:

quoted

quoted

So we would want to do any adjustment to the fix when we merge up to
maint.

OK. Then Junio, you may need to resolve the conflict with something
like this. Originally match_basename uses fnmatch, not wildmatch. But
using wildmatch there too should be fine, now that both
match_{base,path}name share fnmatch_icase_mem().

Thanks.

The result still smells somewhat funny, though.

fnmatch_icase_mem() is meant to be a wrapper of fnmatch_icase() for
counted strings and its matching semantics should be the same as
fnmatch_icase().

With the merge-fix, fnmatch_icase_mem() calls into wildmatch(), but
fnmatch_icase() still calls into fnmatch().

The latter's flags are meant to be taken from FNM_* family, but the
former takes flags from WM_* family of bits, no?

Yeah, that does not seem right. If match_pathname has learned to call
into wildmatch instead of fnmatch_icase in the interim, then the right
resolution is to convert its call to fnmatch_icase_mem to a new
wildmatch_mem.  Presumably that can be done by either tweaking
fnmatch_icase_mem, or, if wildmatch is ready to take counted strings,
calling into it with the right options.

I think you are running with USE_WILDMATCH which may make the
differences harder to notice, but the name fnmatch_icase_mem() that is
not in the same family as fnmatch but is from the wildmatch() family
smells like an accident waiting to happen.

Agreed.

I tend to think in the longer term it may be a good idea to build with
USE_WILDMATCH unconditionally (we can lose compat/fnmatch), so in the
end this may not matter that much

Yeah, I think that is a sane long-term goal.

-Peff