From 17bc1f62a51ad5fc44dc97acf6ddb33e6d00b0c4 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Fri, 22 Nov 2019 15:40:46 -0600 Subject: [PATCH 1/7] Split `std.fs.Dir.openDir` into `openDirList` and `openDirTraverse` to clarify what directories can be iterated. Closes ziglang/zig#3741. The Windows-inspired nomenclature of "List" and "Traverse" was chosen over POSIX-style "Read" and "Path" (from `O_PATH`) for clarity. Using "Path" makes it look like the function is manipulating strings, and the generic "Read" ending isn't useful when there is no generic read method. Even in implementation details, `read` is never used. Actual exploitation of the difference between the two functions will come in a later commit. --- lib/std/fs.zig | 70 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 14 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 761e3b8c14..16aa00d07f 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -657,8 +657,8 @@ pub const Dir = struct { } } - /// Returns an open handle to the current working directory. - /// Closing the returned `Dir` is checked illegal behavior. + /// Returns an handle to the current working directory that is open for traversal. + /// Closing the returned `Dir` is checked illegal behavior. Iterating over the result is illegal behavior. /// On POSIX targets, this function is comptime-callable. pub fn cwd() Dir { if (builtin.os == .windows) { @@ -683,14 +683,14 @@ pub const Dir = struct { DeviceBusy, } || os.UnexpectedError; - /// Call `close` to free the directory handle. + /// Deprecated; call `Dir.cwd().openDirList` directly. pub fn open(dir_path: []const u8) OpenError!Dir { - return cwd().openDir(dir_path); + return cwd().openDirList(dir_path); } - /// Same as `open` except the parameter is null-terminated. + /// Deprecated; call `Dir.cwd().openDirListC` directly. pub fn openC(dir_path_c: [*:0]const u8) OpenError!Dir { - return cwd().openDirC(dir_path_c); + return cwd().openDirListC(dir_path_c); } pub fn close(self: *Dir) void { @@ -775,22 +775,57 @@ pub const Dir = struct { } } - /// Call `close` on the result when done. + /// Deprecated; call `openDirList` directly. pub fn openDir(self: Dir, sub_path: []const u8) OpenError!Dir { + return self.openDirList(sub_path); + } + + /// Deprecated; call `openDirListC` directly. + pub fn openDirC(self: Dir, sub_path_c: [*:0]const u8) OpenError!Dir { + return self.openDirListC(sub_path_c); + } + + /// Opens a directory at the given path with the ability to access subpaths + /// of the result. Calling `iterate` on the result is illegal behavior; to + /// list the contents of a directory, open it with `openDirList`. + /// + /// Call `close` on the result when done. + pub fn openDirTraverse(self: Dir, sub_path: []const u8) OpenError!Dir { if (builtin.os == .windows) { const sub_path_w = try os.windows.sliceToPrefixedFileW(sub_path); - return self.openDirW(&sub_path_w); + return self.openDirTraverseW(&sub_path_w); } const sub_path_c = try os.toPosixPath(sub_path); - return self.openDirC(&sub_path_c); + return self.openDirTraverseC(&sub_path_c); } - /// Same as `openDir` except the parameter is null-terminated. - pub fn openDirC(self: Dir, sub_path_c: [*:0]const u8) OpenError!Dir { + /// Opens a directory at the given path with the ability to access subpaths and list contents + /// of the result. If the ability to list contents is unneeded, `openDirTraverse` acts the + /// same and may be more efficient. + /// + /// Call `close` on the result when done. + pub fn openDirList(self: Dir, sub_path: []const u8) OpenError!Dir { + if (builtin.os == .windows) { + const sub_path_w = try os.windows.sliceToPrefixedFileW(sub_path); + return self.openDirListW(&sub_path_w); + } + + const sub_path_c = try os.toPosixPath(sub_path); + return self.openDirListC(&sub_path_c); + } + + /// Same as `openDirTraverse` except the parameter is null-terminated. + pub fn openDirTraverseC(self: Dir, sub_path_c: [*:0]const u8) OpenError!Dir { + // TODO: use O_PATH where supported + return self.openDirListC(sub_path_c); + } + + /// Same as `openDirList` except the parameter is null-terminated. + pub fn openDirListC(self: Dir, sub_path_c: [*:0]const u8) OpenError!Dir { if (builtin.os == .windows) { const sub_path_w = try os.windows.cStrToPrefixedFileW(sub_path_c); - return self.openDirW(&sub_path_w); + return self.openDirListW(&sub_path_w); } const flags = os.O_RDONLY | os.O_DIRECTORY | os.O_CLOEXEC; @@ -804,9 +839,16 @@ pub const Dir = struct { return Dir{ .fd = fd }; } - /// Same as `openDir` except the path parameter is UTF16LE, NT-prefixed. + /// Same as `openDirTraverse` except the path parameter is UTF16LE, NT-prefixed. /// This function is Windows-only. - pub fn openDirW(self: Dir, sub_path_w: [*:0]const u16) OpenError!Dir { + pub fn openDirTraverseW(self: Dir, sub_path_w: [*:0]const u16) OpenError!Dir { + // TODO: open without FILE_LIST_DIRECTORY + return self.openDirListW(sub_path_w); + } + + /// Same as `openDirList` except the path parameter is UTF16LE, NT-prefixed. + /// This function is Windows-only. + pub fn openDirListW(self: Dir, sub_path_w: [*:0]const u16) OpenError!Dir { const w = os.windows; var result = Dir{ From 07120c87451636f65d2917f0355b4493034a2c0f Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Fri, 22 Nov 2019 15:56:46 -0600 Subject: [PATCH 2/7] Use `O_PATH` where available in `std.fs.Dir.openPathTraverse`. --- lib/std/fs.zig | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 16aa00d07f..23628764f0 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -817,8 +817,13 @@ pub const Dir = struct { /// Same as `openDirTraverse` except the parameter is null-terminated. pub fn openDirTraverseC(self: Dir, sub_path_c: [*:0]const u8) OpenError!Dir { - // TODO: use O_PATH where supported - return self.openDirListC(sub_path_c); + if (builtin.os == .windows) { + const sub_path_w = try os.windows.cStrToPrefixedFileW(sub_path_c); + return self.openDirTraverseW(&sub_path_w); + } else { + const O_PATH = if (@hasDecl(os, "O_PATH")) os.O_PATH else 0; + return self.openDirFlagsC(sub_path_c, os.O_RDONLY | os.O_DIRECTORY | os.O_CLOEXEC | O_PATH); + } } /// Same as `openDirList` except the parameter is null-terminated. @@ -826,9 +831,12 @@ pub const Dir = struct { if (builtin.os == .windows) { const sub_path_w = try os.windows.cStrToPrefixedFileW(sub_path_c); return self.openDirListW(&sub_path_w); + } else { + return self.openDirFlagsC(sub_path_c, os.O_RDONLY | os.O_DIRECTORY | os.O_CLOEXEC); } + } - const flags = os.O_RDONLY | os.O_DIRECTORY | os.O_CLOEXEC; + fn openDirFlagsC(self: Dir, sub_path_c: [*]const u8, flags: u32) OpenError!Dir { const fd = os.openatC(self.fd, sub_path_c, flags, 0) catch |err| switch (err) { error.FileTooBig => unreachable, // can't happen for directories error.IsDir => unreachable, // we're providing O_DIRECTORY From 51c57408794ae4411a0059d7f9d30b47d83a534e Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Fri, 22 Nov 2019 16:14:39 -0600 Subject: [PATCH 3/7] Use a specific access mask in `Dir.openDirListW` instead of a generic one. Untested. The actual desired access mask in this case seems quite confusing and badly documented. The previous combination of `GENERIC_READ` and `SYNCHRONIZE` seems both illegal and redundant according to the [`ntifs.h` documentation](https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/nf-ntifs-ntcreatefile), which specifies that `GENERIC_READ` should not be used for directories and includes `SYNCHRONIZE`. `winnt.h` contains a number of relevant-sounding flags such as `FILE_ADD_FILE`, `FILE_ADD_SUBDIRECTORY`, and `FILE_DELETE_CHILD` that do not show up in documentation at all. These are equal in value to file-specific flags that are documented as flags you should not specify when opening a directory. --- lib/std/fs.zig | 2 +- lib/std/os/windows/bits.zig | 22 +++++++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 23628764f0..7c25895c44 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -889,7 +889,7 @@ pub const Dir = struct { var io: w.IO_STATUS_BLOCK = undefined; const rc = w.ntdll.NtCreateFile( &result.fd, - w.GENERIC_READ | w.SYNCHRONIZE, + w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE | w.FILE_TRAVERSE | w.FILE_LIST_DIRECTORY, &attr, &io, null, diff --git a/lib/std/os/windows/bits.zig b/lib/std/os/windows/bits.zig index 2f65c27e47..aa478fbe5b 100644 --- a/lib/std/os/windows/bits.zig +++ b/lib/std/os/windows/bits.zig @@ -412,7 +412,10 @@ pub const READ_CONTROL = 0x00020000; pub const WRITE_DAC = 0x00040000; pub const WRITE_OWNER = 0x00080000; pub const SYNCHRONIZE = 0x00100000; -pub const STANDARD_RIGHTS_REQUIRED = 0x000f0000; +pub const STANDARD_RIGHTS_READ = READ_CONTROL; +pub const STANDARD_RIGHTS_WRITE = READ_CONTROL; +pub const STANDARD_RIGHTS_EXECUTE = READ_CONTROL; +pub const STANDARD_RIGHTS_REQUIRED = DELETE | READ_CONTROL | WRITE_DAC | WRITE_OWNER; // disposition for NtCreateFile pub const FILE_SUPERSEDE = 0; @@ -424,6 +427,21 @@ pub const FILE_OVERWRITE_IF = 5; pub const FILE_MAXIMUM_DISPOSITION = 5; // flags for NtCreateFile and NtOpenFile +pub const FILE_READ_DATA = 0x00000001; +pub const FILE_LIST_DIRECTORY = 0x00000001; +pub const FILE_WRITE_DATA = 0x00000002; +pub const FILE_ADD_FILE = 0x00000002; +pub const FILE_APPEND_DATA = 0x00000004; +pub const FILE_ADD_SUBDIRECTORY = 0x00000004; +pub const FILE_CREATE_PIPE_INSTANCE = 0x00000004; +pub const FILE_READ_EA = 0x00000008; +pub const FILE_WRITE_EA = 0x00000010; +pub const FILE_EXECUTE = 0x00000020; +pub const FILE_TRAVERSE = 0x00000020; +pub const FILE_DELETE_CHILD = 0x00000040; +pub const FILE_READ_ATTRIBUTES = 0x00000080; +pub const FILE_WRITE_ATTRIBUTES = 0x00000100; + pub const FILE_DIRECTORY_FILE = 0x00000001; pub const FILE_WRITE_THROUGH = 0x00000002; pub const FILE_SEQUENTIAL_ONLY = 0x00000004; @@ -755,8 +773,6 @@ pub const FILE_ACTION_RENAMED_NEW_NAME = 0x00000005; pub const LPOVERLAPPED_COMPLETION_ROUTINE = ?extern fn (DWORD, DWORD, *OVERLAPPED) void; -pub const FILE_LIST_DIRECTORY = 1; - pub const FILE_NOTIFY_CHANGE_CREATION = 64; pub const FILE_NOTIFY_CHANGE_SIZE = 8; pub const FILE_NOTIFY_CHANGE_SECURITY = 256; From 001334266a0f6d0d7da4e0eba5b5e317456a39ff Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Fri, 22 Nov 2019 16:25:43 -0600 Subject: [PATCH 4/7] Don't pass `FILE_LIST_DIRECTORY` in `openDirTraverseW`. --- lib/std/fs.zig | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 7c25895c44..aa45513e05 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -836,7 +836,7 @@ pub const Dir = struct { } } - fn openDirFlagsC(self: Dir, sub_path_c: [*]const u8, flags: u32) OpenError!Dir { + fn openDirFlagsC(self: Dir, sub_path_c: [*:0]const u8, flags: u32) OpenError!Dir { const fd = os.openatC(self.fd, sub_path_c, flags, 0) catch |err| switch (err) { error.FileTooBig => unreachable, // can't happen for directories error.IsDir => unreachable, // we're providing O_DIRECTORY @@ -850,13 +850,16 @@ pub const Dir = struct { /// Same as `openDirTraverse` except the path parameter is UTF16LE, NT-prefixed. /// This function is Windows-only. pub fn openDirTraverseW(self: Dir, sub_path_w: [*:0]const u16) OpenError!Dir { - // TODO: open without FILE_LIST_DIRECTORY - return self.openDirListW(sub_path_w); + return self.openDirAccessMaskW(sub_path_w, w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE | w.FILE_TRAVERSE); } /// Same as `openDirList` except the path parameter is UTF16LE, NT-prefixed. /// This function is Windows-only. pub fn openDirListW(self: Dir, sub_path_w: [*:0]const u16) OpenError!Dir { + return self.openDirAccessMaskW(sub_path_w, w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE | w.FILE_TRAVERSE | w.FILE_LIST_DIRECTORY); + } + + fn openDirAccessMaskW(self: Dir, sub_path_w: [*:0]const u16, access_mask: u32) OpenError!Dir { const w = os.windows; var result = Dir{ @@ -889,7 +892,7 @@ pub const Dir = struct { var io: w.IO_STATUS_BLOCK = undefined; const rc = w.ntdll.NtCreateFile( &result.fd, - w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE | w.FILE_TRAVERSE | w.FILE_LIST_DIRECTORY, + access_mask, &attr, &io, null, From 4014a8e4b44bad5aca896ce0fdf120ef4212fc8d Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Fri, 22 Nov 2019 16:32:50 -0600 Subject: [PATCH 5/7] Avoid deprecated cwd-based functions for opening directories, preferring to open explicitly relative to `Dir.cwd()`. --- lib/std/fs.zig | 10 +++++----- lib/std/os/test.zig | 2 +- src-self-hosted/main.zig | 2 +- src-self-hosted/stage1.zig | 2 +- tools/process_headers.zig | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index aa45513e05..e2ac561c7e 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -350,7 +350,7 @@ pub fn deleteTree(full_path: []const u8) !void { CannotDeleteRootDirectory, }.CannotDeleteRootDirectory; - var dir = try Dir.open(dirname); + var dir = try Dir.cwd().openDirList(dirname); defer dir.close(); return dir.deleteTree(path.basename(full_path)); @@ -1066,7 +1066,7 @@ pub const Dir = struct { error.Unexpected, => |e| return e, } - var dir = self.openDir(sub_path) catch |err| switch (err) { + var dir = self.openDirList(sub_path) catch |err| switch (err) { error.NotDir => { if (got_access_denied) { return error.AccessDenied; @@ -1131,7 +1131,7 @@ pub const Dir = struct { => |e| return e, } - const new_dir = dir.openDir(entry.name) catch |err| switch (err) { + const new_dir = dir.openDirList(entry.name) catch |err| switch (err) { error.NotDir => { if (got_access_denied) { return error.AccessDenied; @@ -1222,7 +1222,7 @@ pub const Walker = struct { try self.name_buffer.appendByte(path.sep); try self.name_buffer.append(base.name); if (base.kind == .Directory) { - var new_dir = top.dir_it.dir.openDir(base.name) catch |err| switch (err) { + var new_dir = top.dir_it.dir.openDirList(base.name) catch |err| switch (err) { error.NameTooLong => unreachable, // no path sep in base.name else => |e| return e, }; @@ -1260,7 +1260,7 @@ pub const Walker = struct { pub fn walkPath(allocator: *Allocator, dir_path: []const u8) !Walker { assert(!mem.endsWith(u8, dir_path, path.sep_str)); - var dir = try Dir.open(dir_path); + var dir = try Dir.cwd().openDirList(dir_path); errdefer dir.close(); var name_buffer = try std.Buffer.init(allocator, dir_path); diff --git a/lib/std/os/test.zig b/lib/std/os/test.zig index dd4a8e1860..778a39eb3c 100644 --- a/lib/std/os/test.zig +++ b/lib/std/os/test.zig @@ -20,7 +20,7 @@ test "makePath, put some files in it, deleteTree" { try io.writeFile("os_test_tmp" ++ fs.path.sep_str ++ "b" ++ fs.path.sep_str ++ "c" ++ fs.path.sep_str ++ "file.txt", "nonsense"); try io.writeFile("os_test_tmp" ++ fs.path.sep_str ++ "b" ++ fs.path.sep_str ++ "file2.txt", "blah"); try fs.deleteTree("os_test_tmp"); - if (fs.Dir.open("os_test_tmp")) |dir| { + if (fs.Dir.cwd().openDirTraverse("os_test_tmp")) |dir| { @panic("expected error"); } else |err| { expect(err == error.FileNotFound); diff --git a/src-self-hosted/main.zig b/src-self-hosted/main.zig index 501c8679f6..bf3fb4dea5 100644 --- a/src-self-hosted/main.zig +++ b/src-self-hosted/main.zig @@ -715,7 +715,7 @@ async fn fmtPath(fmt: *Fmt, file_path_ref: []const u8, check_mode: bool) FmtErro // ) catch |err| switch (err) { // error.IsDir, error.AccessDenied => { // // TODO make event based (and dir.next()) - // var dir = try fs.Dir.open(file_path); + // var dir = try fs.Dir.cwd().openDirList(file_path); // defer dir.close(); // var group = event.Group(FmtError!void).init(fmt.allocator); diff --git a/src-self-hosted/stage1.zig b/src-self-hosted/stage1.zig index 7220c3d0d9..96bf4b7fdf 100644 --- a/src-self-hosted/stage1.zig +++ b/src-self-hosted/stage1.zig @@ -279,7 +279,7 @@ fn fmtPath(fmt: *Fmt, file_path_ref: []const u8, check_mode: bool) FmtError!void const source_code = io.readFileAlloc(fmt.allocator, file_path) catch |err| switch (err) { error.IsDir, error.AccessDenied => { // TODO make event based (and dir.next()) - var dir = try fs.Dir.open(file_path); + var dir = try fs.Dir.cwd().openDirList(file_path); defer dir.close(); var dir_it = dir.iterate(); diff --git a/tools/process_headers.zig b/tools/process_headers.zig index ffa2bdd8ef..3a56cab162 100644 --- a/tools/process_headers.zig +++ b/tools/process_headers.zig @@ -340,7 +340,7 @@ pub fn main() !void { try dir_stack.append(target_include_dir); while (dir_stack.popOrNull()) |full_dir_name| { - var dir = std.fs.Dir.open(full_dir_name) catch |err| switch (err) { + var dir = std.fs.Dir.cwd().openDirList(full_dir_name) catch |err| switch (err) { error.FileNotFound => continue :search, error.AccessDenied => continue :search, else => return err, From 8baf226d693a8c522be8d66ecd4edb11fa99a749 Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Fri, 22 Nov 2019 16:40:23 -0600 Subject: [PATCH 6/7] Add missing shortening of os.windows. --- lib/std/fs.zig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index e2ac561c7e..0a08126c00 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -850,12 +850,16 @@ pub const Dir = struct { /// Same as `openDirTraverse` except the path parameter is UTF16LE, NT-prefixed. /// This function is Windows-only. pub fn openDirTraverseW(self: Dir, sub_path_w: [*:0]const u16) OpenError!Dir { + const w = os.windows; + return self.openDirAccessMaskW(sub_path_w, w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE | w.FILE_TRAVERSE); } /// Same as `openDirList` except the path parameter is UTF16LE, NT-prefixed. /// This function is Windows-only. pub fn openDirListW(self: Dir, sub_path_w: [*:0]const u16) OpenError!Dir { + const w = os.windows; + return self.openDirAccessMaskW(sub_path_w, w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE | w.FILE_TRAVERSE | w.FILE_LIST_DIRECTORY); } From ec569ff26a325f359649bc1cc3515799ceccb62f Mon Sep 17 00:00:00 2001 From: Jonathan S Date: Mon, 25 Nov 2019 01:46:35 -0600 Subject: [PATCH 7/7] Or in O_DIRECTORY in openDirFlagsC to capture the fact that it is intended for opening directories --- lib/std/fs.zig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 0a08126c00..f580cf2045 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -822,7 +822,7 @@ pub const Dir = struct { return self.openDirTraverseW(&sub_path_w); } else { const O_PATH = if (@hasDecl(os, "O_PATH")) os.O_PATH else 0; - return self.openDirFlagsC(sub_path_c, os.O_RDONLY | os.O_DIRECTORY | os.O_CLOEXEC | O_PATH); + return self.openDirFlagsC(sub_path_c, os.O_RDONLY | os.O_CLOEXEC | O_PATH); } } @@ -832,12 +832,12 @@ pub const Dir = struct { const sub_path_w = try os.windows.cStrToPrefixedFileW(sub_path_c); return self.openDirListW(&sub_path_w); } else { - return self.openDirFlagsC(sub_path_c, os.O_RDONLY | os.O_DIRECTORY | os.O_CLOEXEC); + return self.openDirFlagsC(sub_path_c, os.O_RDONLY | os.O_CLOEXEC); } } fn openDirFlagsC(self: Dir, sub_path_c: [*:0]const u8, flags: u32) OpenError!Dir { - const fd = os.openatC(self.fd, sub_path_c, flags, 0) catch |err| switch (err) { + const fd = os.openatC(self.fd, sub_path_c, flags | os.O_DIRECTORY, 0) catch |err| switch (err) { error.FileTooBig => unreachable, // can't happen for directories error.IsDir => unreachable, // we're providing O_DIRECTORY error.NoSpaceLeft => unreachable, // not providing O_CREAT