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.


git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@273060 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Eric Fiselier
2016-06-17 22:22:37 +00:00
parent 40d9e09d89
commit 6f3b01af9c
3 changed files with 48 additions and 24 deletions

View File

@@ -165,6 +165,7 @@ recursive_directory_iterator::recursive_directory_iterator(const path& p,
directory_options opt, error_code *ec) directory_options opt, error_code *ec)
: __imp_(nullptr), __rec_(true) : __imp_(nullptr), __rec_(true)
{ {
if (ec) ec->clear();
std::error_code m_ec; std::error_code m_ec;
__dir_stream new_s(p, opt, m_ec); __dir_stream new_s(p, opt, m_ec);
if (m_ec) set_or_throw(m_ec, ec, "recursive_directory_iterator", p); if (m_ec) set_or_throw(m_ec, ec, "recursive_directory_iterator", p);
@@ -226,6 +227,8 @@ void recursive_directory_iterator::__advance(error_code* ec) {
__imp_.reset(); __imp_.reset();
if (m_ec) if (m_ec)
set_or_throw(m_ec, ec, "recursive_directory_iterator::operator++()"); set_or_throw(m_ec, ec, "recursive_directory_iterator::operator++()");
else if (ec)
ec->clear();
} }
bool recursive_directory_iterator::__try_recursion(error_code *ec) { bool recursive_directory_iterator::__try_recursion(error_code *ec) {

View File

@@ -152,6 +152,7 @@ TEST_CASE(access_denied_on_recursion_test_case)
const path startDir = testFiles[0]; const path startDir = testFiles[0];
const path permDeniedDir = testFiles[1]; const path permDeniedDir = testFiles[1];
const path otherFile = testFiles[3]; const path otherFile = testFiles[3];
auto SkipEPerm = directory_options::skip_permission_denied;
// Change the permissions so we can no longer iterate // Change the permissions so we can no longer iterate
permissions(permDeniedDir, perms::none); permissions(permDeniedDir, perms::none);
@@ -161,44 +162,49 @@ TEST_CASE(access_denied_on_recursion_test_case)
// Test that recursion resulting in a "EACCESS" error is not ignored // Test that recursion resulting in a "EACCESS" error is not ignored
// by default. // by default.
{ {
std::error_code ec; std::error_code ec = GetTestEC();
recursive_directory_iterator it(startDir, ec); recursive_directory_iterator it(startDir, ec);
TEST_REQUIRE(ec != GetTestEC());
TEST_REQUIRE(!ec); TEST_REQUIRE(!ec);
while (it != endIt && it->path() != permDeniedDir)
++it;
TEST_REQUIRE(it != endIt); TEST_REQUIRE(it != endIt);
const path elem = *it; TEST_REQUIRE(*it == permDeniedDir);
TEST_REQUIRE(elem == permDeniedDir);
it.increment(ec); it.increment(ec);
TEST_REQUIRE(ec); TEST_CHECK(ec);
TEST_REQUIRE(it == endIt); TEST_CHECK(it == endIt);
} }
// Same as obove but test operator++(). // Same as obove but test operator++().
{ {
std::error_code ec; std::error_code ec = GetTestEC();
recursive_directory_iterator it(startDir, ec); recursive_directory_iterator it(startDir, ec);
TEST_REQUIRE(!ec); TEST_REQUIRE(!ec);
while (it != endIt && it->path() != permDeniedDir)
++it;
TEST_REQUIRE(it != endIt); TEST_REQUIRE(it != endIt);
const path elem = *it; TEST_REQUIRE(*it == permDeniedDir);
TEST_REQUIRE(elem == permDeniedDir);
TEST_REQUIRE_THROW(filesystem_error, ++it); TEST_REQUIRE_THROW(filesystem_error, ++it);
} }
// Test that recursion resulting in a "EACCESS" error is ignored when the // Test that recursion resulting in a "EACCESS" error is ignored when the
// correct options are given to the constructor. // correct options are given to the constructor.
{ {
std::error_code ec; std::error_code ec = GetTestEC();
recursive_directory_iterator it( recursive_directory_iterator it(startDir, SkipEPerm, ec);
startDir,directory_options::skip_permission_denied,
ec);
TEST_REQUIRE(!ec); TEST_REQUIRE(!ec);
TEST_REQUIRE(it != endIt); TEST_REQUIRE(it != endIt);
const path elem = *it; const path elem = *it;
TEST_REQUIRE(elem == permDeniedDir); if (elem == permDeniedDir) {
it.increment(ec);
it.increment(ec); TEST_REQUIRE(!ec);
TEST_REQUIRE(!ec); TEST_REQUIRE(it != endIt);
TEST_REQUIRE(it != endIt); TEST_CHECK(*it == otherFile);
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 // Test that construction resulting in a "EACCESS" error is not ignored
// by default. // by default.
@@ -216,10 +222,8 @@ TEST_CASE(access_denied_on_recursion_test_case)
// Test that construction resulting in a "EACCESS" error constructs the // Test that construction resulting in a "EACCESS" error constructs the
// end iterator when the correct options are given. // end iterator when the correct options are given.
{ {
std::error_code ec; std::error_code ec = GetTestEC();
recursive_directory_iterator it(permDeniedDir, recursive_directory_iterator it(permDeniedDir, SkipEPerm, ec);
directory_options::skip_permission_denied,
ec);
TEST_REQUIRE(!ec); TEST_REQUIRE(!ec);
TEST_REQUIRE(it == endIt); TEST_REQUIRE(it == endIt);
} }

View File

@@ -130,16 +130,33 @@ TEST_CASE(increment_resets_value)
TEST_CASE(pop_does_not_reset_value) TEST_CASE(pop_does_not_reset_value)
{ {
const recursive_directory_iterator endIt; 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); recursive_directory_iterator it(StaticEnv::Dir);
TEST_REQUIRE(it != endIt);
while (it.depth() == 0) { while (it.depth() == 0) {
notSeenDepth0.erase(it->path());
++it; ++it;
TEST_REQUIRE(it != endIt); TEST_REQUIRE(it != endIt);
} }
TEST_REQUIRE(it.depth() == 1);
it.disable_recursion_pending(); it.disable_recursion_pending();
it.pop(); it.pop();
TEST_REQUIRE(it != endIt); // Since the order of iteration is unspecified the pop() could result
TEST_CHECK(it.recursion_pending() == false); // 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() TEST_SUITE_END()