Re: [RFC][BZ #10278] glob() gives inconsistent results with trailing "/"

[OndÅej BÃlka <neleai at seznam dot cz>]

On Wed, Oct 16, 2013 at 09:03:56PM -0400, Carlos O'Donell wrote:
> On 10/11/2013 11:24 AM, OndÅej BÃlka wrote:
> > On Thu, Oct 03, 2013 at 04:49:40AM -0400, Carlos O'Donell wrote:
> >> On 10/03/2013 03:59 AM, OndÅej BÃlka wrote:
> >>> Hi,
> >>>
> >>> For this bug https://sourceware.org/bugzilla/show_bug.cgi?id=10278 there
> >>> is a simple fix when we have DIRENT_H defined.
> >>>
> >>> For cross platform solution we would need surround this by ifdefs when
> >>> separator is not slash and handling case of no dirent.
> >>>
> >>> Comments?
> >>
> >> Sounds correct.
> >>
> >> We should add a regression test for this.
> >>
> > 
> > When I tried add a test I encountered an unexpected result, any insight
> > into this?
> > 
> > results for glob ("*/*/", GLOB_ALTDIRFUNC)
> >   dir1lev1/dir1lev2/
> >   dir1lev1/dir2lev2/
> >   dir1lev1/dir3lev2/
> >   dir2lev1/dir1lev2/
> >   file2lev1/dir2lev1 *** WRONG
> 
> No idea, I'd have to dig into this to figure out why.
> 
> You definitely need to figure out why before we can
> checkin the patch.
> 

It turned out that testcase was wrong, after uncommenting DEBUG a output
was

my_opendir(".") == { level: 1, idx: 0 }
my_readdir ({ level: 1, idx: 0 }) = { d_ino: 0, d_type: 4, d_name: "." }
my_readdir ({ level: 1, idx: 1 }) = { d_ino: 1, d_type: 4, d_name: ".." }
my_readdir ({ level: 1, idx: 2 }) = { d_ino: 2, d_type: 8, d_name: "file1lev1" }
my_readdir ({ level: 1, idx: 3 }) = { d_ino: 3, d_type: 0, d_name: "file2lev1" }
looking for file2lev1, level 1
my_stat ("./file2lev1", { st_mode: 40777 }) = 0
my_readdir ({ level: 1, idx: 4 }) = { d_ino: 4, d_type: 0, d_name: "dir1lev1" }
looking for dir1lev1, level 1
my_stat ("./dir1lev1", { st_mode: 40777 }) = 0
my_readdir ({ level: 1, idx: 28 }) = { d_ino: 28, d_type: 4, d_name: "dir2lev1" }
my_readdir ({ level: 1, idx: -1 }) = NULL
my_closedir ()
looking for file2lev1, level 1
my_opendir("file2lev1") == { level: 1, idx: 4 }
my_readdir ({ level: 1, idx: 4 }) = { d_ino: 4, d_type: 0, d_name: "dir1lev1" }

So my_opendir lacked check that files are not directories. After fixing
that test passes.

	[BZ #10278]
	* posix/glob.c: Match only directories when trailing slash is present.
	* posix/tst-gnuglob.c (my_opendir): Do not open files.
	(main): Add testcase.

diff --git a/posix/glob.c b/posix/glob.c
index ece71c1..c1d61f0 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -276,6 +276,11 @@ glob (pattern, flags, errfunc, pglob)
       return -1;
     }
 
+  /* Posix requires all slashes to be matched.  This means that with 
+     trailing slash we could match only directories.  */
+  if (pattern[0] && pattern[strlen (pattern) - 1] == '/')
+    flags |= GLOB_ONLYDIR;
+
   if (!(flags & GLOB_DOOFFS))
     /* Have to do this so `globfree' knows where to start freeing.  It
        also makes all the code that uses gl_offs simpler. */
diff --git a/posix/tst-gnuglob.c b/posix/tst-gnuglob.c
index 0c967d0..5ef8909 100644
--- a/posix/tst-gnuglob.c
+++ b/posix/tst-gnuglob.c
@@ -168,7 +168,7 @@ my_opendir (const char *s)
   my_DIR *dir;
 
 
-  if (idx == -1)
+  if (idx == -1 || filesystem[idx].type != DT_DIR)
     {
       PRINTF ("my_opendir(\"%s\") == NULL\n", s);
       return NULL;
@@ -358,7 +358,7 @@ test_result (const char *fmt, int flags, glob_t *gl, const char *str[])
 	      break;
 
 	  if (str[inner] == NULL)
-	    errstr =  ok ? "" : " *** WRONG";
+	    errstr = ok ? "" : " *** WRONG";
 	  else
 	    errstr = ok ? "" : " * wrong position";
 
@@ -483,6 +483,12 @@ main (void)
 	"/file1lev1",
 	"/file2lev1");
 
+  test ("*/*/", 0 , 0,
+	"dir1lev1/dir1lev2/",
+	"dir1lev1/dir2lev2/",
+	"dir1lev1/dir3lev2/",
+	"dir2lev1/dir1lev2/");
+
   test ("", 0, GLOB_NOMATCH, NULL);
 
   test ("", GLOB_NOCHECK, 0, "");