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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> writes:

> On Fri, Mar 29, 2013 at 09:44:32AM -0700, Junio C Hamano wrote:
> ...
>> 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.

This may be just the matter of naming.

It smelled wrong to me only because the "fnmatch" in the helper
fnmatch_icase_mem() told me that it should forever use fnmatch
semantics.  After reading its (only) two callsites, they are both
"the caller has transformed the inputs to this lowest level pathname
vs pattern matching function in order to reduce the cost of
matching, and now it is time to exercise the matcher".  The only
thing they care about is that they are calling "the lowest level
pathname vs pattern matching function."

If we pronounce "fnmatch_icase_mem()" as "match_path_with_pattern()"
or something in the original series, the problem may go away ;-)

Does any caller pass FNM_* bits to a callchain that reach the new *_mem()
function?
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]