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

2012-12-27 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com 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


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

2012-12-27 Thread Nguyen Thai Ngoc Duy
On Fri, Dec 28, 2012 at 1:24 PM, Junio C Hamano gits...@pobox.com 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


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

2012-12-21 Thread Nguyễn Thái Ngọc Duy
So far, wildmatch() has always honoured directory boundary and there
was no way to turn it off. Make it behave more like fnmatch() by
requiring all callers that want the FNM_PATHNAME behaviour to pass
that in the equivalent flag WM_PATHNAME. Callers that do not specify
WM_PATHNAME will get wildcards like ? and * in their patterns matched
against '/', just like not passing FNM_PATHNAME to fnmatch().

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 dir.c|  2 +-
 t/t3070-wildmatch.sh | 27 +++
 test-wildmatch.c |  6 --
 wildmatch.c  | 13 +
 wildmatch.h  |  1 +
 5 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/dir.c b/dir.c
index 175a182..6ef0396 100644
--- a/dir.c
+++ b/dir.c
@@ -595,7 +595,7 @@ int match_pathname(const char *pathname, int pathlen,
}
 
return wildmatch(pattern, name,
-ignore_case ? WM_CASEFOLD : 0,
+WM_PATHNAME | (ignore_case ? WM_CASEFOLD : 0),
 NULL) == 0;
 }
 
diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
index d5bafef..dbfa903 100755
--- a/t/t3070-wildmatch.sh
+++ b/t/t3070-wildmatch.sh
@@ -29,6 +29,18 @@ match() {
 fi
 }
 
+pathmatch() {
+if [ $1 = 1 ]; then
+   test_expect_success pathmatch:match '$2' '$3' 
+   test-wildmatch pathmatch '$2' '$3'
+   
+else
+   test_expect_success pathmatch: no match '$2' '$3' 
+   ! test-wildmatch pathmatch '$2' '$3'
+   
+fi
+}
+
 # Basic wildmat features
 match 1 1 foo foo
 match 0 0 foo bar
@@ -192,4 +204,19 @@ match 0 0 
'XXX/adobe/courier/bold/o/normal//12/120/75/75/X/70/iso8859/1' 'XXX/*/
 match 1 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txt' '**/*a*b*g*n*t'
 match 0 0 'abcd/abcdefg/abcdefghijk/abcdefghijklmnop.txtz' '**/*a*b*g*n*t'
 
+pathmatch 1 foo foo
+pathmatch 0 foo fo
+pathmatch 1 foo/bar foo/bar
+pathmatch 1 foo/bar 'foo/*'
+pathmatch 1 foo/bba/arr 'foo/*'
+pathmatch 1 foo/bba/arr 'foo/**'
+pathmatch 1 foo/bba/arr 'foo*'
+pathmatch 1 foo/bba/arr 'foo**'
+pathmatch 1 foo/bba/arr 'foo/*arr'
+pathmatch 1 foo/bba/arr 'foo/**arr'
+pathmatch 0 foo/bba/arr 'foo/*z'
+pathmatch 0 foo/bba/arr 'foo/**z'
+pathmatch 1 foo/bar 'foo?bar'
+pathmatch 1 foo/bar 'foo[/]bar'
+
 test_done
diff --git a/test-wildmatch.c b/test-wildmatch.c
index 4bb23b4..a5f4833 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -12,9 +12,11 @@ int main(int argc, char **argv)
argv[i] += 3;
}
if (!strcmp(argv[1], wildmatch))
-   return !!wildmatch(argv[3], argv[2], 0, NULL);
+   return !!wildmatch(argv[3], argv[2], WM_PATHNAME, NULL);
else if (!strcmp(argv[1], iwildmatch))
-   return !!wildmatch(argv[3], argv[2], WM_CASEFOLD, NULL);
+   return !!wildmatch(argv[3], argv[2], WM_PATHNAME | WM_CASEFOLD, 
NULL);
+   else if (!strcmp(argv[1], pathmatch))
+   return !!wildmatch(argv[3], argv[2], 0, NULL);
else if (!strcmp(argv[1], fnmatch))
return !!fnmatch(argv[3], argv[2], FNM_PATHNAME);
else
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 == '/') ||
(*p == '\0' || *p == '/' ||
 (p[0] == '\\'  p[1] == '/'))) {
/*
@@ -103,7 +106,8 @@ static int dowild(const uchar *p, const uchar *text, 
unsigned int flags)
} else
return WM_ABORT_MALFORMED;
} else
-   special = 0;
+   /* without WM_PATHNAME, '*' == '**' */
+   special = flags  WM_PATHNAME ? 0 : 1;
if (*p == '\0') {
/* Trailing ** matches everything.  Trailing 
* matches
 * only if there are no more slash