public inbox for libstdc++-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-8661] libstdc++: Improve directory iterator abstractions for openat
@ 2022-08-03 13:47 Jonathan Wakely
  0 siblings, 0 replies; only message in thread
From: Jonathan Wakely @ 2022-08-03 13:47 UTC (permalink / raw)
  To: gcc-cvs, libstdc++-cvs

https://gcc.gnu.org/g:3df2f035871e913b94a35d4b5c12d37828f313f7

commit r12-8661-g3df2f035871e913b94a35d4b5c12d37828f313f7
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jun 27 14:43:54 2022 +0100

    libstdc++: Improve directory iterator abstractions for openat
    
    Currently the _Dir::open_subdir function decides whether to construct a
    _Dir_base with just a pathname, or a file descriptor and pathname. But
    that means it is tightly coupled to the implementation of
    _Dir_base::openat, which is what actually decides whether to use a file
    descriptor or not. If the derived class passes a file descriptor and
    filename, but the base class expects a full path and ignores the file
    descriptor, then recursive_directory_iterator cannot recurse.
    
    This change introduces a new type that provides the union of all the
    information available to the derived class (the full pathname, as well
    as a file descriptor for a directory and another pathname relative to
    that directory). This allows the derived class to be agnostic to how the
    base class will use that information.
    
    libstdc++-v3/ChangeLog:
    
            * src/c++17/fs_dir.cc (_Dir::dir_and_pathname):: Replace with
            current() returning _At_path.
            (_Dir::_Dir, _Dir::open_subdir, _Dir::do_unlink): Adjust.
            * src/filesystem/dir-common.h (_Dir_base::_At_path): New class.
            (_Dir_base::_Dir_Base, _Dir_base::openat): Use _At_path.
            * src/filesystem/dir.cc (_Dir::dir_and_pathname): Replace with
            current() returning _At_path.
            (_Dir::_Dir, _Dir::open_subdir): Adjust.
    
    (cherry picked from commit 198781144f33b0ef17dd2094580b5c77ad97d6e8)

Diff:
---
 libstdc++-v3/src/c++17/fs_dir.cc         | 29 ++++++-------
 libstdc++-v3/src/filesystem/dir-common.h | 70 ++++++++++++++++++++++++--------
 libstdc++-v3/src/filesystem/dir.cc       | 21 +++++-----
 3 files changed, 77 insertions(+), 43 deletions(-)

