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

[Nguyen Thai Ngoc Duy]

On Fri, Dec 28, 2012 at 1:24 PM, Junio C Hamano  wrote:
>>   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).

> 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.

>
>>   (*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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> diff --git a/wildmatch.c b/wildmatch.c
> index a79f97e..4fe1d65 100644
> --- a/wildmatch.c
> +++ b/wildmatch.c
> @@ -77,14 +77,17 @@ static int dowild(const uchar *p, const uchar *text, 
> unsigned int flags)
>   continue;
>   case '?':
>   /* Match anything but '/'. */
> - if (t_ch == '/')
> + if ((flags & WM_PATHNAME) && t_ch == '/')
>   return WM_NOMATCH;
>   continue;
>   case '*':
>   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, 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?

>   (*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 "[/]"?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html