Skip to content

Commit

Permalink
Fix #12578 - Bug in directory_iterator, recursive_directory_iterator,…
Browse files Browse the repository at this point in the history
… equality testing of copied iterator also at end.
  • Loading branch information
Beman committed Nov 23, 2016
1 parent a3c1014 commit 5004d7b
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 7 deletions.
6 changes: 6 additions & 0 deletions doc/release_history.html
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ <h2>1.63.0</h2>
<a href="reference.html#filename_is_dot­_dot">filename_is_dot_dot</a></code>.
These add convenience and the implementations may be more efficient that user
coded equivalent functions.</li>
<li>Fix <a href="https://svn.boost.org/trac/boost/ticket/12578">#12578</a>, <i>
Make directory iterators able to detect when a copy has advanced to the end</i>.
This bug in <code>directory_iterator</code> and <code>
recursive_directory_iterator</code> equality testing has existed more than a
dozen years. Nowadays test driven development would likely have detected the
problem in early development. Sigh.</li>
<li>Fix broken link to
<a href="https://svn.boost.org/trac/boost/ticket/7506">#7506</a> in 1.60.0 Release History (Daniel Krügler).</li>
<li>Refactor <code>push_directory()</code>internal logic so it is easier to
Expand Down
21 changes: 15 additions & 6 deletions include/boost/filesystem/operations.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -921,8 +921,8 @@ namespace detail
friend BOOST_FILESYSTEM_DECL void detail::directory_iterator_increment(directory_iterator& it,
system::error_code* ec);

// shared_ptr provides shallow-copy semantics required for InputIterators.
// m_imp.get()==0 indicates the end iterator.
// shared_ptr provides the shallow-copy semantics required for single pass iterators
// (i.e. InputIterators). The end iterator is indicated by !m_imp || !m_imp->handle
boost::shared_ptr< detail::dir_itr_imp > m_imp;

friend class boost::iterator_core_access;
Expand All @@ -939,7 +939,11 @@ namespace detail
void increment() { detail::directory_iterator_increment(*this, 0); }

bool equal(const directory_iterator& rhs) const
{ return m_imp == rhs.m_imp; }
{
return m_imp == rhs.m_imp
|| (!m_imp && rhs.m_imp && !rhs.m_imp->handle)
|| (!rhs.m_imp && m_imp && !m_imp->handle);
}

}; // directory_iterator

Expand Down Expand Up @@ -1256,8 +1260,9 @@ namespace filesystem

private:

// shared_ptr provides shallow-copy semantics required for InputIterators.
// m_imp.get()==0 indicates the end iterator.
// shared_ptr provides the shallow-copy semantics required for single pass iterators
// (i.e. InputIterators).
// The end iterator is indicated by !m_imp || m_imp->m_stack.empty()
boost::shared_ptr< detail::recur_dir_itr_imp > m_imp;

friend class boost::iterator_core_access;
Expand All @@ -1283,7 +1288,11 @@ namespace filesystem
}

bool equal(const recursive_directory_iterator& rhs) const
{ return m_imp == rhs.m_imp; }
{
return m_imp == rhs.m_imp
|| (!m_imp && rhs.m_imp && rhs.m_imp->m_stack.empty())
|| (!rhs.m_imp && m_imp && m_imp->m_stack.empty()) ;
}

}; // recursive directory iterator

Expand Down
14 changes: 13 additions & 1 deletion test/operations_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -558,14 +558,22 @@ namespace
BOOST_TEST(dir_itr->path() != p);

// test case reported in comment to SourceForge bug tracker [937606]
// augmented to test single pass semantics of a copied iterator [#12578]
fs::directory_iterator itx(dir);
fs::directory_iterator itx2(itx);
BOOST_TEST(itx == itx2);
const fs::path p1 = (*itx++).path();
BOOST_TEST(itx == itx2);
BOOST_TEST(itx != fs::directory_iterator());
const fs::path p2 = (*itx++).path();
BOOST_TEST(itx == itx2);
BOOST_TEST(p1 != p2);
++itx;
BOOST_TEST(itx == itx2);
++itx;
BOOST_TEST(itx == itx2);
BOOST_TEST(itx == fs::directory_iterator());
BOOST_TEST(itx2 == fs::directory_iterator());
}

// Windows has a tricky special case when just the root-name is given,
Expand Down Expand Up @@ -631,15 +639,19 @@ namespace
cout << " with error_code argument" << endl;
boost::system::error_code ec;
int d1f1_count = 0;
for (fs::recursive_directory_iterator it (dir, fs::symlink_option::no_recurse);
fs::recursive_directory_iterator it(dir, fs::symlink_option::no_recurse);
fs::recursive_directory_iterator it2(it); // test single pass shallow copy semantics
for (;
it != fs::recursive_directory_iterator();
it.increment(ec))
{
if (it->path().filename() == "d1f1")
++d1f1_count;
BOOST_TEST(it == it2); // verify single pass shallow copy semantics
}
BOOST_TEST(!ec);
BOOST_TEST_EQ(d1f1_count, 1);
BOOST_TEST(it == it2); // verify single pass shallow copy semantics

cout << " recursive_directory_iterator_tests complete" << endl;
}
Expand Down

0 comments on commit 5004d7b

Please sign in to comment.