directory_iterator and recursive_directory_iterator fail to meet this
requirement in http://wg21.link/n4099#Class-directory_iterator
The directory_iterator default constructor shall create an iterator
equal to the end iterator value, and this shall be the only valid
iterator for the end condition.
The current code creates the end iterator when an error occurs during
construction and an error_code parameter was used (so an exception
is not thrown, but construction finishes normally and sets the
error_code).
This fixes it by creating a distinct error state that is not the end
iterator state:
// An error occurred, we need a non-empty shared_ptr so that *this will
// not compare equal to the end iterator.
_M_dir.reset(static_cast<fs::_Dir*>(nullptr));
This way the shared_ptr owns a null pointer, so (bool)_M_dir is false
(and we don't allow incrementing or dereferencing) but it can be
distinguished from an empty shared_ptr by comparing them using
shared_ptr::owner_before.
(The order of the owner_before checks is chosen so that the common
case of testing iter != directory_iterator() should short-circuit and
only check the first condition).
There were a few other problems with directory iterators, including
the fact that the get_file_type function never worked because autoconf
was defining _GLIBCXX_GLIBCXX_HAVE_STRUCT_DIRENT_D_TYPE instead of
the macro I was checking, _GLIBCXX_HAVE_STRUCT_DIRENT_D_TYPE.
I've removed the ErrorCode utility that was meant to simplify
clearing/setting an error_code that may or may not be present, but
really just obsfuscated things.
I'm also now consistently checking the skip_permission_denied flag
everywhere it matters.
Tested x86_64-linux, powerpc64le-linux, x86_64-dragonfly4.1, committed
to trunk.
commit 8d08e1c6724cb433e1ca4f975ce85bd277ba2389
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Wed Sep 23 00:28:19 2015 +0100
Fix semantics of Filesystem TS directory iterators
[class.directory_iterator] p4 and [directory_iterator.members] p4
require that only the default constructor and ignored permission denied
errors can create the end iterator.
* acinclude.m4 (GLIBCXX_CHECK_FILESYSTEM_DEPS): Remove _GLIBCXX_
prefix from HAVE_STRUCT_DIRENT_D_TYPE.
* config.h.in: Regenerate.
* configure: Regenerate.
* include/experimental/fs_dir.h (operator==, operator==):
Use owner_before instead of pointer equality.
(directory_iterator(std::shared_ptr<_Dir>, error_code*)): Remove.
* src/filesystem/dir.cc (ErrorCode): Remove.
(_Dir::advance): Change ErrorCode parameter to error_code*, add
directory_options parameter and check it on error.
(opendir): Rename to open_dir to avoid clashing with macro. Change
ErrorCode parameter to error_code*.
(make_shared_dir): Remove.
(native_readdir) [_GLIBCXX_FILESYSTEM_IS_WINDOWS]: Don't set errno.
(directory_iterator(std::shared_ptr<_Dir>, error_code*)): Remove.
(directory_iterator(const path&, directory_options, error_code*)):
Pass options to _Dir::advance and create non-end iterator on error.
(recursive_directory_iterator(const path&, directory_options,
error_code*)): Clear error_code on ignored error, create non-end
iterator otherwise.
(recursive_directory_iterator::increment): Pass _M_options to
_Dir::advance.
(recursive_directory_iterator::pop): Likewise.
* testsuite/experimental/filesystem/iterators/directory_iterator.cc:
New.
* testsuite/experimental/filesystem/iterators/
recursive_directory_iterator.cc: New.
diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4
index c133c25..4b031f7 100644
--- a/libstdc++-v3/acinclude.m4
+++ b/libstdc++-v3/acinclude.m4
@@ -3940,7 +3940,7 @@ dnl
[glibcxx_cv_dirent_d_type=no])
])
if test $glibcxx_cv_dirent_d_type = yes; then
- AC_DEFINE(_GLIBCXX_HAVE_STRUCT_DIRENT_D_TYPE, 1, [Define to 1 if `d_type'
is a member of `struct dirent'.])
+ AC_DEFINE(HAVE_STRUCT_DIRENT_D_TYPE, 1, [Define to 1 if `d_type' is a
member of `struct dirent'.])
fi
AC_MSG_RESULT($glibcxx_cv_dirent_d_type)
dnl
diff --git a/libstdc++-v3/include/experimental/fs_dir.h
b/libstdc++-v3/include/experimental/fs_dir.h
index d46d41b..0c5253f 100644
--- a/libstdc++-v3/include/experimental/fs_dir.h
+++ b/libstdc++-v3/include/experimental/fs_dir.h
@@ -201,14 +201,12 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
return __tmp;
}
- friend bool
- operator==(const directory_iterator& __lhs,
- const directory_iterator& __rhs)
- { return __lhs._M_dir == __rhs._M_dir; }
-
private:
directory_iterator(const path&, directory_options, error_code*);
- directory_iterator(std::shared_ptr<_Dir>, error_code*);
+
+ friend bool
+ operator==(const directory_iterator& __lhs,
+ const directory_iterator& __rhs);
friend class recursive_directory_iterator;
@@ -222,6 +220,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
end(directory_iterator) { return directory_iterator(); }
inline bool
+ operator==(const directory_iterator& __lhs, const directory_iterator& __rhs)
+ {
+ return !__rhs._M_dir.owner_before(__lhs._M_dir)
+ && !__lhs._M_dir.owner_before(__rhs._M_dir);
+ }
+
+ inline bool
operator!=(const directory_iterator& __lhs, const directory_iterator& __rhs)
{ return !(__lhs == __rhs); }
@@ -287,14 +292,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
void disable_recursion_pending() { _M_pending = false; }
- friend bool
- operator==(const recursive_directory_iterator& __lhs,
- const recursive_directory_iterator& __rhs)
- { return __lhs._M_dirs == __rhs._M_dirs; }
-
private:
recursive_directory_iterator(const path&, directory_options, error_code*);
+ friend bool
+ operator==(const recursive_directory_iterator& __lhs,
+ const recursive_directory_iterator& __rhs);
+
struct _Dir_stack;
std::shared_ptr<_Dir_stack> _M_dirs;
directory_options _M_options;
@@ -308,6 +312,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
end(recursive_directory_iterator) { return recursive_directory_iterator(); }
inline bool
+ operator==(const recursive_directory_iterator& __lhs,
+ const recursive_directory_iterator& __rhs)
+ {
+ return !__rhs._M_dirs.owner_before(__lhs._M_dirs)
+ && !__lhs._M_dirs.owner_before(__rhs._M_dirs);
+ }
+
+ inline bool
operator!=(const recursive_directory_iterator& __lhs,
const recursive_directory_iterator& __rhs)
{ return !(__lhs == __rhs); }
diff --git a/libstdc++-v3/src/filesystem/dir.cc
b/libstdc++-v3/src/filesystem/dir.cc
index 016a78d..bce751c 100644
--- a/libstdc++-v3/src/filesystem/dir.cc
+++ b/libstdc++-v3/src/filesystem/dir.cc
@@ -43,28 +43,6 @@
namespace fs = std::experimental::filesystem;
-namespace
-{
- struct ErrorCode
- {
- ErrorCode(std::error_code* p) : ec(p) { }
-
- ErrorCode(ErrorCode&& e) : ec(std::exchange(e.ec, nullptr)) { }
-
- ~ErrorCode() { if (ec) ec->clear(); }
-
- void assign(int err)
- {
- ec->assign(err, std::generic_category());
- ec = nullptr;
- }
-
- explicit operator bool() { return ec != nullptr; }
-
- std::error_code* ec;
- };
-}
-
struct fs::_Dir
{
_Dir() : dirp(nullptr) { }
@@ -80,7 +58,7 @@ struct fs::_Dir
~_Dir() { if (dirp) ::closedir(dirp); }
- bool advance(ErrorCode);
+ bool advance(std::error_code*, directory_options = directory_options::none);
DIR* dirp;
fs::path path;
@@ -96,9 +74,14 @@ namespace
return (obj & bits) != Bitmask::none;
}
+ // Returns {dirp, p} on success, {nullptr, p} on error.
+ // If an ignored EACCES error occurs returns {}.
fs::_Dir
- opendir(const fs::path& p, fs::directory_options options, ErrorCode ec)
+ open_dir(const fs::path& p, fs::directory_options options, std::error_code*
ec)
{
+ if (ec)
+ ec->clear();
+
if (DIR* dirp = ::opendir(p.c_str()))
return {dirp, p};
@@ -112,16 +95,8 @@ namespace
"directory iterator cannot open directory", p,
std::error_code(err, std::generic_category())));
- ec.assign(err);
- return {};
- }
-
- inline std::shared_ptr<fs::_Dir>
- make_shared_dir(fs::_Dir&& dir)
- {
- if (dir.dirp)
- return std::make_shared<fs::_Dir>(std::move(dir));
- return {};
+ ec->assign(err, std::generic_category());
+ return {nullptr, p};
}
inline fs::file_type
@@ -158,7 +133,6 @@ namespace
native_readdir(DIR* dirp, ::dirent*& entryp)
{
#ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
- errno = 0;
if ((entryp = ::readdir(dirp)))
return 0;
return errno;
@@ -168,25 +142,34 @@ namespace
}
}
+// Returns false when the end of the directory entries is reached.
+// Reports errors by setting ec or throwing.
bool
-fs::_Dir::advance(ErrorCode ec)
+fs::_Dir::advance(error_code* ec, directory_options options)
{
+ if (ec)
+ ec->clear();
+
::dirent ent;
::dirent* result = &ent;
if (int err = native_readdir(dirp, result))
{
+ if (err == EACCES
+ && is_set(options, directory_options::skip_permission_denied))
+ return false;
+
if (!ec)
_GLIBCXX_THROW_OR_ABORT(filesystem_error(
"directory iterator cannot advance",
std::error_code(err, std::generic_category())));
- ec.assign(err);
+ ec->assign(err, std::generic_category());
return true;
}
else if (result != nullptr)
{
// skip past dot and dot-dot
if (!strcmp(ent.d_name, ".") || !strcmp(ent.d_name, ".."))
- return advance(std::move(ec));
+ return advance(ec, options);
entry = fs::directory_entry{path / ent.d_name};
type = get_file_type(ent);
return true;
@@ -202,15 +185,21 @@ fs::_Dir::advance(ErrorCode ec)
fs::directory_iterator::
directory_iterator(const path& p, directory_options options, error_code* ec)
-: directory_iterator(make_shared_dir(opendir(p, options, ec)), ec)
-{ }
-
-fs::directory_iterator::
-directory_iterator(std::shared_ptr<_Dir> dir, error_code* ec)
-: _M_dir(std::move(dir))
{
- if (_M_dir && !_M_dir->advance(ec))
- _M_dir.reset();
+ _Dir dir = open_dir(p, options, ec);
+
+ if (dir.dirp)
+ {
+ auto sp = std::make_shared<fs::_Dir>(std::move(dir));
+ if (sp->advance(ec, options))
+ _M_dir.swap(sp);
+ }
+ else if (!dir.path.empty())
+ {
+ // An error occurred, we need a non-empty shared_ptr so that *this will
+ // not compare equal to the end iterator.
+ _M_dir.reset(static_cast<fs::_Dir*>(nullptr));
+ }
}
const fs::directory_entry&
@@ -257,7 +246,7 @@ struct fs::recursive_directory_iterator::_Dir_stack :
std::stack<_Dir>
fs::recursive_directory_iterator::
recursive_directory_iterator(const path& p, directory_options options,
- error_code* ec)
+ error_code* ec)
: _M_options(options), _M_pending(true)
{
if (DIR* dirp = ::opendir(p.c_str()))
@@ -272,7 +261,11 @@ recursive_directory_iterator(const path& p,
directory_options options,
const int err = errno;
if (err == EACCES
&& is_set(options, fs::directory_options::skip_permission_denied))
- return;
+ {
+ if (ec)
+ ec->clear();
+ return;
+ }
if (!ec)
_GLIBCXX_THROW_OR_ABORT(filesystem_error(
@@ -280,6 +273,10 @@ recursive_directory_iterator(const path& p,
directory_options options,
std::error_code(err, std::generic_category())));
ec->assign(err, std::generic_category());
+
+ // An error occurred, we need a non-empty shared_ptr so that *this will
+ // not compare equal to the end iterator.
+ _M_dirs.reset(static_cast<_Dir_stack*>(nullptr));
}
}
@@ -358,21 +355,14 @@ fs::recursive_directory_iterator::increment(error_code&
ec) noexcept
if (std::exchange(_M_pending, true) && recurse(top, _M_options, ec))
{
- _Dir dir = opendir(top.entry.path(), _M_options, &ec);
- if (ec.value())
+ _Dir dir = open_dir(top.entry.path(), _M_options, &ec);
+ if (ec)
return *this;
if (dir.dirp)
- {
_M_dirs->push(std::move(dir));
- if (!_M_dirs->top().advance(&ec)) // dir is empty
- pop();
- return *this;
- }
- // else skip permission denied and continue in parent dir
}
- ec.clear();
- while (!_M_dirs->top().advance(&ec) && !ec.value())
+ while (!_M_dirs->top().advance(&ec, _M_options) && !ec)
{
_M_dirs->pop();
if (_M_dirs->empty())
@@ -399,5 +389,5 @@ fs::recursive_directory_iterator::pop()
_M_dirs.reset();
return;
}
- } while (!_M_dirs->top().advance(nullptr));
+ } while (!_M_dirs->top().advance(nullptr, _M_options));
}
diff --git
a/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
new file mode 100644
index 0000000..56b808d
--- /dev/null
+++
b/libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc
@@ -0,0 +1,77 @@
+// Copyright (C) 2015 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++11 -lstdc++fs" }
+// { dg-require-filesystem-ts "" }
+
+#include <experimental/filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::experimental::filesystem;
+
+void
+test01()
+{
+ bool test __attribute__((unused)) = false;
+ std::error_code ec;
+
+ // Test non-existent path.
+ const auto p = __gnu_test::nonexistent_path();
+ fs::directory_iterator iter(p, ec);
+ VERIFY( ec );
+ VERIFY( iter != fs::directory_iterator() );
+
+ // Test empty directory.
+ create_directory(p, fs::current_path(), ec);
+ VERIFY( !ec );
+ iter = fs::directory_iterator(p, ec);
+ VERIFY( !ec );
+ VERIFY( iter == fs::directory_iterator() );
+
+ // Test non-empty directory.
+ create_directory_symlink(p, p / "l", ec);
+ VERIFY( !ec );
+ iter = fs::directory_iterator(p, ec);
+ VERIFY( !ec );
+ VERIFY( iter != fs::directory_iterator() );
+ VERIFY( iter->path() == p/"l" );
+ ++iter;
+ VERIFY( iter == fs::directory_iterator() );
+
+ // Test inaccessible directory.
+ permissions(p, fs::perms::none, ec);
+ VERIFY( !ec );
+ iter = fs::directory_iterator(p, ec);
+ VERIFY( ec );
+ VERIFY( iter != fs::directory_iterator() );
+
+ // Test inaccessible directory, skipping permission denied.
+ const auto opts = fs::directory_options::skip_permission_denied;
+ iter = fs::directory_iterator(p, opts, ec);
+ VERIFY( !ec );
+ VERIFY( iter == fs::directory_iterator() );
+
+ permissions(p, fs::perms::owner_all, ec);
+ remove_all(p, ec);
+}
+
+int
+main()
+{
+ test01();
+}
diff --git
a/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
new file mode 100644
index 0000000..9424c80
--- /dev/null
+++
b/libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc
@@ -0,0 +1,104 @@
+// Copyright (C) 2015 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++11 -lstdc++fs" }
+// { dg-require-filesystem-ts "" }
+
+#include <experimental/filesystem>
+#include <testsuite_hooks.h>
+#include <testsuite_fs.h>
+
+namespace fs = std::experimental::filesystem;
+
+void
+test01()
+{
+ bool test __attribute__((unused)) = false;
+ std::error_code ec;
+
+ // Test non-existent path.
+ const auto p = __gnu_test::nonexistent_path();
+ fs::recursive_directory_iterator iter(p, ec);
+ VERIFY( ec );
+ VERIFY( iter != fs::recursive_directory_iterator() );
+
+ // Test empty directory.
+ create_directory(p, fs::current_path(), ec);
+ VERIFY( !ec );
+ iter = fs::recursive_directory_iterator(p, ec);
+ VERIFY( !ec );
+ VERIFY( iter == fs::recursive_directory_iterator() );
+
+ // Test non-empty directory.
+ create_directories(p / "d1/d2");
+ VERIFY( !ec );
+ iter = fs::recursive_directory_iterator(p, ec);
+ VERIFY( !ec );
+ VERIFY( iter != fs::recursive_directory_iterator() );
+ VERIFY( iter->path() == p/"d1" );
+ ++iter;
+ VERIFY( iter->path() == p/"d1/d2" );
+ ++iter;
+ VERIFY( iter == fs::recursive_directory_iterator() );
+
+ // Test inaccessible directory.
+ permissions(p, fs::perms::none, ec);
+ VERIFY( !ec );
+ iter = fs::recursive_directory_iterator(p, ec);
+ VERIFY( ec );
+ VERIFY( iter != fs::recursive_directory_iterator() );
+
+ // Test inaccessible directory, skipping permission denied.
+ const auto opts = fs::directory_options::skip_permission_denied;
+ iter = fs::recursive_directory_iterator(p, opts, ec);
+ VERIFY( !ec );
+ VERIFY( iter == fs::recursive_directory_iterator() );
+
+ // Test inaccessible sub-directory.
+ permissions(p, fs::perms::owner_all, ec);
+ VERIFY( !ec );
+ permissions(p/"d1/d2", fs::perms::none, ec);
+ VERIFY( !ec );
+ iter = fs::recursive_directory_iterator(p, ec);
+ VERIFY( !ec );
+ VERIFY( iter != fs::recursive_directory_iterator() );
+ VERIFY( iter->path() == p/"d1" );
+ ++iter; // should recurse into d1
+ VERIFY( iter->path() == p/"d1/d2" );
+ iter.increment(ec); // should fail to recurse into p/d1/d2
+ VERIFY( ec );
+
+ // Test inaccessible sub-directory, skipping permission denied.
+ iter = fs::recursive_directory_iterator(p, opts, ec);
+ VERIFY( !ec );
+ VERIFY( iter != fs::recursive_directory_iterator() );
+ VERIFY( iter->path() == p/"d1" );
+ ++iter; // should recurse into d1
+ VERIFY( iter->path() == p/"d1/d2" );
+ iter.increment(ec); // should fail to recurse into p/d1/d2, so skip it
+ VERIFY( !ec );
+ VERIFY( iter == fs::recursive_directory_iterator() );
+
+ permissions(p/"d1/d2", fs::perms::owner_all, ec);
+ remove_all(p, ec);
+}
+
+int
+main()
+{
+ test01();
+}
