[libcxx] r273070 - Fix bugs in recursive_directory_iterator::increment(ec) implementation and tests.

[Eric Fiselier via cfe-commits]

Author: ericwf
Date: Fri Jun 17 18:57:16 2016
New Revision: 273070

URL: http://llvm.org/viewvc/llvm-project?rev=273070&view=rev
Log:
Fix bugs in recursive_directory_iterator::increment(ec) implementation and tests.

r273060 didn't completely fix the issues in recursive_directory_iterator and
the tests. This patch follows up with more fixes

* Fix bug where recursive_directory_iterator::increment(ec) did not reset
  the error code if no failure occurred.

* Fix bad assertion in the recursive_directory_iterator::increment(ec) test
  that would only fire for certain iteration orders.

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

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=273070&r1=273069&r2=273070&view=diff
==============================================================================
--- libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/directory_iterator.cpp Fri Jun 17 18:57:16 2016
@@ -179,13 +179,12 @@ recursive_directory_iterator::recursive_
 void recursive_directory_iterator::__pop(error_code* ec)
 {
     _LIBCPP_ASSERT(__imp_, "Popping the end iterator");
+    if (ec) ec->clear();
     __imp_->__stack_.pop();
-    if (__imp_->__stack_.size() == 0) {
+    if (__imp_->__stack_.size() == 0)
         __imp_.reset();
-        if (ec) ec->clear();
-    } else {
+    else
         __advance(ec);
-    }
 }
 
 directory_options recursive_directory_iterator::options() const {
@@ -203,6 +202,7 @@ const directory_entry& recursive_directo
 recursive_directory_iterator&
 recursive_directory_iterator::__increment(error_code *ec)
 {
+    if (ec) ec->clear();
     if (recursion_pending()) {
         if (__try_recursion(ec) || (ec && *ec))
             return *this;
@@ -213,22 +213,19 @@ recursive_directory_iterator::__incremen
 }
 
 void recursive_directory_iterator::__advance(error_code* ec) {
+    // REQUIRES: ec must be cleared before calling this function.
     const directory_iterator end_it;
     auto& stack = __imp_->__stack_;
     std::error_code m_ec;
     while (stack.size() > 0) {
-        if (stack.top().advance(m_ec)) {
-            if (ec) ec->clear();
+        if (stack.top().advance(m_ec))
             return;
-        }
         if (m_ec) break;
         stack.pop();
     }
     __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=273070&r1=273069&r2=273070&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 18:57:16 2016
@@ -194,16 +194,24 @@ TEST_CASE(access_denied_on_recursion_tes
         recursive_directory_iterator it(startDir, SkipEPerm, ec);
         TEST_REQUIRE(!ec);
         TEST_REQUIRE(it != endIt);
-        const path elem = *it;
-        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);
+
+        bool seenOtherFile = false;
+        if (*it == otherFile) {
+            ++it;
+            seenOtherFile = true;
+            TEST_REQUIRE (it != endIt);
+        }
+        TEST_REQUIRE(*it == permDeniedDir);
+
+        ec = GetTestEC();
+        it.increment(ec);
+        TEST_REQUIRE(!ec);
+
+        if (seenOtherFile) {
             TEST_CHECK(it == endIt);
+        } else {
+            TEST_CHECK(it != endIt);
+            TEST_CHECK(*it == otherFile);
         }
     }
     // Test that construction resulting in a "EACCESS" error is not ignored