From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2181) id 6CE3638983B6; Wed, 3 Aug 2022 13:47:24 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6CE3638983B6 MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="utf-8" From: Jonathan Wakely To: gcc-cvs@gcc.gnu.org, libstdc++-cvs@gcc.gnu.org Subject: [gcc r12-8661] libstdc++: Improve directory iterator abstractions for openat X-Act-Checkin: gcc X-Git-Author: Jonathan Wakely X-Git-Refname: refs/heads/releases/gcc-12 X-Git-Oldrev: 1a9681e60964c7f7ce0892e14745e6dcf6100157 X-Git-Newrev: 3df2f035871e913b94a35d4b5c12d37828f313f7 Message-Id: <20220803134724.6CE3638983B6@sourceware.org> Date: Wed, 3 Aug 2022 13:47:24 +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, 03 Aug 2022 13:47:24 -0000 https://gcc.gnu.org/g:3df2f035871e913b94a35d4b5c12d37828f313f7 commit r12-8661-g3df2f035871e913b94a35d4b5c12d37828f313f7 Author: Jonathan Wakely 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 - 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 // uint32_t #include // strcmp #include #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 - 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()); }