From 429ee11aa39ba700510d789b446d74b65e202754 Mon Sep 17 00:00:00 2001 From: Jonathan Wakely Date: Wed, 23 Sep 2015 12:26:45 +0100 Subject: [PATCH] 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. From-SVN: r228042 --- libstdc++-v3/ChangeLog | 28 +++++ libstdc++-v3/acinclude.m4 | 2 +- libstdc++-v3/config.h.in | 6 +- libstdc++-v3/configure | 2 +- libstdc++-v3/include/experimental/fs_dir.h | 34 ++++-- libstdc++-v3/src/filesystem/dir.cc | 108 ++++++++---------- .../iterators/directory_iterator.cc | 77 +++++++++++++ .../iterators/recursive_directory_iterator.cc | 104 +++++++++++++++++ 8 files changed, 286 insertions(+), 75 deletions(-) create mode 100644 libstdc++-v3/testsuite/experimental/filesystem/iterators/directory_iterator.cc create mode 100644 libstdc++-v3/testsuite/experimental/filesystem/iterators/recursive_directory_iterator.cc diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog index a8dc5eba381..578c5510b01 100644 --- a/libstdc++-v3/ChangeLog +++ b/libstdc++-v3/ChangeLog @@ -1,5 +1,33 @@ 2015-09-23 Jonathan Wakely + * 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. + * src/filesystem/ops.cc (is_dot, is_dotdot): Define new helpers. (create_directories): Fix error handling. * testsuite/experimental/filesystem/operations/create_directories.cc: diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4 index c133c25407b..4b031f7117f 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/config.h.in b/libstdc++-v3/config.h.in index 58eef93f5ec..8ae1c5bfe69 100644 --- a/libstdc++-v3/config.h.in +++ b/libstdc++-v3/config.h.in @@ -378,6 +378,9 @@ /* Define to 1 if you have the `strtold' function. */ #undef HAVE_STRTOLD +/* Define to 1 if `d_type' is a member of `struct dirent'. */ +#undef HAVE_STRUCT_DIRENT_D_TYPE + /* Define if strxfrm_l is available in . */ #undef HAVE_STRXFRM_L @@ -741,9 +744,6 @@ /* Define if gthreads library is available. */ #undef _GLIBCXX_HAS_GTHREADS -/* Define to 1 if `d_type' is a member of `struct dirent'. */ -#undef _GLIBCXX_HAVE_STRUCT_DIRENT_D_TYPE - /* Define to 1 if a full hosted library is built, or 0 if freestanding. */ #undef _GLIBCXX_HOSTED diff --git a/libstdc++-v3/configure b/libstdc++-v3/configure index dea0a2d3506..73d45b1a680 100755 --- a/libstdc++-v3/configure +++ b/libstdc++-v3/configure @@ -79165,7 +79165,7 @@ fi if test $glibcxx_cv_dirent_d_type = yes; then -$as_echo "#define _GLIBCXX_HAVE_STRUCT_DIRENT_D_TYPE 1" >>confdefs.h +$as_echo "#define HAVE_STRUCT_DIRENT_D_TYPE 1" >>confdefs.h fi { $as_echo "$as_me:${as_lineno-$LINENO}: result: $glibcxx_cv_dirent_d_type" >&5 diff --git a/libstdc++-v3/include/experimental/fs_dir.h b/libstdc++-v3/include/experimental/fs_dir.h index d46d41bfb02..0c5253fb62d 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; @@ -221,6 +219,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 inline directory_iterator 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; @@ -307,6 +311,14 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11 inline recursive_directory_iterator 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) diff --git a/libstdc++-v3/src/filesystem/dir.cc b/libstdc++-v3/src/filesystem/dir.cc index 016a78dc91b..bce751c3fb1 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 - make_shared_dir(fs::_Dir&& dir) - { - if (dir.dirp) - return std::make_shared(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(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(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 00000000000..56b808d3252 --- /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 +// . + +// { dg-options "-std=gnu++11 -lstdc++fs" } +// { dg-require-filesystem-ts "" } + +#include +#include +#include + +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 00000000000..9424c80ab83 --- /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 +// . + +// { dg-options "-std=gnu++11 -lstdc++fs" } +// { dg-require-filesystem-ts "" } + +#include +#include +#include + +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(); +} -- 2.30.2