From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2140) id 62883385AE61; Wed, 29 Jun 2022 03:13:31 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 62883385AE61 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Alexandre Oliva To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc(refs/users/aoliva/heads/testme)] libstdc++: auto-rewind dirs for remove_all X-Act-Checkin: gcc X-Git-Author: Alexandre Oliva X-Git-Refname: refs/users/aoliva/heads/testme X-Git-Oldrev: eabd44a02529f7921a4997240aaaefb15f1ef81f X-Git-Newrev: 7843d77e49529939c1bab3c2c95c282eb5d7cb67 Message-Id: <20220629031331.62883385AE61@sourceware.org> Date: Wed, 29 Jun 2022 03:13:31 +0000 (GMT) X-BeenThere: libstdc++-cvs@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Jun 2022 03:13:31 -0000 https://gcc.gnu.org/g:7843d77e49529939c1bab3c2c95c282eb5d7cb67 commit 7843d77e49529939c1bab3c2c95c282eb5d7cb67 Author: Alexandre Oliva Date: Tue Jun 28 23:57:52 2022 -0300 libstdc++: auto-rewind dirs for remove_all On some target systems (e.g. rtems6.0), removing directory components while iterating over directory entries may cause some of the directory entries to be skipped, which prevents the removal of the parent directory from succeeding. Advancing the iterator before removing a member proved not to be enough, so I've arranged the directory reading implementation to implicitly rewind a directory when reaching the end, as long as there were any entries and they have all been removed. Rewinding will only ever take place for users of recursive_directory_iterator::__erase, and thus of _Dir::do_unlink, and since __erase is a private member function, it can only be used by the remove_all functions because they're granted friendship. for libstdc++-v3/ChangeLog * src/filesystem/dir-common.h (__gnu_posix::rewinddir): Define. * src/c++17/fs_dir.cc (fs::_Dir::auto_rewind): New enum. (fs::_Dir::rewind): New data member. (fs::_Dir::advance): Update rewind, rewinddir if found entries have all been removed. (fs::_Dir::do_unlink): Drop const. Update rewind. (fs::_Dir::unlink, fs::_Dir::rmdir): Drop const. Diff: --- libstdc++-v3/src/c++17/fs_dir.cc | 75 ++++++++++++++++++++++++++++++-- libstdc++-v3/src/filesystem/dir-common.h | 3 ++ 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/libstdc++-v3/src/c++17/fs_dir.cc b/libstdc++-v3/src/c++17/fs_dir.cc index 2258399da25..6f535c95a32 100644 --- a/libstdc++-v3/src/c++17/fs_dir.cc +++ b/libstdc++-v3/src/c++17/fs_dir.cc @@ -44,6 +44,30 @@ template class std::__shared_ptr; struct fs::_Dir : _Dir_base { + // On some systems, removing a directory entry causes readdir to + // skip the subsequent entry. This state machine enables remove_all + // to not miss entries, by arranging for directories to + // automatically rewind iff every visited entry got removed, hitting + // the end only when no entries are found. We start with + // no_entries, and advance moves from that to found_entry, that + // do_unlink changes to removed_entry. If advance is called again + // without unlink, it moves to remaining_entry instead, so that we + // will know not to rewind (otherwise we'd visit the same entry + // again). OTOH, if advance doesn't find any entry and the state is + // removed_entry, it resets to no_entries and rewinds; at other + // states, no rewind takes place. Skipped entries (dot and dotdot + // and permission-denied) do not affect the state machine: they're + // skipped every time anyway. It is envisioned that, with a + // reliable detection mechanism for systems that don't need + // rewinding, rewind could be initialized to remaining_entry, that + // is a final state that prevents rewinding. + enum auto_rewind { + no_entries, + found_entry, + removed_entry, + remaining_entry + }; + _Dir(const fs::path& p, bool skip_permission_denied, bool nofollow, [[maybe_unused]] bool filename_only, error_code& ec) : _Dir_base(p.c_str(), skip_permission_denied, nofollow, ec) @@ -67,6 +91,21 @@ struct fs::_Dir : _Dir_base { if (const auto entp = _Dir_base::advance(skip_permission_denied, ec)) { + switch (rewind) + { + case no_entries: + case removed_entry: + rewind = found_entry; + break; + case found_entry: + rewind = remaining_entry; + break; + case remaining_entry: + break; + default: + __builtin_unreachable (); + break; + } auto name = path; name /= entp->d_name; file_type type = file_type::none; @@ -80,6 +119,22 @@ struct fs::_Dir : _Dir_base } else if (!ec) { + switch (rewind) + { + case removed_entry: + rewind = no_entries; + posix::rewinddir(this->dirp); + return advance (skip_permission_denied, ec); + case found_entry: + rewind = remaining_entry; + break; + case no_entries: + case remaining_entry: + break; + default: + __builtin_unreachable (); + break; + } // reached the end entry = {}; } @@ -144,8 +199,21 @@ struct fs::_Dir : _Dir_base } bool - do_unlink(bool is_directory, error_code& ec) const noexcept + do_unlink(bool is_directory, error_code& ec) noexcept { + switch (rewind) + { + case no_entries: + case removed_entry: + default: + __builtin_unreachable (); + break; + case found_entry: + rewind = removed_entry; + break; + case remaining_entry: + break; + } #if _GLIBCXX_HAVE_UNLINKAT const auto atp = current(); if (::unlinkat(atp.dir(), atp.path_at_dir(), @@ -166,16 +234,17 @@ struct fs::_Dir : _Dir_base // Remove the non-directory that this->entry refers to. bool - unlink(error_code& ec) const noexcept + unlink(error_code& ec) noexcept { return do_unlink(/* is_directory*/ false, ec); } // Remove the directory that this->entry refers to. bool - rmdir(error_code& ec) const noexcept + rmdir(error_code& ec) noexcept { return do_unlink(/* is_directory*/ true, ec); } fs::path path; // Empty if only using unlinkat with file descr. directory_entry entry; + enum auto_rewind rewind = no_entries; }; namespace diff --git a/libstdc++-v3/src/filesystem/dir-common.h b/libstdc++-v3/src/filesystem/dir-common.h index 228fab55afb..7eb39ae7437 100644 --- a/libstdc++-v3/src/filesystem/dir-common.h +++ b/libstdc++-v3/src/filesystem/dir-common.h @@ -55,6 +55,7 @@ using char_type = wchar_t; using DIR = ::_WDIR; using dirent = _wdirent; inline DIR* opendir(const wchar_t* path) { return ::_wopendir(path); } +inline void rewinddir(DIR* dir) { ::_wrewinddir(dir); } inline dirent* readdir(DIR* dir) { return ::_wreaddir(dir); } inline int closedir(DIR* dir) { return ::_wclosedir(dir); } #elif defined _GLIBCXX_HAVE_DIRENT_H @@ -64,11 +65,13 @@ typedef struct ::dirent dirent; using ::opendir; using ::readdir; using ::closedir; +using ::rewinddir; #else using char_type = char; struct dirent { const char* d_name; }; struct DIR { }; inline DIR* opendir(const char*) { return nullptr; } +inline void rewinddir(DIR*) { } inline dirent* readdir(DIR*) { return nullptr; } inline int closedir(DIR*) { return -1; } #undef _GLIBCXX_HAVE_DIRFD