On Fri, Mar 29, 2013 at 09:44:32AM -0700, Junio C Hamano wrote:
> Duy Nguyen <pclouds@xxxxxxxxx> writes:
>
> >> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html