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() _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits