From patchwork Tue Jun 28 11:12:12 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jonathan Wakely X-Patchwork-Id: 1649563 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=FoClK4eU; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4LXRhQ5cXMz9sFx for ; Wed, 29 Jun 2022 00:25:26 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id CAE7238F6542 for ; Tue, 28 Jun 2022 14:25:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org CAE7238F6542 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1656426322; bh=P73mZIFUYsZfkWuOOSF82mMSORumNMjdQN6lL/P1DuQ=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=FoClK4eUGBmyW+SeP6QuDLOtiYr/6XeAfmn8k2Qwp04sgzrEPJUXZM28z1rTaji1Z xfyKEJnRHLTlNcOdJi1mYgkJ+0LGIUUx0l86+vcQen3uOiOnWFgldofkTjbkHJvtzB oDZgiZaiQZmzVD6Pr/xRkz56fSjfVeXsIJKRbgoc= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id D624C38346A8 for ; Tue, 28 Jun 2022 11:12:16 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D624C38346A8 Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-150-axLGbAokO3O5RSP1AmtPHg-1; Tue, 28 Jun 2022 07:12:13 -0400 X-MC-Unique: axLGbAokO3O5RSP1AmtPHg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 3056F38164CC; Tue, 28 Jun 2022 11:12:13 +0000 (UTC) Received: from localhost (unknown [10.33.36.212]) by smtp.corp.redhat.com (Postfix) with ESMTP id E853EC15D42; Tue, 28 Jun 2022 11:12:12 +0000 (UTC) To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Subject: [committed] libstdc++: Improve directory iterator abstractions for openat Date: Tue, 28 Jun 2022 12:12:12 +0100 Message-Id: <20220628111212.206509-1-jwakely@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Jonathan Wakely via Gcc-patches From: Jonathan Wakely Reply-To: Jonathan Wakely Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" Tested powerpc64le-linux, pushed to trunk. -- >8 -- 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 tiughtly 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. --- libstdc++-v3/src/c++17/fs_dir.cc | 29 ++++++----- libstdc++-v3/src/filesystem/dir-common.h | 66 +++++++++++++++++------- libstdc++-v3/src/filesystem/dir.cc | 21 ++++---- 3 files changed, 73 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..4844b1ac453 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,50 @@ 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 char* p) noexcept + : pathname(p), dir_fd(fdcwd()), offset(0) + { } + + _At_path(int fd, const char* p, size_t offset) noexcept + : pathname(p), dir_fd(fd), offset(offset) + { } + + const char* path() const noexcept { return pathname; } + + int dir() const noexcept { return dir_fd; } + const char* 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 +182,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 +203,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 +227,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 +250,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()); }