* Re: [PING][PATCH] posix: if glob has a trailing slash match directories only.
@ 2017-12-11 16:08 Dmitry Goncharov
0 siblings, 0 replies; only message in thread
From: Dmitry Goncharov @ 2017-12-11 16:08 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha, bug-gnulib
ping
On Mon, Dec 4, 2017 at 11:58 PM, Dmitry Goncharov
<dgoncharov@users.sf.net> wrote:
> On Mon, Dec 04, 2017 at 11:32:06AM -0800, Paul Eggert wrote:
>> On 12/04/2017 07:46 AM, Dmitry Goncharov wrote:
>> > 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 @@
> +2017-11-28 Dmitry Goncharov <dgoncharov@users.sf.net>
> +
> + [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.
> +
> 2017-11-13 Florian Weimer <fweimer@redhat.com>
>
> * 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
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2017-12-11 16:08 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-11 16:08 [PING][PATCH] posix: if glob has a trailing slash match directories only Dmitry Goncharov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).