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 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.
This commit is contained in:
Jonathan Wakely 2022-06-27 14:43:54 +01:00
parent 835b19936b
commit 198781144f
3 changed files with 73 additions and 43 deletions

View File

@ -46,7 +46,7 @@ struct fs::_Dir : _Dir_base
{ {
_Dir(const fs::path& p, bool skip_permission_denied, bool nofollow, _Dir(const fs::path& p, bool skip_permission_denied, bool nofollow,
[[maybe_unused]] bool filename_only, error_code& ec) [[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 _GLIBCXX_HAVE_DIRFD && _GLIBCXX_HAVE_OPENAT && _GLIBCXX_HAVE_UNLINKAT
if (filename_only) if (filename_only)
@ -117,18 +117,19 @@ struct fs::_Dir : _Dir_base
return false; return false;
} }
// Return a file descriptor for the directory and current entry's path. // Return a pathname for the current directory entry, as an _At_path.
// If dirfd is available, use it and return only the filename. _Dir_base::_At_path
// Otherwise, return AT_FDCWD and return the full path. current() const noexcept
pair<int, const posix::char_type*>
dir_and_pathname() const noexcept
{ {
const fs::path& p = entry.path(); const fs::path& p = entry.path();
#if _GLIBCXX_HAVE_DIRFD && _GLIBCXX_HAVE_OPENAT #if _GLIBCXX_HAVE_DIRFD
if (!p.empty()) if (!p.empty()) [[__likely__]]
return {::dirfd(this->dirp), std::prev(p.end())->c_str()}; {
auto len = std::prev(p.end())->native().size();
return {::dirfd(this->dirp), p.c_str(), p.native().size() - len};
}
#endif #endif
return {this->fdcwd(), p.c_str()}; return p.c_str();
} }
// Create a new _Dir for the directory this->entry.path(). // 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, open_subdir(bool skip_permission_denied, bool nofollow,
error_code& ec) const noexcept error_code& ec) const noexcept
{ {
auto [dirfd, pathname] = dir_and_pathname(); _Dir_base d(current(), skip_permission_denied, nofollow, ec);
_Dir_base d(dirfd, pathname, skip_permission_denied, nofollow, ec);
// If this->path is empty, the new _Dir should have an empty path too. // 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(); const fs::path& p = this->path.empty() ? this->path : this->entry.path();
return _Dir(std::move(d), p); 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 do_unlink(bool is_directory, error_code& ec) const noexcept
{ {
#if _GLIBCXX_HAVE_UNLINKAT #if _GLIBCXX_HAVE_UNLINKAT
auto [dirfd, pathname] = dir_and_pathname(); const auto atp = current();
if (::unlinkat(dirfd, pathname, is_directory ? AT_REMOVEDIR : 0) == -1) if (::unlinkat(atp.dir(), atp.path_at_dir(),
is_directory ? AT_REMOVEDIR : 0) == -1)
{ {
ec.assign(errno, std::generic_category()); ec.assign(errno, std::generic_category());
return false; return false;

View File

@ -25,6 +25,7 @@
#ifndef _GLIBCXX_DIR_COMMON_H #ifndef _GLIBCXX_DIR_COMMON_H
#define _GLIBCXX_DIR_COMMON_H 1 #define _GLIBCXX_DIR_COMMON_H 1
#include <stdint.h> // uint32_t
#include <string.h> // strcmp #include <string.h> // strcmp
#include <errno.h> #include <errno.h>
#if _GLIBCXX_FILESYSTEM_IS_WINDOWS #if _GLIBCXX_FILESYSTEM_IS_WINDOWS
@ -91,12 +92,50 @@ is_permission_denied_error(int e)
struct _Dir_base 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, // If no error occurs then dirp is non-null,
// otherwise null (even if a permission denied error is ignored). // 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, bool skip_permission_denied, bool nofollow,
error_code& ec) noexcept error_code& ec) noexcept
: dirp(_Dir_base::openat(fd, pathname, nofollow)) : dirp(_Dir_base::openat(atp, nofollow))
{ {
if (dirp) if (dirp)
ec.clear(); 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 static bool is_dot_or_dotdot(const char* s) noexcept
{ return !strcmp(s, ".") || !strcmp(s, ".."); } { return !strcmp(s, ".") || !strcmp(s, ".."); }
@ -174,7 +203,7 @@ struct _Dir_base
} }
static posix::DIR* 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 \ #if _GLIBCXX_HAVE_FDOPENDIR && defined O_RDONLY && defined O_DIRECTORY \
&& ! _GLIBCXX_FILESYSTEM_IS_WINDOWS && ! _GLIBCXX_FILESYSTEM_IS_WINDOWS
@ -198,16 +227,17 @@ struct _Dir_base
nofollow = false; nofollow = false;
#endif #endif
int fd;
#if _GLIBCXX_HAVE_OPENAT && defined AT_FDCWD #if _GLIBCXX_HAVE_OPENAT
fd = ::openat(fd, pathname, flags); fd = ::openat(atp.dir(), atp.path_at_dir(), flags);
#else #else
// If we cannot use openat, there's no benefit to using posix::open unless // 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. // we will use O_NOFOLLOW, so just use the simpler posix::opendir.
if (!nofollow) if (!nofollow)
return posix::opendir(pathname); return posix::opendir(atp.path());
fd = ::open(pathname, flags); fd = ::open(atp.path(), flags);
#endif #endif
if (fd == -1) if (fd == -1)
@ -220,7 +250,7 @@ struct _Dir_base
errno = err; errno = err;
return nullptr; return nullptr;
#else #else
return posix::opendir(pathname); return posix::opendir(atp.path());
#endif #endif
} }

View File

@ -53,7 +53,7 @@ struct fs::_Dir : std::filesystem::_Dir_base
{ {
_Dir(const fs::path& p, bool skip_permission_denied, bool nofollow, _Dir(const fs::path& p, bool skip_permission_denied, bool nofollow,
error_code& ec) 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) if (!ec)
path = p; path = p;
@ -113,17 +113,17 @@ struct fs::_Dir : std::filesystem::_Dir_base
return false; return false;
} }
// Return a file descriptor for the directory and current entry's path. // Return a pathname for the current directory entry, as an _At_path.
// If dirfd is available, use it and return only the filename. _Dir_base::_At_path
// Otherwise, return AT_FDCWD and return the full path. current() const noexcept
pair<int, const posix::char_type*>
dir_and_pathname() const noexcept
{ {
const fs::path& p = entry.path(); const fs::path& p = entry.path();
#if _GLIBCXX_HAVE_DIRFD && _GLIBCXX_HAVE_OPENAT #if _GLIBCXX_HAVE_DIRFD
return {::dirfd(this->dirp), std::prev(p.end())->c_str()}; 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 #endif
return {this->fdcwd(), p.c_str()};
} }
// Create a new _Dir for the directory this->entry.path(). // 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, open_subdir(bool skip_permission_denied, bool nofollow,
error_code& ec) noexcept error_code& ec) noexcept
{ {
auto [dirfd, pathname] = dir_and_pathname(); _Dir_base d(current(), skip_permission_denied, nofollow, ec);
_Dir_base d(dirfd, pathname, skip_permission_denied, nofollow, ec);
return _Dir(std::move(d), entry.path()); return _Dir(std::move(d), entry.path());
} }