Re: [PATCH] libstdc++: retry removal of dir entries if dir removal fails

[Alexandre Oliva <oliva@adacore.com>]

On Jun 27, 2022, Alexandre Oliva <oliva@adacore.com> wrote:

> I see two potential ways to avoid this:

Another possibility occurred to me: seeking back to the entry we're
about to remove, before removing it.  Then, POSIX-compliant
implementations will just skip the removed entry and find the next one,
while RTEMS will find the next entry at the spot where the removed entry
used to be.

It is syscall-heavier, and it may invoke O(n^2) behavior for each
directory in remove_all, since prev_pos is quite likely to always hold
the initial offset, requiring scanning past more and more removed
entries after each removal, so I don't submit this formally for
inclusion, but post it FTR.  I've only confirmed that it solves the
problem on RTEMS, passing libstdc++ filesystem test, but I haven't
tested it further.


diff --git a/libstdc++-v3/src/c++17/fs_dir.cc b/libstdc++-v3/src/c++17/fs_dir.cc
index 2258399da2587..43e2d9678eae5 100644
--- a/libstdc++-v3/src/c++17/fs_dir.cc
+++ b/libstdc++-v3/src/c++17/fs_dir.cc
@@ -65,6 +65,7 @@ struct fs::_Dir : _Dir_base
   // Reports errors by setting ec.
   bool advance(bool skip_permission_denied, error_code& ec) noexcept
   {
+    prev_pos = posix::telldir(dirp);
     if (const auto entp = _Dir_base::advance(skip_permission_denied, ec))
       {
 	auto name = path;
@@ -146,6 +147,12 @@ struct fs::_Dir : _Dir_base
   bool
   do_unlink(bool is_directory, error_code& ec) const noexcept
   {
+    // On some systems, removing the just-read entry causes the next
+    // readdir to skip the entry that comes after it.  That's not
+    // POSIX-compliant, but we can work around this problem by moving
+    // back to the position of the last-read entry, as if it was to be
+    // read again next, before removing it.
+    posix::seekdir(dirp, prev_pos);
 #if _GLIBCXX_HAVE_UNLINKAT
     const auto atp = current();
     if (::unlinkat(atp.dir(), atp.path_at_dir(),
@@ -176,6 +183,7 @@ struct fs::_Dir : _Dir_base
 
   fs::path		path; // Empty if only using unlinkat with file descr.
   directory_entry	entry;
+  long			prev_pos;
 };
 
 namespace
diff --git a/libstdc++-v3/src/filesystem/dir-common.h b/libstdc++-v3/src/filesystem/dir-common.h
index 228fab55afbcf..6174a8ef3c228 100644
--- a/libstdc++-v3/src/filesystem/dir-common.h
+++ b/libstdc++-v3/src/filesystem/dir-common.h
@@ -55,6 +55,8 @@ using char_type = wchar_t;
 using DIR = ::_WDIR;
 using dirent = _wdirent;
 inline DIR* opendir(const wchar_t* path) { return ::_wopendir(path); }
+inline long telldir(DIR* dir) { ::_wtelldir(dir); }
+inline void seekdir(DIR *dir, long loc) { ::_wseekdir(dir, loc); }
 inline dirent* readdir(DIR* dir) { return ::_wreaddir(dir); }
 inline int closedir(DIR* dir) { return ::_wclosedir(dir); }
 #elif defined _GLIBCXX_HAVE_DIRENT_H
@@ -64,6 +66,8 @@ typedef struct ::dirent dirent;
 using ::opendir;
 using ::readdir;
 using ::closedir;
+using ::telldir;
+using ::seekdir;
 #else
 using char_type = char;
 struct dirent { const char* d_name; };
@@ -71,6 +75,8 @@ struct DIR { };
 inline DIR* opendir(const char*) { return nullptr; }
 inline dirent* readdir(DIR*) { return nullptr; }
 inline int closedir(DIR*) { return -1; }
+inline long telldir(DIR *) { return -1; }
+inline void seekdir(DIR *, long) { }
 #undef _GLIBCXX_HAVE_DIRFD
 #undef _GLIBCXX_HAVE_UNLINKAT
 #endif


-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>