Opened 12 years ago
Closed 8 years ago
#5403 closed Bugs (fixed)
filesystem3::recursive_directory_iterator::increment(system::error_code& ec) unexpected behaviour
| Reported by: | Owned by: | Beman Dawes | |
|---|---|---|---|
| Milestone: | To Be Determined | Component: | filesystem |
| Version: | Boost Development Trunk | Severity: | Showstopper |
| Keywords: | recursive_directory_iterator increment | Cc: |
Description
please see this link http://boost.2283326.n4.nabble.com/How-to-skip-a-sub-directory-with-filesystem3-recursive-directory-iterator-in-case-of-exception-td3417953.html for details.
ideally, something like below should be working for either throwing(ec.m_val !=0) or no-throwing(ec.m_val ==0 ) case.
boost::system::error_code ec;
for (recursive_directory_iterator itr(root, ec), itr_end; itr!=itr_end;
)
{
... ;
itr.increment(ec);
if(ec) itr.pop();
}
Additionally, the intuitive expectation is that: recursive_directory_iterator::increment(ec) is a drop-in replacement and behaves like recursive_directory_iterator::increment() or recursive_directory_iterator::operator++() in case of no throw(ec.m_val == 0). To be more specifically, in such a case, the above code is expected to be effectively the same as:
for (recursive_directory_iterator itr(root), itr_end; itr!=itr_end; ) {
... ;
itr.increment(); or itr++;
}
and
for (recursive_directory_iterator itr(root), itr_end; itr!=itr_end; itr++) {
... ;
}
the current implementation does not allow it.
Attachments (2)
Change History (9)
by , 12 years ago
| Attachment: | patch.patch added |
|---|
comment:1 by , 12 years ago
Further debug shows in file operations.hpp
void recur_dir_itr_imp::increment(system::error_code* ec)
{
//...skipped
m_stack.push(directory_iterator(m_stack.top()->path(), *ec));
//... skipped
}
an directory_iterator object would be inserted into m_stack even when ec!=0, but m_level is not incremented in this case. This seems to be a bug beccause
In another place in
void recur_dir_itr_imp::pop()
m_level is eppected to be incremented, and asserts if not.
The above attached patch increments m_level in increment() and solves this problem.
comment:2 by , 12 years ago
The link in the original report doesn not work somehow. For completeness of the context, this is another link describing the context:http://lists.boost.org/boost-users/2011/03/67329.php
comment:3 by , 10 years ago
I just came across this bug trying to skip subdirectories that could not be accessed. In my case I received a segmentation fault on the ++ line:
system::error_code ec;
iter.increment(ec);
if (ec) {
iter.no_push();
++iter;
}
I believe the problem is in the line the previous comment identified. Unlike the ec == 0 case, if the directory_iterator contructor fails and *ec is set to an error code, the invalid directory_iterator object is still added to the stack.
In subsequent calls to increment(), it will eventually be incremented by the while (!m_stack.empty() && ++m_stack.top() == directory_iterator()) loop, which is where the segfault happened.
I believe a fix might be as simple as changing the aforementioned chunk to something like
directory_iterator it(m_stack.top()->path(), *ec); if (*ec) return; m_stack.push(it);
comment:4 by , 10 years ago
The way to achieve skipping inaccessible directory in the client code without patching Boost is to follow this pattern:
for (recursive_directory_iterator itr(root, ec), itr_end; itr!=itr_end;/* itr++*/)
{
// do stuff
// replace ++itr with a no-throw version and
// pop this directory out to resume with the next one
itr.increment(ec);
if(ec) itr.pop();
}
it works on my trials. Help it's helpful to others as well
comment:5 by , 10 years ago
As long as the inaccessible directory is not in the first level, the itr.pop() works great. Otherwise you have problems with the level:
***** Internal Program Error - assertion (m_level > 0) failed in void boost::filesystem::detail::recur_dir_itr_imp::pop(): /usr/local/include/boost/filesystem/operations.hpp(818): pop() on recursive_directory_iterator with level < 1 Aborted (core dumped)
comment:6 by , 10 years ago
Imagine we are listing "dir1", and we are at element "node2" in this listing. iter->path() is "dir1/node2". There're several reasons for recur_dir_itr_imp::increment() to fail:
- at the beginning if "m_stack.top()->symlink_status(*ec)" or "m_stack.top()->status(*ec)" fails (stat or lstat on "dir1/node2" fails : we can't know if it is a directory). In this case, to continue, one need to call no_push() (using pop(), we would miss other files/folders after "node2" in "dir1").
- during the sublisting of "dir1/node2", in "m_stack.push(directory_iterator(m_stack.top()->path(),*ec))". At this time, one need to call pop() to continue, but, as said ttan, there's a bug is incrementing m_level. However, like Benjamin Herr, I think it's better not to push the bad directory_iterator: therefore, one just needs to call no_push() to continue listing.
- during the listing of "dir1" after "node2", in "++m_stack.top()". Note that this will always throw, even if ec is not null. Imagine this is corrected (using "m_stack.top().increment(*ec)"), one need to call pop() to continue listing. As far as I know, failing in the middle of a directory listing is really uncommon.
So, the first two cases are errors for going deeper in the hierarchy: to continue, one needs to no_push(). In the last case, error is when continuing listing at the same level: pop() is required to continue. As it's not possible to know what kind of error we have (without analysing exact error code, which may be difficult and implementation specific), I propose the following strategy:
iter.increment(ec);
while(ec) {
path cur_path = iter->path();
iter.no_push(); // if it an error of the third case, no_push() won't help, but is harmless as we'll get exactly the same error.
error_code saved_ec = ec;
iter.increment(ec);
if(!ec) {
cerr << "can't go inside " << cur_path << " " << saved_ec;
break;
} else {
cerr << "can't go inside and/or can't list after " << cur_path << " " << saved_ec;
iter.pop(ec);
}
}
Note that there's currently no iter.pop(error_code&) function, just a iter.pop() that will throw in case of an error.
Furthermore, in my understanding of the concept, pop() should work even at level 0 and return the end iterator. For me, pop() is just a way to pass all remaining filesystem nodes at the current level, and MUST NOT be followed by another increment (otherwise you'll miss one item, pop() being somehow an increment).
I think also that the first item returned by recursive_directory_iterator("/test") should be "/test", as in all POSIX recursive commands. But this last point may be disputed ;). Note that in this case, /test is level 0 and then calling pop() at this level become again a non-sense.
comment:7 by , 8 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
recursive_directory_iterator::increment implementation code has been reorganized, adding an invariant that progress is always made even if an error is reported by exception or error_code. Thanks to Claudio Bley for his pull request, which was incorporated into the reorganized code.
Tickets 5403 and 6821 have been closed. Please open a new issue and supply test cases if you continue to experience problems.

fix Ticket #5403