Re: [PATCH 4/8] wildmatch: support "no FNM_PATHNAME" mode

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Dec 28, 2012, 07:15 UTC
Message-ID: <CACsJy8C7Y8xLVVhUtt1g234XnHrTbjRCBcY261QEe-y81A1YUw@mail.gmail.com>
In-Reply-To: <7vwqw2k833.fsf@alter.siamese.dyndns.org>

On Fri, Dec 28, 2012 at 1:24 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 10 quoted lines

>>                       if (*++p == '*') {
>>                               const uchar *prev_p = p - 2;
>>                               while (*++p == '*') {}
>> -                             if ((prev_p == text || *prev_p == '/') ||
>> +                             if (!(flags & WM_PATHNAME))
>> +                                     /* without WM_PATHNAME, '*' == '**' */
>> +                                     special = 1;
>> +                             else if ((prev_p == text || *prev_p == '/') ||
>
> Not a new issue in this patch,

No, it's an issue from nd/wildmatch, 40bbee0 (wildmatch: adjust "**" behavior - 2012-10-15).

Show 6 quoted lines

> but here, "prev_p" points into the
> pattern string, two bytes before p, which is the byte before the
> "**" that we are looking at (which might be before the beginning of
> the pattern).  "text" is the string we are trying to match that
> pattern against.  How can these two pointers be compared to yield a
> meaningful value?

They can't. I wanted to check whether "**" is at the start of the pattern (so no preceding '/' needed) and used a wrong pointer to compare to. Funny there is a test about this and it does not catch it because prev_p access something before the pattern. Will fix.

Show 10 quoted lines

>
>>                                   (*p == '\0' || *p == '/' ||
>>                                    (p[0] == '\\' && p[1] == '/'))) {
>
> OK.  "**/", "**" (end of pattern), and "**\/" are handled here.
>
> Do we have to worry about "**[/]" the same way, or a class never
> matches the directory separator, even if it is a singleton class
> that consists of '/' (which is fine by me)?  If so, is "\/" more or
> less like "[/]"?

This is a special case of "**" with FNM_PATHNAME on. With FNM_PATHNAME, '[]' and '?' cannot match '/' so any patterns with '[/]' match nothing. I think we don't need to worry about this case.

-- 
Duy