[libcxx] r273060 - Fix bugs in recursive_directory_iterator implementation and tests.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 17 15:22:37 PDT 2016


Author: ericwf
Date: Fri Jun 17 17:22:37 2016
New Revision: 273060

URL: http://llvm.org/viewvc/llvm-project?rev=273060&view=rev
Log:
Fix bugs in recursive_directory_iterator implementation and tests.

There are two fixes in this patch:

* Fix bug where the constructor of recursive_directory_iterator did not reset
  the error code if no failure occurred.

* Fix tests were dependent on the iteration order of the test directories.

Modified:
    libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp
    libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
    libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/recursion_pending.pass.cpp

Modified: libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp?rev=273060&r1=273059&r2=273060&view=diff
==============================================================================
--- libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp Fri Jun 17 17:22:37 2016
@@ -165,6 +165,7 @@ recursive_directory_iterator::recursive_
     directory_options opt, error_code *ec)
     : __imp_(nullptr), __rec_(true)
 {
+    if (ec) ec->clear();
     std::error_code m_ec;
     __dir_stream new_s(p, opt, m_ec);
     if (m_ec) set_or_throw(m_ec, ec, "recursive_directory_iterator", p);
@@ -226,6 +227,8 @@ void recursive_directory_iterator::__adv
     __imp_.reset();
     if (m_ec)
         set_or_throw(m_ec, ec, "recursive_directory_iterator::operator++()");
+    else if (ec)
+        ec->clear();
 }
 
 bool recursive_directory_iterator::__try_recursion(error_code *ec) {

Modified: libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp?rev=273060&r1=273059&r2=273060&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp Fri Jun 17 17:22:37 2016
@@ -152,6 +152,7 @@ TEST_CASE(access_denied_on_recursion_tes
     const path startDir = testFiles[0];
     const path permDeniedDir = testFiles[1];
     const path otherFile = testFiles[3];
+    auto SkipEPerm = directory_options::skip_permission_denied;
 
     // Change the permissions so we can no longer iterate
     permissions(permDeniedDir, perms::none);
@@ -161,44 +162,49 @@ TEST_CASE(access_denied_on_recursion_tes
     // Test that recursion resulting in a "EACCESS" error is not ignored
     // by default.
     {
-        std::error_code ec;
+        std::error_code ec = GetTestEC();
         recursive_directory_iterator it(startDir, ec);
+        TEST_REQUIRE(ec != GetTestEC());
         TEST_REQUIRE(!ec);
+        while (it != endIt && it->path() != permDeniedDir)
+            ++it;
         TEST_REQUIRE(it != endIt);
-        const path elem = *it;
-        TEST_REQUIRE(elem == permDeniedDir);
+        TEST_REQUIRE(*it == permDeniedDir);
 
         it.increment(ec);
-        TEST_REQUIRE(ec);
-        TEST_REQUIRE(it == endIt);
+        TEST_CHECK(ec);
+        TEST_CHECK(it == endIt);
     }
     // Same as obove but test operator++().
     {
-        std::error_code ec;
+        std::error_code ec = GetTestEC();
         recursive_directory_iterator it(startDir, ec);
         TEST_REQUIRE(!ec);
+        while (it != endIt && it->path() != permDeniedDir)
+            ++it;
         TEST_REQUIRE(it != endIt);
-        const path elem = *it;
-        TEST_REQUIRE(elem == permDeniedDir);
+        TEST_REQUIRE(*it == permDeniedDir);
 
         TEST_REQUIRE_THROW(filesystem_error, ++it);
     }
     // Test that recursion resulting in a "EACCESS" error is ignored when the
     // correct options are given to the constructor.
     {
-        std::error_code ec;
-        recursive_directory_iterator it(
-            startDir,directory_options::skip_permission_denied,
-                                        ec);
+        std::error_code ec = GetTestEC();
+        recursive_directory_iterator it(startDir, SkipEPerm, ec);
         TEST_REQUIRE(!ec);
         TEST_REQUIRE(it != endIt);
         const path elem = *it;
-        TEST_REQUIRE(elem == permDeniedDir);
-
-        it.increment(ec);
-        TEST_REQUIRE(!ec);
-        TEST_REQUIRE(it != endIt);
-        TEST_CHECK(*it == otherFile);
+        if (elem == permDeniedDir) {
+            it.increment(ec);
+            TEST_REQUIRE(!ec);
+            TEST_REQUIRE(it != endIt);
+            TEST_CHECK(*it == otherFile);
+        } else if (elem == otherFile) {
+            it.increment(ec);
+            TEST_REQUIRE(!ec);
+            TEST_CHECK(it == endIt);
+        }
     }
     // Test that construction resulting in a "EACCESS" error is not ignored
     // by default.
@@ -216,10 +222,8 @@ TEST_CASE(access_denied_on_recursion_tes
     // Test that construction resulting in a "EACCESS" error constructs the
     // end iterator when the correct options are given.
     {
-        std::error_code ec;
-        recursive_directory_iterator it(permDeniedDir,
-                                        directory_options::skip_permission_denied,
-                                        ec);
+        std::error_code ec = GetTestEC();
+        recursive_directory_iterator it(permDeniedDir, SkipEPerm, ec);
         TEST_REQUIRE(!ec);
         TEST_REQUIRE(it == endIt);
     }

Modified: libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/recursion_pending.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/recursion_pending.pass.cpp?rev=273060&r1=273059&r2=273060&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/recursion_pending.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/recursion_pending.pass.cpp Fri Jun 17 17:22:37 2016
@@ -130,16 +130,33 @@ TEST_CASE(increment_resets_value)
 TEST_CASE(pop_does_not_reset_value)
 {
     const recursive_directory_iterator endIt;
+
+    auto& DE0 = StaticEnv::DirIterationList;
+    std::set<path> notSeenDepth0(std::begin(DE0), std::end(DE0));
+
     recursive_directory_iterator it(StaticEnv::Dir);
+    TEST_REQUIRE(it != endIt);
 
     while (it.depth() == 0) {
+        notSeenDepth0.erase(it->path());
         ++it;
         TEST_REQUIRE(it != endIt);
     }
+    TEST_REQUIRE(it.depth() == 1);
     it.disable_recursion_pending();
     it.pop();
-    TEST_REQUIRE(it != endIt);
-    TEST_CHECK(it.recursion_pending() == false);
+    // Since the order of iteration is unspecified the pop() could result
+    // in the end iterator. When this is the case it is undefined behavior
+    // to call recursion_pending().
+    if (it == endIt) {
+        TEST_CHECK(notSeenDepth0.empty());
+#if defined(_LIBCPP_VERSION)
+        TEST_CHECK(it.recursion_pending() == false);
+#endif
+    } else {
+        TEST_CHECK(! notSeenDepth0.empty());
+        TEST_CHECK(it.recursion_pending() == false);
+    }
 }
 
 TEST_SUITE_END()




More information about the cfe-commits mailing list