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

[Dmitry Goncharov]

ping

On Mon, Dec 4, 2017 at 11:58 PM, Dmitry Goncharov

The stable algorithm results in more iterations and more copying. Are you sure?

Sorry, I'm not seeing why an extra iteration would be needed. I expect
users would prefer that this bug-fix does not alter the order of
returned values.

i was comparing 2 pass algorithms.
Please have a look at this one pass implementaion.
regards, Dmitry
diff --git a/ChangeLog b/ChangeLog
index ffeb208132..1b7e3984d2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+
+ [BZ #22513]
+ * posix/glob.c (glob_in_dir): Make pattern with a trailing slash
+ match directores only.
+ * posix/globtest.sh: Add tests.
+ * posix/globtest.c: Print gl_pathv before errors.
+
* support/next_to_fault.h, support/next_to_fault.c: New files.
diff --git a/posix/glob.c b/posix/glob.c
index cb39779d07..c63d628c59 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -1123,9 +1123,10 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (flags & GLOB_MARK)
{
/* Append slashes to directory names. */
- size_t i;
+ size_t i, e;
- for (i = oldcount; i < pglob->gl_pathc + pglob->gl_offs; ++i)
+ for (i = e = oldcount; i < pglob->gl_pathc + pglob->gl_offs; ++i)
+ {
if (is_dir (pglob->gl_pathv[i], flags, pglob))
{
size_t len = strlen (pglob->gl_pathv[i]) + 2;
@@ -1138,8 +1139,26 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
goto out;
}
strcpy (&new[len - 2], "/");
+ if (pglob->gl_pathv[e] == NULL)
+ {
+ pglob->gl_pathv[e++] = new;
+ pglob->gl_pathv[i] = NULL;
+ }
+ else
pglob->gl_pathv[i] = new;
}
+ else if (flags & GLOB_ONLYDIR)
+ {
+ free (pglob->gl_pathv[i]);
+ pglob->gl_pathv[i] = NULL;
+ if (pglob->gl_pathv[e] != NULL)
+ e = i;
+ }
+ }
+ if (pglob->gl_pathv[e] == NULL)
+ pglob->gl_pathc = e - pglob->gl_offs;
+ if (pglob->gl_pathc == 0)
+ retval = GLOB_NOMATCH;
}
if (!(flags & GLOB_NOSORT))
diff --git a/posix/globtest.c b/posix/globtest.c
index 878ae33044..0a94550d26 100644
--- a/posix/globtest.c
+++ b/posix/globtest.c
@@ -93,14 +93,6 @@ main (int argc, char *argv[])
glob_flags |= GLOB_APPEND;
}
- /* Was there an error? */
- if (i == GLOB_NOSPACE)
- puts ("GLOB_NOSPACE");
- else if (i == GLOB_ABORTED)
- puts ("GLOB_ABORTED");
- else if (i == GLOB_NOMATCH)
- puts ("GLOB_NOMATCH");
-
/* If we set an offset, fill in the first field.
(Unless glob() has filled it in already - which is an error) */
if ((glob_flags & GLOB_DOOFFS) && g.gl_pathv[0] == NULL)
@@ -109,10 +101,25 @@ main (int argc, char *argv[])
/* Print out the names. Unless otherwise specified, qoute them. */
if (g.gl_pathv)
{
- for (i = 0; i < g.gl_offs + g.gl_pathc; ++i)
+ int k;
+ for (k = 0; k < g.gl_offs + g.gl_pathc; ++k)
printf ("%s%s%s\n", quotes ? "`" : "",
- g.gl_pathv[i] ? g.gl_pathv[i] : "(null)",
+ g.gl_pathv[k] ? g.gl_pathv[k] : "(null)",
quotes ? "'" : "");
+
}
+
+ /* Was there an error?
+ * Printing the error after printing g.gl_pathv simplifies the test suite,
+ * because when -o is used 'abc' is always printed first regardless of
+ * whether the pattern matches anyting.
+ */
+ if (i == GLOB_NOSPACE)
+ puts ("GLOB_NOSPACE");
+ else if (i == GLOB_ABORTED)
+ puts ("GLOB_ABORTED");
+ else if (i == GLOB_NOMATCH)
+ puts ("GLOB_NOMATCH");
+
return 0;
}
diff --git a/posix/globtest.sh b/posix/globtest.sh
index 73f7ae31cc..bc86fcc232 100755
--- a/posix/globtest.sh
+++ b/posix/globtest.sh
@@ -43,13 +43,22 @@ export LC_ALL
# Create the arena
testdir=${common_objpfx}posix/globtest-dir
+tdir2=${common_objpfx}posix/globtest-dir2
testout=${common_objpfx}posix/globtest-out
-rm -rf $testdir $testout
+rm -rf $testdir $tdir2 $testout
mkdir $testdir
+mkdir $tdir2
+mkdir $tdir2/hello1d
+ln -s $tdir2/hello1d $tdir2/hello1ds
+mkdir $tdir2/hello2d
+echo 1 > $tdir2/hello1f
+ln -s $tdir2/hello1f $tdir2/hello1fs
+echo 2 > $tdir2/hello2f
+ln $tdir2/hello2f $tdir2/hello2fl
cleanup() {
chmod 777 $testdir/noread
- rm -fr $testdir $testout
+ rm -fr $testdir $tdir2 $testout
}
trap cleanup 0 HUP INT QUIT TERM
@@ -74,6 +83,30 @@ echo 1_1 > $testdir/dir1/file1_1
echo 1_2 > $testdir/dir1/file1_2
ln -fs dir1 $testdir/link1
+trailing_slash_test()
+{
+ local dir="$1"
+ local pattern="$2"
+ local expected="$3"
+ failed=0
+ ${test_program_prefix} \
+ ${common_objpfx}posix/globtest -q "$dir" "$pattern" | sort -fd > $testout
+ echo -e "$expected" | $CMP - $testout >> $logfile || failed=1
+ if test $failed -ne 0; then
+ echo "$pattern test failed" >> $logfile
+ result=1
+ fi
+
+ failed=0
+ ${test_program_prefix} \
+ ${common_objpfx}posix/globtest -qo "$dir" "$pattern" | sort -fd > $testout
+ echo -e "abc\n$expected" | $CMP - $testout >> $logfile || failed=1
+ if test $failed -ne 0; then
+ echo "$pattern with dooffs test failed" >> $logfile
+ result=1
+ fi
+}
+
# Run some tests.
result=0
rm -f $logfile
@@ -692,6 +725,22 @@ if test $failed -ne 0; then
result=1
fi
+# Try multiple patterns (GLOB_APPEND) with offset (GLOB_DOOFFS)
+# The pattern has a trailing slash which eliminates files in the
+# subdirectories.
+failed=0
+${test_program_prefix} \
+${common_objpfx}posix/globtest -o "$testdir" "file1" "*/*/" |
+sort > $testout
+cat <<"EOF" | $CMP - $testout >> $logfile || failed=1
+`abc'
+`file1'
+EOF
+if test $failed -ne 0; then
+ echo "GLOB_APPEND2 with slash test failed" >> $logfile
+ result=1
+fi
+
# Test NOCHECK with non-existing file in subdir.
failed=0
${test_program_prefix} \
@@ -815,6 +864,93 @@ if test $failed -ne 0; then
result=1
fi
+# This code tests the algo that filters out non directories when the pattern
+# has a trailing slash.
+# There are at least the following cases
+# - The pattern matches files and directories.
+# - The pattern matches nothing.
+# - The pattern matches only files.
+# - The pattern matches 1 file.
+# - The pattern matches only directories.
+# - The pattern matches 1 directory.
+# Each test is run twice, with and without a trailing slash to demonstrate
+# the differentce that the slash makes.
+# Each test is also run from the target directory by doing chdir and from the
+# current directory by specifying the full path in the pattern.
+# Each test is also run with and without dooffs.
+
+# The pattern matches files and directories.
+pattern='hello*'
+expected="hello1d\nhello1ds\nhello1f\nhello1fs\nhello2d\nhello2f\nhello2fl"
+trailing_slash_test "$tdir2" "$pattern" "$expected"
+
+expected="hello1d/\nhello1ds/\nhello2d/"
+trailing_slash_test "$tdir2" "$pattern/" "$expected"
+
+pattern="$tdir2/hello*"
+expected="$tdir2/hello1d\n$tdir2/hello1ds\n$tdir2/hello1f\n\
+$tdir2/hello1fs\n$tdir2/hello2d\n$tdir2/hello2f\n$tdir2/hello2fl"
+trailing_slash_test . "$pattern" "$expected"
+
+expected="$tdir2/hello1d/\n$tdir2/hello1ds/\n$tdir2/hello2d/"
+trailing_slash_test . "$pattern/" "$expected"
+
+# The pattern matches nothing.
+pattern='nomatch*'
+trailing_slash_test "$tdir2" "$pattern" GLOB_NOMATCH
+trailing_slash_test "$tdir2" "$pattern/" GLOB_NOMATCH
+trailing_slash_test . "$pattern" GLOB_NOMATCH
+trailing_slash_test . "$pattern/" GLOB_NOMATCH
+
+# The pattern matches only files.
+pattern='hello?f*'
+expected="hello1f\nhello1fs\nhello2f\nhello2fl"
+trailing_slash_test "$tdir2" "$pattern" "$expected"
+trailing_slash_test "$tdir2" "$pattern/" GLOB_NOMATCH
+
+pattern="$tdir2/hello?f*"
+expected="$tdir2/hello1f\n$tdir2/hello1fs\n$tdir2/hello2f\n$tdir2/hello2fl"
+trailing_slash_test . "$pattern" "$expected"
+trailing_slash_test . "$pattern/" GLOB_NOMATCH
+
+# The pattern matches only 1 file.
+pattern='hello*fl'
+expected="hello2fl"
+trailing_slash_test "$tdir2" "$pattern" "$expected"
+trailing_slash_test "$tdir2" "$pattern/" GLOB_NOMATCH
+
+pattern="$tdir2/hello*fl"
+expected="$tdir2/hello2fl"
+trailing_slash_test . "$pattern" "$expected"
+trailing_slash_test . "$pattern/" GLOB_NOMATCH
+
+# The pattern matches only directores.
+pattern='hello?d*'
+expected="hello1d\nhello1ds\nhello2d"
+trailing_slash_test "$tdir2" "$pattern" "$expected"
+
+expected="hello1d/\nhello1ds/\nhello2d/"
+trailing_slash_test "$tdir2" "$pattern/" "$expected"
+
+pattern="$tdir2/hello?d*"
+expected="$tdir2/hello1d\n$tdir2/hello1ds\n$tdir2/hello2d"
+trailing_slash_test . "$pattern" "$expected"
+
+expected="$tdir2/hello1d/\n$tdir2/hello1ds/\n$tdir2/hello2d/"
+trailing_slash_test . "$pattern/" "$expected"
+
+# The pattern matches only 1 directory.
+pattern='hello*ds'
+expected="hello1ds"
+trailing_slash_test "$tdir2" "$pattern" "$expected"
+trailing_slash_test "$tdir2" "$pattern/" "$expected/"
+
+pattern="$tdir2/hello*ds"
+expected="$tdir2/hello1ds"
+trailing_slash_test . "$pattern" "$expected"
+trailing_slash_test . "$pattern/" "$expected/"
+
+
if test $result -eq 0; then
echo "All OK." > $logfile
fi