Re: [PATCH] posix: if glob has a trailing slash match directories only.

[Jonathan Nieder <jrnieder at gmail dot com>]

(+cc: bug-gnulib@, since they share this code)
Dmitry Goncharov wrote:

> If pattern has a trailing slash match directories only.
> This patch fixes [BZ #22513].
>
> +2017-11-28  Dmitry Goncharov  <dgoncharov@users.sf.net>
> +
> + [BZ #22513]
> + * posix/glob.c (glob_in_dir): Make glob with a trailing slash match
> + directores only.
> + * posix/globtest.sh: Add tests.
>
> Tested on x86-64 linux.

Thanks.  Looks like a reasonable thing to do.

> diff --git a/posix/glob.c b/posix/glob.c
> index cb39779d07..78873d83c6 100644
> --- a/posix/glob.c
> +++ b/posix/glob.c
> @@ -1342,7 +1342,33 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
>  	      if (flags & GLOB_ONLYDIR)
>  		switch (readdir_result_type (d))
>  		  {
> -		  case DT_DIR: case DT_LNK: case DT_UNKNOWN: break;
> +		  case DT_DIR: case DT_LNK: break;
> +		  case DT_UNKNOWN:
> +		  {
> +		    int dir;
> +		    size_t namlen = strlen (d.name);
> +		    size_t fullsize;
> +		    bool alloca_fullname
> +		      = (! size_add_wrapv (dirlen + 1, namlen + 1, &fullsize)
> +			 && glob_use_alloca (alloca_used, fullsize));
> +		    char *fullname;
> +		    if (alloca_fullname)
> +		      fullname = alloca_account (fullsize, alloca_used);
> +		    else
> +		      {
> +			fullname = malloc (fullsize);
> +			if (fullname == NULL)
> +			  return GLOB_NOSPACE;
> +		      }
> +		    mempcpy (mempcpy (mempcpy (fullname, directory, dirlen),
> +				      "/", 1),
> +			     d.name, namlen + 1);
> +		    dir = is_dir (fullname, flags, pglob);
> +		    if (__glibc_unlikely (!alloca_fullname))
> +		      free (fullname);
> +		    if (dir)
> +		      break;
> +		  }
>  		  default: continue;
>  		  }
>  

If I understand correctly, this treats DT_UNKNOWN like DT_DIR when stat
indicates that it is a directory.

What should happen in the DT_LNK case?  Should the same logic trip for
it as well so we can distinguish between a symlink to a directory and
other symlinks?

> diff --git a/posix/globtest.sh b/posix/globtest.sh
> index 73f7ae31cc..4f0c03715a 100755
> --- a/posix/globtest.sh
> +++ b/posix/globtest.sh
> @@ -43,13 +43,19 @@ export LC_ALL
>  
>  # Create the arena
>  testdir=${common_objpfx}posix/globtest-dir
> +testdir2=${common_objpfx}posix/globtest-dir2
>  testout=${common_objpfx}posix/globtest-out
>  rm -rf $testdir $testout
>  mkdir $testdir
> +mkdir $testdir2
> +mkdir $testdir2/hello1d
> +mkdir $testdir2/hello2d
> +echo 1 > $testdir2/hello1f
> +echo 2 > $testdir2/hello2f
>  
>  cleanup() {
>      chmod 777 $testdir/noread
> -    rm -fr $testdir $testout
> +    rm -fr $testdir $testdir2 $testout
>  }
>  
>  trap cleanup 0 HUP INT QUIT TERM
> @@ -815,6 +821,41 @@ if test $failed -ne 0; then
>    result=1
>  fi
>  
> +# Test that if the specified glob ends with a slash then only directories are
> +# matched.
> +# First run with the glob that has no slash to demonstrate presence of a slash
> +# makes a difference.
> +failed=0
> +${test_program_prefix} \
> +${common_objpfx}posix/globtest "$testdir2" "hello*" |
> +sort > $testout
> +cat <<"EOF" | $CMP - $testout >> $logfile || failed=1
> +`hello1d'
> +`hello1f'
> +`hello2d'
> +`hello2f'
> +EOF
> +
> +if test $failed -ne 0; then
> +  echo "pattern+meta test failed" >> $logfile
> +  result=1
> +fi
> +
> +# The same pattern+meta test with a slash this time.
> +failed=0
> +${test_program_prefix} \
> +${common_objpfx}posix/globtest "$testdir2" "hello*/" |
> +sort > $testout
> +cat <<"EOF" | $CMP - $testout >> $logfile || failed=1
> +`hello1d/'
> +`hello2d/'
> +EOF
> +
> +if test $failed -ne 0; then
> +  echo "pattern+meta+slash test failed" >> $logfile
> +  result=1
> +fi
> +
>  if test $result -eq 0; then
>      echo "All OK." > $logfile
>  fi

Thanks for the test.  It does a good job of motivating the change.

Thanks and hope that helps,
Jonathan