From patchwork Tue Dec 5 04:58:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Goncharov X-Patchwork-Id: 24722 Received: (qmail 50642 invoked by alias); 5 Dec 2017 04:58:09 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 50627 invoked by uid 89); 5 Dec 2017 04:58:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-qt0-f193.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=yO7FIr/gG1os1EUYh/B5piSN1C5RfAStBHSCR3JKPmI=; b=Db9p7UCEiJRXPM0eMhVvLJMI3z2XJeZPmygIRiBtyQZNWHPGySJqLVu85Y5OyZ2hyl hshQAFFbAyjPsUrgAeOwmNAKPHJurM0PSs5nyIKJsLFhYCgMFTT2tTW5UwohTYz+yhMO PCiwqiWEm/22MeUA8nwrvGvHk6hsXFFja//ydsETem5Y2PiuOh08kVgShNTDq5ekTgRZ rmvB50+F1/EMX1vS2Ioomc5H3Ctt36my4uB/0+Kt46qRDvAZsCWHcYk6mGL9bHIxz/n9 eOGOi6CvDQmBDo8mVxxLI7Sqmk5PFZb5XQvbdYzdNmYYEUS6IBeSY3lrYNxtOsmrT3EQ r97w== X-Gm-Message-State: AKGB3mJd8OlEOdOyRHICC+d1Dg9HsrSe4q8QmGpvyDUZecORZjMkGCj/ nil2KP3PGilhJV3ie/2JkdE= X-Google-Smtp-Source: AGs4zMbX4+1yAjVgNOyyyd47Pm55fFVAKiWuwm3IP+42eLCbpol8RtYEUOSaUukdAc73iL6wG5ChlA== X-Received: by 10.200.34.242 with SMTP id g47mr620365qta.36.1512449885018; Mon, 04 Dec 2017 20:58:05 -0800 (PST) Date: Mon, 4 Dec 2017 23:58:03 -0500 From: Dmitry Goncharov To: Paul Eggert Cc: libc-alpha@sourceware.org, bug-gnulib@gnu.org Subject: Re: [PATCH] posix: if glob has a trailing slash match directories only. Message-ID: <20171205045802.GA11736@madrid> References: <20171129042114.GA2702@madrid> <20171204042021.GA3045@madrid> <8af8f535-5b5b-8a11-b789-bef13b44ac8a@cs.ucla.edu> <6d06182e-f63d-d4d9-b651-af33768a9627@cs.ucla.edu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <6d06182e-f63d-d4d9-b651-af33768a9627@cs.ucla.edu> User-Agent: Mutt/1.5.21 (2010-09-15) 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 + + [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 * 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