diff --git a/libstdc++-v3/src/c++17/fs_dir.cc b/libstdc++-v3/src/c++17/fs_dir.cc
index 025317b0a08..2258399da25 100644
--- a/libstdc++-v3/src/c++17/fs_dir.cc
+++ b/libstdc++-v3/src/c++17/fs_dir.cc
@@ -46,7 +46,7 @@ struct fs::_Dir : _Dir_base
 {
   _Dir(const fs::path& p, bool skip_permission_denied, bool nofollow,
        [[maybe_unused]] bool filename_only, error_code& ec)
-  : _Dir_base(fdcwd(), p.c_str(), skip_permission_denied, nofollow, ec)
+  : _Dir_base(p.c_str(), skip_permission_denied, nofollow, ec)
   {
 #if _GLIBCXX_HAVE_DIRFD && _GLIBCXX_HAVE_OPENAT && _GLIBCXX_HAVE_UNLINKAT
     if (filename_only)
@@ -117,18 +117,19 @@ struct fs::_Dir : _Dir_base
     return false;
   }
 
-  // Return a file descriptor for the directory and current entry's path.
-  // If dirfd is available, use it and return only the filename.
-  // Otherwise, return AT_FDCWD and return the full path.
-  pair<int, const posix::char_type*>
-  dir_and_pathname() const noexcept
+  // Return a pathname for the current directory entry, as an _At_path.
+  _Dir_base::_At_path
+  current() const noexcept
   {
     const fs::path& p = entry.path();
-#if _GLIBCXX_HAVE_DIRFD && _GLIBCXX_HAVE_OPENAT
-    if (!p.empty())
-      return {::dirfd(this->dirp), std::prev(p.end())->c_str()};
+#if _GLIBCXX_HAVE_DIRFD
+    if (!p.empty()) [[__likely__]]
+      {
+	auto len = std::prev(p.end())->native().size();
+	return {::dirfd(this->dirp), p.c_str(), p.native().size() - len};
+      }
 #endif
-    return {this->fdcwd(), p.c_str()};
+    return p.c_str();
   }
 
   // Create a new _Dir for the directory this->entry.path().
@@ -136,8 +137,7 @@ struct fs::_Dir : _Dir_base
   open_subdir(bool skip_permission_denied, bool nofollow,
 	      error_code& ec) const noexcept
   {
-    auto [dirfd, pathname] = dir_and_pathname();
-    _Dir_base d(dirfd, pathname, skip_permission_denied, nofollow, ec);
+    _Dir_base d(current(), skip_permission_denied, nofollow, ec);
     // If this->path is empty, the new _Dir should have an empty path too.
     const fs::path& p = this->path.empty() ? this->path : this->entry.path();
     return _Dir(std::move(d), p);
@@ -147,8 +147,9 @@ struct fs::_Dir : _Dir_base
   do_unlink(bool is_directory, error_code& ec) const noexcept
   {
 #if _GLIBCXX_HAVE_UNLINKAT
-    auto [dirfd, pathname] = dir_and_pathname();
-    if (::unlinkat(dirfd, pathname, is_directory ? AT_REMOVEDIR : 0) == -1)
+    const auto atp = current();
+    if (::unlinkat(atp.dir(), atp.path_at_dir(),
+		   is_directory ? AT_REMOVEDIR : 0) == -1)
       {
 	ec.assign(errno, std::generic_category());
 	return false;
diff --git a/libstdc++-v3/src/filesystem/dir-common.h b/libstdc++-v3/src/filesystem/dir-common.h
index 669780ea23f..228fab55afb 100644
--- a/libstdc++-v3/src/filesystem/dir-common.h
+++ b/libstdc++-v3/src/filesystem/dir-common.h
@@ -25,6 +25,7 @@
 #ifndef _GLIBCXX_DIR_COMMON_H
 #define _GLIBCXX_DIR_COMMON_H 1
 
+#include <stdint.h>  // uint32_t
 #include <string.h>  // strcmp
 #include <errno.h>
 #if _GLIBCXX_FILESYSTEM_IS_WINDOWS
@@ -91,12 +92,54 @@ is_permission_denied_error(int e)
 
 struct _Dir_base
 {
+  // As well as the full pathname (including the directory iterator's path)
+  // this type contains a file descriptor for a directory and a second pathname
+  // relative to that directory. The file descriptor and relative pathname
+  // can be used with POSIX openat and unlinkat.
+  struct _At_path
+  {
+    // No file descriptor given, so interpret the pathname relative to the CWD.
+    _At_path(const posix::char_type* p) noexcept
+    : pathname(p), dir_fd(fdcwd()), offset(0)
+    { }
+
+    _At_path(int fd, const posix::char_type* p, size_t offset) noexcept
+    : pathname(p), dir_fd(fd), offset(offset)
+    { }
+
+    const posix::char_type*
+    path() const noexcept { return pathname; }
+
+    int
+    dir() const noexcept { return dir_fd; }
+
+    const posix::char_type*
+    path_at_dir() const noexcept { return pathname + offset; }
+
+  private:
+    const posix::char_type* pathname; // Full path relative to CWD.
+    int dir_fd; // A directory descriptor (either the parent dir, or AT_FDCWD).
+    uint32_t offset; // Offset into pathname for the part relative to dir_fd.
+
+    // Special value representing the current working directory.
+    // Not a valid file descriptor for an open directory stream.
+    static constexpr int
+    fdcwd() noexcept
+    {
+#ifdef AT_FDCWD
+      return AT_FDCWD;
+#else
+      return -1; // Use invalid fd if AT_FDCWD isn't supported.
+#endif
+    }
+  };
+
   // If no error occurs then dirp is non-null,
   // otherwise null (even if a permission denied error is ignored).
-  _Dir_base(int fd, const posix::char_type* pathname,
+  _Dir_base(const _At_path& atp,
 	    bool skip_permission_denied, bool nofollow,
 	    error_code& ec) noexcept
-  : dirp(_Dir_base::openat(fd, pathname, nofollow))
+  : dirp(_Dir_base::openat(atp, nofollow))
   {
     if (dirp)
       ec.clear();
@@ -143,16 +186,6 @@ struct _Dir_base
       }
   }
 
-  static constexpr int
-  fdcwd() noexcept
-  {
-#ifdef AT_FDCWD
-    return AT_FDCWD;
-#else
-    return -1; // Use invalid fd if AT_FDCWD isn't supported.
-#endif
-  }
-
   static bool is_dot_or_dotdot(const char* s) noexcept
   { return !strcmp(s, ".") || !strcmp(s, ".."); }
 
@@ -174,7 +207,7 @@ struct _Dir_base
   }
 
   static posix::DIR*
-  openat(int fd, const posix::char_type* pathname, bool nofollow)
+  openat(const _At_path& atp, bool nofollow)
   {
 #if _GLIBCXX_HAVE_FDOPENDIR && defined O_RDONLY && defined O_DIRECTORY \
     && ! _GLIBCXX_FILESYSTEM_IS_WINDOWS
@@ -198,16 +231,17 @@ struct _Dir_base
     nofollow = false;
 #endif
 
+    int fd;
 
-#if _GLIBCXX_HAVE_OPENAT && defined AT_FDCWD
-    fd = ::openat(fd, pathname, flags);
+#if _GLIBCXX_HAVE_OPENAT
+    fd = ::openat(atp.dir(), atp.path_at_dir(), flags);
 #else
     // If we cannot use openat, there's no benefit to using posix::open unless
     // we will use O_NOFOLLOW, so just use the simpler posix::opendir.
     if (!nofollow)
-      return posix::opendir(pathname);
+      return posix::opendir(atp.path());
 
-    fd = ::open(pathname, flags);
+    fd = ::open(atp.path(), flags);
 #endif
 
     if (fd == -1)
@@ -220,7 +254,7 @@ struct _Dir_base
     errno = err;
     return nullptr;
 #else
-    return posix::opendir(pathname);
+    return posix::opendir(atp.path());
 #endif
   }
 
diff --git a/libstdc++-v3/src/filesystem/dir.cc b/libstdc++-v3/src/filesystem/dir.cc
index e64489162e5..a66a6773580 100644
--- a/libstdc++-v3/src/filesystem/dir.cc
+++ b/libstdc++-v3/src/filesystem/dir.cc
@@ -53,7 +53,7 @@ struct fs::_Dir : std::filesystem::_Dir_base
 {
   _Dir(const fs::path& p, bool skip_permission_denied, bool nofollow,
        error_code& ec)
-  : _Dir_base(this->fdcwd(), p.c_str(), skip_permission_denied, nofollow, ec)
+  : _Dir_base(p.c_str(), skip_permission_denied, nofollow, ec)
   {
     if (!ec)
       path = p;
@@ -113,17 +113,17 @@ struct fs::_Dir : std::filesystem::_Dir_base
     return false;
   }
 
-  // Return a file descriptor for the directory and current entry's path.
-  // If dirfd is available, use it and return only the filename.
-  // Otherwise, return AT_FDCWD and return the full path.
-  pair<int, const posix::char_type*>
-  dir_and_pathname() const noexcept
+  // Return a pathname for the current directory entry, as an _At_path.
+  _Dir_base::_At_path
+  current() const noexcept
   {
     const fs::path& p = entry.path();
-#if _GLIBCXX_HAVE_DIRFD && _GLIBCXX_HAVE_OPENAT
-    return {::dirfd(this->dirp), std::prev(p.end())->c_str()};
+#if _GLIBCXX_HAVE_DIRFD
+    auto len = std::prev(p.end())->native().size();
+    return {::dirfd(this->dirp), p.c_str(), p.native().size() - len};
+#else
+    return p.c_str();
 #endif
-    return {this->fdcwd(), p.c_str()};
   }
 
   // Create a new _Dir for the directory this->entry.path().
@@ -131,8 +131,7 @@ struct fs::_Dir : std::filesystem::_Dir_base
   open_subdir(bool skip_permission_denied, bool nofollow,
 	      error_code& ec) noexcept
   {
-    auto [dirfd, pathname] = dir_and_pathname();
-    _Dir_base d(dirfd, pathname, skip_permission_denied, nofollow, ec);
+    _Dir_base d(current(), skip_permission_denied, nofollow, ec);
     return _Dir(std::move(d), entry.path());
   }


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-08-03 13:47 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-03 13:47 [gcc r12-8661] libstdc++: Improve directory iterator abstractions for openat Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).