From 011a66b5386f93a80a2963c393c01e0aaf411872 Mon Sep 17 00:00:00 2001 From: Elias Batek Date: Sun, 23 Mar 2025 09:22:43 +0100 Subject: [PATCH 1/6] Make `dirEntries` more robust to changes of the working directory --- std/file.d | 228 ++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 200 insertions(+), 28 deletions(-) diff --git a/std/file.d b/std/file.d index 4ad6400cb32..c45d09ac288 100644 --- a/std/file.d +++ b/std/file.d @@ -3652,13 +3652,25 @@ version (StdDdoc) +/ this(return scope string path); + /++ + Constructs a `DirEntry` for the given file (or directory). + + Params: + path = The file (or directory) to get a DirEntry for. + prefix = A prefix to chomp off the path when querying the name of the DirEntry. + + Throws: + $(LREF FileException) if the file does not exist. + +/ + this(return scope string path, return scope string prefix); + version (Windows) { - private this(string path, in WIN32_FIND_DATAW *fd); + private this(string path, in WIN32_FIND_DATAW *fd, string prefix = null); } else version (Posix) { - private this(string path, core.sys.posix.dirent.dirent* fd); + private this(string path, core.sys.posix.dirent.dirent* fd, string prefix = null); } /++ @@ -3675,6 +3687,13 @@ assert(de2.name == "/usr/share/include"); +/ @property string name() const return scope; + /++ + Returns the path to the file represented by this `DirEntry`. + + Unlike `name`, this property returns the internal name as-is, + potentially starting with an unexpected prefix. + +/ + @property string nameWithPrefix() const return scope; /++ Returns whether the file represented by this `DirEntry` is a @@ -3810,6 +3829,11 @@ else version (Windows) { @safe: public: + /+ + Note for Phobos v3: + This has caused user confusion in cases where nested directory trees are interated. + See for details. + +/ alias name this; this(return scope string path) @@ -3831,13 +3855,21 @@ else version (Windows) } } - private this(string path, WIN32_FIND_DATAW *fd) @trusted + this(return scope string path, return scope string prefix) + { + _namePrefix = prefix; + this(path); + } + + private this(string path, WIN32_FIND_DATAW *fd, string prefix = null) @trusted { import core.stdc.wchar_ : wcslen; import std.conv : to; import std.datetime.systime : FILETIMEToSysTime; import std.path : buildPath; + _namePrefix = prefix; + fd.cFileName[$ - 1] = 0; size_t clength = wcslen(&fd.cFileName[0]); @@ -3850,10 +3882,21 @@ else version (Windows) } @property string name() const pure nothrow return scope + { + import std.string : chompPrefix; + return _name.chompPrefix(_namePrefix); + } + + @property string nameWithPrefix() const pure nothrow return scope { return _name; } + private @property string namePrefix() const pure nothrow return scope + { + return _namePrefix; + } + @property bool isDir() const pure nothrow scope { return (attributes & FILE_ATTRIBUTE_DIRECTORY) != 0; @@ -3904,6 +3947,7 @@ else version (Windows) private: string _name; /// The file or directory represented by this DirEntry. + string _namePrefix; /// A prefix to be chomped off the name (e.g. parent directories of an absolute path). SysTime _timeCreated; /// The time when the file was created. SysTime _timeLastAccessed; /// The time when the file was last accessed. @@ -3919,6 +3963,11 @@ else version (Posix) { @safe: public: + /+ + Note for Phobos v3: + This has caused user confusion in cases where nested directory trees are interated. + See for details. + +/ alias name this; this(return scope string path) @@ -3933,10 +3982,12 @@ else version (Posix) _dTypeSet = false; } - private this(string path, core.sys.posix.dirent.dirent* fd) @safe + private this(string path, core.sys.posix.dirent.dirent* fd, string prefix = null) @safe { import std.path : buildPath; + _namePrefix = prefix; + static if (is(typeof(fd.d_namlen))) immutable len = fd.d_namlen; else @@ -3973,10 +4024,21 @@ else version (Posix) } @property string name() const pure nothrow return scope + { + import std.string : chompPrefix; + return _name.chompPrefix(_namePrefix); + } + + @property string nameWithPrefix() const pure nothrow return scope { return _name; } + private @property string namePrefix() const pure nothrow return scope + { + return _namePrefix; + } + @property bool isDir() scope { _ensureStatOrLStatDone(); @@ -4107,6 +4169,7 @@ else version (Posix) } string _name; /// The file or directory represented by this DirEntry. + string _namePrefix; /// A prefix to be chomped off the name (e.g. parent directories of an absolute path). stat_t _statBuf = void; /// The result of stat(). uint _lstatMode; /// The stat mode from lstat(). @@ -4638,7 +4701,7 @@ private struct DirIteratorImpl DirEntry _cur; DirHandle[] _stack; DirEntry[] _stashed; //used in depth first mode - string _pathPrefix = null; + string _namePrefix = null; //stack helpers void pushExtra(DirEntry de) @@ -4696,7 +4759,6 @@ private struct DirIteratorImpl bool toNext(bool fetch, scope WIN32_FIND_DATAW* findinfo) @trusted { import core.stdc.wchar_ : wcscmp; - import std.string : chompPrefix; if (fetch) { @@ -4713,7 +4775,7 @@ private struct DirIteratorImpl popDirStack(); return false; } - _cur = DirEntry(_stack[$-1].dirpath.chompPrefix(_pathPrefix), findinfo); + _cur = DirEntry(_stack[$-1].dirpath, findinfo, _namePrefix); return true; } @@ -4758,8 +4820,6 @@ private struct DirIteratorImpl bool next() @trusted { - import std.string : chompPrefix; - if (_stack.length == 0) return false; @@ -4769,7 +4829,7 @@ private struct DirIteratorImpl if (core.stdc.string.strcmp(&fdata.d_name[0], ".") && core.stdc.string.strcmp(&fdata.d_name[0], "..")) { - _cur = DirEntry(_stack[$-1].dirpath.chompPrefix(_pathPrefix), fdata); + _cur = DirEntry(_stack[$-1].dirpath, fdata, _namePrefix); return true; } } @@ -4811,7 +4871,7 @@ private struct DirIteratorImpl pathname = pathname.absolutePath; const offset = pathnameAbs.length - pathnameRel.length; - _pathPrefix = pathnameAbs[0 .. offset]; + _namePrefix = pathnameAbs[0 .. offset]; } if (stepIn(pathname)) @@ -4820,7 +4880,7 @@ private struct DirIteratorImpl while (mayStepIn()) { auto thisDir = _cur; - if (stepIn(_cur.name)) + if (stepIn(_cur.nameWithPrefix)) { pushExtra(thisDir); } @@ -4850,7 +4910,7 @@ private struct DirIteratorImpl while (mayStepIn()) { auto thisDir = _cur; - if (stepIn(_cur.name)) + if (stepIn(_cur.nameWithPrefix)) { pushExtra(thisDir); } @@ -4864,7 +4924,7 @@ private struct DirIteratorImpl case SpanMode.breadth: if (mayStepIn()) { - if (!stepIn(_cur.name)) + if (!stepIn(_cur.nameWithPrefix)) while (!empty && !next()){} } else @@ -4918,14 +4978,23 @@ alias DirIterator = _DirIterator!dip1000Enabled; Note: The order of returned directory entries is as it is provided by the operating system / filesystem, and may not follow any particular sorting. + Pitfall: In cases where a change of the working directory (`chdir`) can occur, + it's recommended that one either uses absolute paths + or avoids converting `DirEntry` structures to `string`. + For further details see $(LINK2 https://github.com/dlang/phobos/issues/9584, #9584 on GitHub). + Params: + Path = Type of the directory path. + Can be either a `string` or a `DirEntry`. + useDIP1000 = used to instantiate this function separately for code with and without -preview=dip1000 compiler switch, because it affects the ABI of this function. Set automatically - don't touch. path = The directory to iterate over. - If empty, the current directory will be iterated. + If an empty string (or data that implicitly converts to one) is + provided, the current directory will be iterated. pattern = Optional string with wildcards, such as $(RED "*.d"). When present, it is used to filter the @@ -5013,8 +5082,11 @@ scan(""); // For some reason, doing the same alias-to-a-template trick as with DirIterator // does not work here. -auto dirEntries(bool useDIP1000 = dip1000Enabled) - (string path, SpanMode mode, bool followSymlink = true) +// The template constraint is necessary to prevent this overload from matching +// `DirEntry`. Said type has an `alias this` member of type `string`. +auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) + (const Path path, SpanMode mode, bool followSymlink = true) +if (is(Path == string)) { return _DirIterator!useDIP1000(path, mode, followSymlink); } @@ -5113,21 +5185,13 @@ auto dirEntries(bool useDIP1000 = dip1000Enabled) // https://issues.dlang.org/show_bug.cgi?id=15146 dirEntries("", SpanMode.shallow).walkLength(); - - // https://github.com/dlang/phobos/issues/9584 - string cwd = getcwd(); - foreach (string entry; dirEntries(testdir, SpanMode.shallow)) - { - if (entry.isDir) - chdir(entry); - } - chdir(cwd); // needed for the directories to be removed } /// Ditto -auto dirEntries(bool useDIP1000 = dip1000Enabled) - (string path, string pattern, SpanMode mode, +auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) + (const Path path, string pattern, SpanMode mode, bool followSymlink = true) +if (is(Path == string)) // necessary, see comment on previous overload for details { import std.algorithm.iteration : filter; import std.path : globMatch, baseName; @@ -5247,6 +5311,114 @@ auto dirEntries(bool useDIP1000 = dip1000Enabled) assertThrown!Exception(dirEntries("237f5babd6de21f40915826699582e36", "*.bin", SpanMode.depth)); } +@safe unittest +{ + // This is why all the template constraints on `dirEntries` are necessary. + static assert(isImplicitlyConvertible!(DirEntry, string)); +} + +/// Ditto +auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) + (Path path, SpanMode mode, bool followSymlink = true) +if (isImplicitlyConvertible!(Path, string) && !is(Path == string) && !is(Path == DirEntry)) +{ + return dirEntries!(string, useDIP1000)(path, mode, followSymlink); +} + +/// Ditto +auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) + (Path path, string pattern, SpanMode mode, + bool followSymlink = true) +if (isImplicitlyConvertible!(Path, string) && !is(Path == string) && !is(Path == DirEntry)) +{ + return dirEntries!(string, useDIP1000)( + path, pattern, mode, + followSymlink + ); +} + +@safe unittest +{ + static struct Wrapper + { + string data; + alias data this; + } + + string root = deleteme(); + mkdirRecurse(root); + scope (exit) rmdirRecurse(root); + + auto wrapped = Wrapper(root); + foreach (entry; dirEntries(wrapped, SpanMode.shallow)) {} +} + +/// Ditto +auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) + (Path path, SpanMode mode, bool followSymlink = true) +if (is(Path == DirEntry)) +{ + return dirEntries!(string, useDIP1000)(path.nameWithPrefix, mode, followSymlink); +} + +/// Ditto +auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) + (Path path, string pattern, SpanMode mode, + bool followSymlink = true) +if (is(Path == DirEntry)) +{ + return dirEntries!(string, useDIP1000)( + path.nameWithPrefix, pattern, mode, + followSymlink + ); +} + +// https://github.com/dlang/phobos/issues/9584 +@safe unittest +{ + import std.path : absolutePath, buildPath; + + string root = deleteme(); + mkdirRecurse(root); + scope (exit) rmdirRecurse(root); + + mkdirRecurse(root.buildPath("1", "2")); + mkdirRecurse(root.buildPath("3", "4")); + mkdirRecurse(root.buildPath("3", "5", "6")); + + const origWD = getcwd(); + + /* + This wouldn't work if `entry` were a `string` – for fair reasons: + One cannot (reliably) iterate nested directory trees using relative path strings + while changing directories in between. + + The expected error would be something along the lines of: + > Failed to stat file `./3/5': No such file or directory + + See for further details. + */ + chdir(root); + scope(exit) chdir(origWD); + foreach (DirEntry entry; ".".dirEntries(SpanMode.shallow)) + { + if (entry.isDir) + foreach (DirEntry subEntry; entry.dirEntries(SpanMode.shallow)) + if (subEntry.isDir) + chdir(subEntry.absolutePath); + } + + chdir(root); + scope(exit) chdir(origWD); + foreach (DirEntry entry; ".".dirEntries("*", SpanMode.shallow)) + { + if (entry.isDir) + foreach (DirEntry subEntry; entry.dirEntries("*", SpanMode.shallow)) + if (subEntry.isDir) + chdir(subEntry.absolutePath); + } +} + /** * Reads a file line by line and parses the line into a single value or a * $(REF Tuple, std,typecons) of values depending on the length of `Types`. From 0cf64dd94f58ae16cf67eeb28d65e7c8023490b9 Mon Sep 17 00:00:00 2001 From: Elias Batek Date: Mon, 24 Mar 2025 00:47:45 +0100 Subject: [PATCH 2/6] Add experimental `assertThrown(traverseByString())` unittest If this passes CI, we can keep it in, I suppose. --- std/file.d | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/std/file.d b/std/file.d index c45d09ac288..429f7d103b7 100644 --- a/std/file.d +++ b/std/file.d @@ -5417,6 +5417,27 @@ if (is(Path == DirEntry)) if (subEntry.isDir) chdir(subEntry.absolutePath); } + + /* + This tests whether the “relative-path string” pitfall is still a thing. + It can be removed later in case the underlying issue got fixed somehow. + When doing so, one should make sure to delete the warning from the doc + comment of `dirEntries` as well. + */ + void traverseByString() @safe + { + chdir(root); + scope(exit) chdir(origWD); + foreach (string entry; ".".dirEntries(SpanMode.shallow)) + { + if (entry.isDir) + foreach (string subEntry; entry.dirEntries(SpanMode.shallow)) + if (subEntry.isDir) + chdir(subEntry.absolutePath); + } + } + import std.exception : assertThrown; + assertThrown(traverseByString()); } /** From 1a07ebe04fd3e55085a0a9071882588bff09a17a Mon Sep 17 00:00:00 2001 From: Elias Batek Date: Mon, 24 Mar 2025 02:53:27 +0100 Subject: [PATCH 3/6] Let `dirEntries` resort to absolute paths on non-POSIX targets only --- std/file.d | 117 ++++++++++++++++++++++++----------------------------- 1 file changed, 52 insertions(+), 65 deletions(-) diff --git a/std/file.d b/std/file.d index 429f7d103b7..00e2b3288e2 100644 --- a/std/file.d +++ b/std/file.d @@ -160,7 +160,6 @@ else version (Posix) package enum system_file = "/usr/include/assert.h"; } - /++ Exception thrown for file I/O errors. +/ @@ -3652,26 +3651,10 @@ version (StdDdoc) +/ this(return scope string path); - /++ - Constructs a `DirEntry` for the given file (or directory). - - Params: - path = The file (or directory) to get a DirEntry for. - prefix = A prefix to chomp off the path when querying the name of the DirEntry. - - Throws: - $(LREF FileException) if the file does not exist. - +/ - this(return scope string path, return scope string prefix); - version (Windows) { private this(string path, in WIN32_FIND_DATAW *fd, string prefix = null); } - else version (Posix) - { - private this(string path, core.sys.posix.dirent.dirent* fd, string prefix = null); - } /++ Returns the path to the file represented by this `DirEntry`. @@ -3687,14 +3670,6 @@ assert(de2.name == "/usr/share/include"); +/ @property string name() const return scope; - /++ - Returns the path to the file represented by this `DirEntry`. - - Unlike `name`, this property returns the internal name as-is, - potentially starting with an unexpected prefix. - +/ - @property string nameWithPrefix() const return scope; - /++ Returns whether the file represented by this `DirEntry` is a directory. @@ -3855,7 +3830,7 @@ else version (Windows) } } - this(return scope string path, return scope string prefix) + package this(return scope string path, return scope string prefix) { _namePrefix = prefix; this(path); @@ -3887,16 +3862,11 @@ else version (Windows) return _name.chompPrefix(_namePrefix); } - @property string nameWithPrefix() const pure nothrow return scope + package @property string nameWithPrefix() const pure nothrow return scope { return _name; } - private @property string namePrefix() const pure nothrow return scope - { - return _namePrefix; - } - @property bool isDir() const pure nothrow scope { return (attributes & FILE_ATTRIBUTE_DIRECTORY) != 0; @@ -3982,12 +3952,10 @@ else version (Posix) _dTypeSet = false; } - private this(string path, core.sys.posix.dirent.dirent* fd, string prefix = null) @safe + private this(string path, core.sys.posix.dirent.dirent* fd) @safe { import std.path : buildPath; - _namePrefix = prefix; - static if (is(typeof(fd.d_namlen))) immutable len = fd.d_namlen; else @@ -4024,21 +3992,10 @@ else version (Posix) } @property string name() const pure nothrow return scope - { - import std.string : chompPrefix; - return _name.chompPrefix(_namePrefix); - } - - @property string nameWithPrefix() const pure nothrow return scope { return _name; } - private @property string namePrefix() const pure nothrow return scope - { - return _namePrefix; - } - @property bool isDir() scope { _ensureStatOrLStatDone(); @@ -4169,7 +4126,7 @@ else version (Posix) } string _name; /// The file or directory represented by this DirEntry. - string _namePrefix; /// A prefix to be chomped off the name (e.g. parent directories of an absolute path). + string _fOpendir; /// The directory the file handle was opened in. stat_t _statBuf = void; /// The result of stat(). uint _lstatMode; /// The stat mode from lstat(). @@ -4701,7 +4658,9 @@ private struct DirIteratorImpl DirEntry _cur; DirHandle[] _stack; DirEntry[] _stashed; //used in depth first mode - string _namePrefix = null; + + version (Posix) {} + else string _namePrefix = null; //stack helpers void pushExtra(DirEntry de) @@ -4829,7 +4788,7 @@ private struct DirIteratorImpl if (core.stdc.string.strcmp(&fdata.d_name[0], ".") && core.stdc.string.strcmp(&fdata.d_name[0], "..")) { - _cur = DirEntry(_stack[$-1].dirpath, fdata, _namePrefix); + _cur = DirEntry(_stack[$-1].dirpath, fdata); return true; } } @@ -4859,19 +4818,23 @@ private struct DirIteratorImpl this(string pathname, SpanMode mode, bool followSymlink) { - import std.path : absolutePath, isAbsolute; - _mode = mode; _followSymlink = followSymlink; - if (!pathname.isAbsolute) + version (Posix) {} + else { - const pathnameRel = pathname; - alias pathnameAbs = pathname; - pathname = pathname.absolutePath; + import std.path : absolutePath, isAbsolute; + + if (!pathname.isAbsolute) + { + const pathnameRel = pathname; + alias pathnameAbs = pathname; + pathname = pathname.absolutePath; - const offset = pathnameAbs.length - pathnameRel.length; - _namePrefix = pathnameAbs[0 .. offset]; + const offset = pathnameAbs.length - pathnameRel.length; + _namePrefix = pathnameAbs[0 .. offset]; + } } if (stepIn(pathname)) @@ -4880,7 +4843,13 @@ private struct DirIteratorImpl while (mayStepIn()) { auto thisDir = _cur; - if (stepIn(_cur.nameWithPrefix)) + + version (Posix) + const curName = _cur.name; + else + const curName = _cur.nameWithPrefix; + + if (stepIn(curName)) { pushExtra(thisDir); } @@ -4910,7 +4879,13 @@ private struct DirIteratorImpl while (mayStepIn()) { auto thisDir = _cur; - if (stepIn(_cur.nameWithPrefix)) + + version (Posix) + const curName = _cur.name; + else + const curName = _cur.nameWithPrefix; + + if (stepIn(curName)) { pushExtra(thisDir); } @@ -4924,7 +4899,12 @@ private struct DirIteratorImpl case SpanMode.breadth: if (mayStepIn()) { - if (!stepIn(_cur.nameWithPrefix)) + version (Posix) + const curName = _cur.name; + else + const curName = _cur.nameWithPrefix; + + if (!stepIn(curName)) while (!empty && !next()){} } else @@ -5358,7 +5338,12 @@ auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) (Path path, SpanMode mode, bool followSymlink = true) if (is(Path == DirEntry)) { - return dirEntries!(string, useDIP1000)(path.nameWithPrefix, mode, followSymlink); + version (Posix) + const name = path.name; + else + const name = path.nameWithPrefix; + + return dirEntries!(string, useDIP1000)(name, mode, followSymlink); } /// Ditto @@ -5367,10 +5352,12 @@ auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) bool followSymlink = true) if (is(Path == DirEntry)) { - return dirEntries!(string, useDIP1000)( - path.nameWithPrefix, pattern, mode, - followSymlink - ); + version (Posix) + const name = path.name; + else + const name = path.nameWithPrefix; + + return dirEntries!(string, useDIP1000)(name, pattern, mode, followSymlink); } // https://github.com/dlang/phobos/issues/9584 From 3860823fcfb54447dcfe49247d931025eed0ceb5 Mon Sep 17 00:00:00 2001 From: Elias Batek Date: Mon, 24 Mar 2025 06:28:29 +0100 Subject: [PATCH 4/6] Use `fdopendir()` and `openat()` with `dirEntries` --- std/file.d | 132 +++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 109 insertions(+), 23 deletions(-) diff --git a/std/file.d b/std/file.d index 00e2b3288e2..55b18f3093e 100644 --- a/std/file.d +++ b/std/file.d @@ -160,6 +160,28 @@ else version (Posix) package enum system_file = "/usr/include/assert.h"; } +// Shim for missing bindings in druntime +version (Posix) +{ + version (none) + import core.sys.posix.dirent : core_sys_posix_dirent_fdopendir = fdopendir; + else + extern(C) pragma(mangle, "fdopendir") + extern DIR* core_sys_posix_dirent_fdopendir(int fd) @trusted nothrow @nogc; + + version (none) + import core.sys.posix.dirent : core_sys_posix_dirent_dirfd = dirfd; + else + extern(C) pragma(mangle, "dirfd") + extern int core_sys_posix_dirent_dirfd(DIR* dirp) @trusted nothrow @nogc; + + version (none) + import core.sys.posix.fcntl : core_sys_posix_fcntl_openat = openat; + else + extern(C) pragma(mangle, "openat") + extern int core_sys_posix_fcntl_openat(int fd, const(char)* path, int oflag, ...) @system nothrow @nogc; +} + /++ Exception thrown for file I/O errors. +/ @@ -3991,6 +4013,12 @@ else version (Posix) } } + private this(string path, core.sys.posix.dirent.dirent* fd, DIR* parent) @safe + { + _parent = parent; + this(path, fd); + } + @property string name() const pure nothrow return scope { return _name; @@ -4126,7 +4154,8 @@ else version (Posix) } string _name; /// The file or directory represented by this DirEntry. - string _fOpendir; /// The directory the file handle was opened in. + + DIR* _parent; // A handle to the parent directory or `null`. stat_t _statBuf = void; /// The result of stat(). uint _lstatMode; /// The stat mode from lstat(). @@ -4708,6 +4737,11 @@ private struct DirIteratorImpl return toNext(false, &_findinfo); } + bool stepIn(DirEntry directory) @safe + { + return stepIn(directory.nameWithPrefix); + } + bool next() { if (_stack.length == 0) @@ -4762,6 +4796,13 @@ private struct DirIteratorImpl { string dirpath; DIR* h; + int fd = -1; + + void close() @system + { + closedir(this.h); + core.sys.posix.unistd.close(this.fd); + } } bool stepIn(string directory) @@ -4777,6 +4818,45 @@ private struct DirIteratorImpl return next(); } + bool stepIn(string dirName, int dirFD) + { + static auto trustedOpendir(int fd) @trusted + { + return core_sys_posix_dirent_fdopendir(fd); + } + + auto h = trustedOpendir(dirFD); + cenforce(h, dirName); + _stack ~= (DirHandle(dirName, h, dirFD)); + return next(); + } + + bool stepIn(DirEntry directory) + { + import std.path : baseName, dirName; + + if (directory._parent is null) + return stepIn(directory.name); + + static auto trustedOpenat(int fd, string dir, int oflag) @trusted + { + return core_sys_posix_fcntl_openat(fd, dir.tempCString(), oflag); + } + + const fdParent = core_sys_posix_dirent_dirfd(directory._parent); + cenforce(fdParent != -1, dirName(directory.name)); + + const fd = trustedOpenat( + fdParent, + baseName(directory.name), + core.sys.posix.fcntl.O_RDONLY + ); + cenforce(fd != 1, directory.name); + //scope (exit) core.sys.posix.unistd.close(fd); + + return stepIn(directory.name, fd); + } + bool next() @trusted { if (_stack.length == 0) @@ -4788,7 +4868,7 @@ private struct DirIteratorImpl if (core.stdc.string.strcmp(&fdata.d_name[0], ".") && core.stdc.string.strcmp(&fdata.d_name[0], "..")) { - _cur = DirEntry(_stack[$-1].dirpath, fdata); + _cur = DirEntry(_stack[$-1].dirpath, fdata, _stack[$-1].h); return true; } } @@ -4800,14 +4880,14 @@ private struct DirIteratorImpl void popDirStack() @trusted { assert(_stack.length != 0); - closedir(_stack[$-1].h); + _stack[$-1].close(); _stack.popBack(); } void releaseDirStack() @trusted { foreach (d; _stack) - closedir(d.h); + d.close(); } bool mayStepIn() @@ -4837,19 +4917,35 @@ private struct DirIteratorImpl } } - if (stepIn(pathname)) + initialStepIn(pathname); + } + + this(DirEntry path, SpanMode mode, bool followSymlink) + { + version (Posix) + { + _mode = mode; + _followSymlink = followSymlink; + + initialStepIn(path); + } + else + { + this(pathname, mode, followSymlink); + } + } + + private void initialStepIn(T)(T pathOrPathName) + if (is(T == DirEntry) || is(T == string)) + { + if (stepIn(pathOrPathName)) { if (_mode == SpanMode.depth) while (mayStepIn()) { auto thisDir = _cur; - version (Posix) - const curName = _cur.name; - else - const curName = _cur.nameWithPrefix; - - if (stepIn(curName)) + if (stepIn(_cur)) { pushExtra(thisDir); } @@ -4880,12 +4976,7 @@ private struct DirIteratorImpl { auto thisDir = _cur; - version (Posix) - const curName = _cur.name; - else - const curName = _cur.nameWithPrefix; - - if (stepIn(curName)) + if (stepIn(_cur)) { pushExtra(thisDir); } @@ -4899,12 +4990,7 @@ private struct DirIteratorImpl case SpanMode.breadth: if (mayStepIn()) { - version (Posix) - const curName = _cur.name; - else - const curName = _cur.nameWithPrefix; - - if (!stepIn(curName)) + if (!stepIn(_cur)) while (!empty && !next()){} } else From aedd460d744efad24ed26e2c61123cffc0f593d3 Mon Sep 17 00:00:00 2001 From: Elias Batek Date: Mon, 24 Mar 2025 06:54:09 +0100 Subject: [PATCH 5/6] Update `dirEntry` functions to use the new facilities --- std/file.d | 69 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/std/file.d b/std/file.d index 55b18f3093e..459fe364c86 100644 --- a/std/file.d +++ b/std/file.d @@ -3889,6 +3889,11 @@ else version (Windows) return _name; } + package @property string namePrefixOnly() const pure nothrow return scope + { + return _namePrefix; + } + @property bool isDir() const pure nothrow scope { return (attributes & FILE_ATTRIBUTE_DIRECTORY) != 0; @@ -4835,14 +4840,14 @@ private struct DirIteratorImpl { import std.path : baseName, dirName; - if (directory._parent is null) - return stepIn(directory.name); - static auto trustedOpenat(int fd, string dir, int oflag) @trusted { return core_sys_posix_fcntl_openat(fd, dir.tempCString(), oflag); } + if (directory._parent is null) + return stepIn(directory.name); + const fdParent = core_sys_posix_dirent_dirfd(directory._parent); cenforce(fdParent != -1, dirName(directory.name)); @@ -4852,7 +4857,6 @@ private struct DirIteratorImpl core.sys.posix.fcntl.O_RDONLY ); cenforce(fd != 1, directory.name); - //scope (exit) core.sys.posix.unistd.close(fd); return stepIn(directory.name, fd); } @@ -4896,7 +4900,8 @@ private struct DirIteratorImpl } } - this(string pathname, SpanMode mode, bool followSymlink) + this(Path)(Path pathname, SpanMode mode, bool followSymlink) + if (is(Path == string)) { _mode = mode; _followSymlink = followSymlink; @@ -4920,7 +4925,19 @@ private struct DirIteratorImpl initialStepIn(pathname); } - this(DirEntry path, SpanMode mode, bool followSymlink) + version (Posix) {} + else + this(string nameWithPrefix, string namePrefix, SpanMode mode, bool followSymlink) + { + _mode = mode; + _followSymlink = followSymlink; + _namePrefix = namePrefix; + + initialStepIn(nameWithPrefix); + } + + this(Path)(Path path, SpanMode mode, bool followSymlink) + if (is(Path == DirEntry)) { version (Posix) { @@ -4931,7 +4948,10 @@ private struct DirIteratorImpl } else { - this(pathname, mode, followSymlink); + if (path.namePrefixOnly != "") + this(path.nameWithPrefix, path.namePrefixOnly, mode, followSymlink); + else + this(path.name, mode, followSymlink); } } @@ -5018,10 +5038,12 @@ struct _DirIterator(bool useDIP1000) private: SafeRefCounted!(DirIteratorImpl, RefCountedAutoInitialize.no) impl; - this(string pathname, SpanMode mode, bool followSymlink) @trusted + this(Path)(Path pathname, SpanMode mode, bool followSymlink) @trusted + if (is(Path == string) || is(Path == DirEntry)) { impl = typeof(impl)(pathname, mode, followSymlink); } + public: @property bool empty() @trusted { return impl.empty; } @property DirEntry front() @trusted { return impl.front; } @@ -5032,6 +5054,17 @@ public: // template instance alias DirIterator = _DirIterator!dip1000Enabled; +private auto dirEntriesFiltered(Path, bool useDIP1000 = dip1000Enabled) + (Path path, string pattern, SpanMode mode, + bool followSymlink = true) +{ + import std.algorithm.iteration : filter; + import std.path : globMatch, baseName; + + bool f(DirEntry de) { return globMatch(baseName(de.name), pattern); } + return filter!f(_DirIterator!useDIP1000(path, mode, followSymlink)); +} + /++ Returns an $(REF_ALTTEXT input range, isInputRange, std,range,primitives) of `DirEntry` that lazily iterates a given directory, @@ -5259,11 +5292,7 @@ auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) bool followSymlink = true) if (is(Path == string)) // necessary, see comment on previous overload for details { - import std.algorithm.iteration : filter; - import std.path : globMatch, baseName; - - bool f(DirEntry de) { return globMatch(baseName(de.name), pattern); } - return filter!f(_DirIterator!useDIP1000(path, mode, followSymlink)); + return dirEntriesFiltered!(Path, useDIP1000)(path, pattern, mode, followSymlink); } @safe unittest @@ -5424,12 +5453,7 @@ auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) (Path path, SpanMode mode, bool followSymlink = true) if (is(Path == DirEntry)) { - version (Posix) - const name = path.name; - else - const name = path.nameWithPrefix; - - return dirEntries!(string, useDIP1000)(name, mode, followSymlink); + return _DirIterator!useDIP1000(path, mode, followSymlink); } /// Ditto @@ -5438,12 +5462,7 @@ auto dirEntries(Path, bool useDIP1000 = dip1000Enabled) bool followSymlink = true) if (is(Path == DirEntry)) { - version (Posix) - const name = path.name; - else - const name = path.nameWithPrefix; - - return dirEntries!(string, useDIP1000)(name, pattern, mode, followSymlink); + return dirEntriesFiltered!(string, useDIP1000)(path, pattern, mode, followSymlink); } // https://github.com/dlang/phobos/issues/9584 From dd4494c668d3038d43e29713f6e6c7620b19998f Mon Sep 17 00:00:00 2001 From: Elias Batek Date: Mon, 24 Mar 2025 06:58:49 +0100 Subject: [PATCH 6/6] Add TODO notes --- std/file.d | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/std/file.d b/std/file.d index 459fe364c86..bfc023a8c08 100644 --- a/std/file.d +++ b/std/file.d @@ -4109,7 +4109,7 @@ else version (Posix) return; cenforce(stat(_name.tempCString(), &_statBuf) == 0, - "Failed to stat file `" ~ _name ~ "'"); + "Failed to stat file `" ~ _name ~ "'"); // TODO: statat() _didStat = true; } @@ -4126,7 +4126,7 @@ else version (Posix) if (_didStat) return; - if (stat(_name.tempCString(), &_statBuf) != 0) + if (stat(_name.tempCString(), &_statBuf) != 0) // TODO: statat() { _ensureLStatDone(); @@ -4150,7 +4150,7 @@ else version (Posix) stat_t statbuf = void; cenforce(lstat(_name.tempCString(), &statbuf) == 0, - "Failed to stat file `" ~ _name ~ "'"); + "Failed to stat file `" ~ _name ~ "'"); // TODO: fstatat(… AT_SYMLINK_NOFOLLOW) _lstatMode = statbuf.st_mode;