From 4843c3b4c386008418ff8ab7238feebab3711352 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 16 Mar 2020 11:39:18 +0100 Subject: [PATCH 1/5] std: Introduce fnctl wrapper --- lib/std/os.zig | 25 +++++++++++++++++++++++++ lib/std/os/bits/dragonfly.zig | 2 ++ lib/std/os/bits/freebsd.zig | 2 ++ lib/std/os/bits/linux.zig | 2 ++ lib/std/os/bits/netbsd.zig | 2 ++ lib/std/os/linux.zig | 4 ++++ lib/std/os/test.zig | 31 ++++++++++++++++++++++++++++++- 7 files changed, 67 insertions(+), 1 deletion(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index 3333a91788..2dec2c4c1d 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -3163,6 +3163,31 @@ pub fn lseek_CUR_get(fd: fd_t) SeekError!u64 { } } +pub const FcntlError = error{ + PermissionDenied, + FileBusy, + ProcessFdQuotaExceeded, + Locked, +} || UnexpectedError; + +pub fn fcntl(fd: fd_t, cmd: i32, arg: usize) FcntlError!usize { + while (true) { + const rc = system.fcntl(fd, cmd, arg); + switch (errno(rc)) { + 0 => return @intCast(usize, rc), + EINTR => continue, + EACCES => return error.Locked, + EBADF => unreachable, + EBUSY => return error.FileBusy, + EINVAL => unreachable, // invalid parameters + EPERM => return error.PermissionDenied, + EMFILE => return error.ProcessFdQuotaExceeded, + ENOTDIR => unreachable, // invalid parameter + else => |err| return unexpectedErrno(err), + } + } +} + pub const RealPathError = error{ FileNotFound, AccessDenied, diff --git a/lib/std/os/bits/dragonfly.zig b/lib/std/os/bits/dragonfly.zig index 27b2733b76..4a9b51d472 100644 --- a/lib/std/os/bits/dragonfly.zig +++ b/lib/std/os/bits/dragonfly.zig @@ -283,6 +283,8 @@ pub const F_LOCK = 1; pub const F_TLOCK = 2; pub const F_TEST = 3; +pub const FD_CLOEXEC = 1; + pub const AT_FDCWD = -328243; pub const AT_SYMLINK_NOFOLLOW = 1; pub const AT_REMOVEDIR = 2; diff --git a/lib/std/os/bits/freebsd.zig b/lib/std/os/bits/freebsd.zig index 02fe7e75b3..6f97884e16 100644 --- a/lib/std/os/bits/freebsd.zig +++ b/lib/std/os/bits/freebsd.zig @@ -355,6 +355,8 @@ pub const F_GETOWN_EX = 16; pub const F_GETOWNER_UIDS = 17; +pub const FD_CLOEXEC = 1; + pub const SEEK_SET = 0; pub const SEEK_CUR = 1; pub const SEEK_END = 2; diff --git a/lib/std/os/bits/linux.zig b/lib/std/os/bits/linux.zig index 2a58c14490..89364909c9 100644 --- a/lib/std/os/bits/linux.zig +++ b/lib/std/os/bits/linux.zig @@ -136,6 +136,8 @@ pub const MAP_FIXED_NOREPLACE = 0x100000; /// For anonymous mmap, memory could be uninitialized pub const MAP_UNINITIALIZED = 0x4000000; +pub const FD_CLOEXEC = 1; + pub const F_OK = 0; pub const X_OK = 1; pub const W_OK = 2; diff --git a/lib/std/os/bits/netbsd.zig b/lib/std/os/bits/netbsd.zig index 735485695a..4abd4c8c5e 100644 --- a/lib/std/os/bits/netbsd.zig +++ b/lib/std/os/bits/netbsd.zig @@ -312,6 +312,8 @@ pub const F_GETLK = 7; pub const F_SETLK = 8; pub const F_SETLKW = 9; +pub const FD_CLOEXEC = 1; + pub const SEEK_SET = 0; pub const SEEK_CUR = 1; pub const SEEK_END = 2; diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index f3265eaa61..ae48d831cf 100644 --- a/lib/std/os/linux.zig +++ b/lib/std/os/linux.zig @@ -588,6 +588,10 @@ pub fn waitpid(pid: pid_t, status: *u32, flags: u32) usize { return syscall4(SYS_wait4, @bitCast(usize, @as(isize, pid)), @ptrToInt(status), flags, 0); } +pub fn fcntl(fd: fd_t, cmd: i32, arg: usize) usize { + return syscall3(SYS_fcntl, @bitCast(usize, @as(isize, fd)), @bitCast(usize, @as(isize, cmd)), arg); +} + var vdso_clock_gettime = @ptrCast(?*const c_void, init_vdso_clock_gettime); // We must follow the C calling convention when we call into the VDSO diff --git a/lib/std/os/test.zig b/lib/std/os/test.zig index 1ef6798b50..b0452f4d66 100644 --- a/lib/std/os/test.zig +++ b/lib/std/os/test.zig @@ -1,7 +1,8 @@ const std = @import("../std.zig"); const os = std.os; const testing = std.testing; -const expect = std.testing.expect; +const expect = testing.expect; +const expectEqual = testing.expectEqual; const io = std.io; const fs = std.fs; const mem = std.mem; @@ -446,3 +447,31 @@ test "getenv" { expect(os.getenvZ("BOGUSDOESNOTEXISTENVVAR") == null); } } + +test "fcntl" { + if (builtin.os.tag == .windows) + return error.SkipZigTest; + + const test_out_file = "os_tmp_test"; + + const file = try fs.cwd().createFile(test_out_file, .{}); + defer file.close(); + + // Note: The test assumes createFile opens the file with O_CLOEXEC + { + const flags = try os.fcntl(file.handle, os.F_GETFD, 0); + expect((flags & os.FD_CLOEXEC) != 0); + } + { + _ = try os.fcntl(file.handle, os.F_SETFD, 0); + const flags = try os.fcntl(file.handle, os.F_GETFD, 0); + expect((flags & os.FD_CLOEXEC) == 0); + } + { + _ = try os.fcntl(file.handle, os.F_SETFD, os.FD_CLOEXEC); + const flags = try os.fcntl(file.handle, os.F_GETFD, 0); + expect((flags & os.FD_CLOEXEC) != 0); + } + + try fs.cwd().deleteFile(test_out_file); +} From e15605e1c1f28e06035a6740619295151195dabb Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 16 Mar 2020 12:01:41 +0100 Subject: [PATCH 2/5] std: Safety check for iterate() Calling iterate() on a Dir object returned by openDirTraverse is always an error. --- lib/std/fs.zig | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 231235baf5..7e647230e6 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -555,6 +555,36 @@ pub const Dir = struct { }; pub fn iterate(self: Dir) Iterator { + // Make sure the directory was not open with openDirTraverse + if (std.debug.runtime_safety) { + var ok = true; + + if (builtin.os.tag == .windows) { + const w = os.windows; + + var io_status_block: w.IO_STATUS_BLOCK = undefined; + var info: w.FILE_ACCESS_INFORMATION = undefined; + + const rc = w.ntdll.NtQueryInformationFile( + self.fd, + &io_status_block, + &info, + @sizeOf(w.FILE_ACCESS_INFORMATION), + .FileAccessInformation, + ); + assert(rc == .SUCCESS); + + ok = (info.AccessFlags & w.FILE_LIST_DIRECTORY) != 0; + } else if (@hasDecl(os, "O_PATH")) { + const f = os.fcntl(self.fd, os.F_GETFL, 0) catch unreachable; + ok = (f & os.O_PATH) == 0; + } + + if (!ok) { + std.debug.panic("iterate() called on Dir open with openDirTraverse", .{}); + } + } + switch (builtin.os.tag) { .macosx, .ios, .freebsd, .netbsd, .dragonfly => return Iterator{ .dir = self, From c45fe2759fdb882efb4caf89a35136b8b205ee7c Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Mon, 16 Mar 2020 12:03:16 +0100 Subject: [PATCH 3/5] build: Fix silly bug in directory traversal --- lib/std/build.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/std/build.zig b/lib/std/build.zig index e5d83f22b9..1b80f5f061 100644 --- a/lib/std/build.zig +++ b/lib/std/build.zig @@ -2156,10 +2156,10 @@ pub const LibExeObjStep = struct { const build_output_dir = mem.trimRight(u8, output_dir_nl, "\r\n"); if (self.output_dir) |output_dir| { - var src_dir = try std.fs.cwd().openDirTraverse(build_output_dir); + var src_dir = try std.fs.cwd().openDirList(build_output_dir); defer src_dir.close(); - var dest_dir = try std.fs.cwd().openDirList(output_dir); + var dest_dir = try std.fs.cwd().openDirTraverse(output_dir); defer dest_dir.close(); var it = src_dir.iterate(); From 27affde592653ac7f92489cec404b4bf3e0d1b29 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 18 Mar 2020 14:45:01 -0400 Subject: [PATCH 4/5] (breaking) clarify openDir API * remove deprecated `std.fs.Dir` APIs * `std.fs.Dir.openDir` now takes a options struct with bool fields for `access_sub_paths` and `iterate`. It's now much more clear how opening directories works. * fixed the std lib and various zig code calling the wrong openDir function. * the runtime safety check for dir flags is removed in favor of the cheaper option of putting a comment on the same line as handling EBADF / ACCESS_DENIED, since that will show up in stack traces. --- lib/std/build.zig | 4 +- lib/std/build/write_file.zig | 2 +- lib/std/fs.zig | 163 +++++++------------------- lib/std/os/test.zig | 11 +- lib/std/zig/system.zig | 2 +- src-self-hosted/libc_installation.zig | 10 +- src-self-hosted/main.zig | 2 +- src-self-hosted/print_targets.zig | 2 +- src-self-hosted/stage2.zig | 2 +- 9 files changed, 62 insertions(+), 136 deletions(-) diff --git a/lib/std/build.zig b/lib/std/build.zig index 1b80f5f061..7585fe20e2 100644 --- a/lib/std/build.zig +++ b/lib/std/build.zig @@ -2156,10 +2156,10 @@ pub const LibExeObjStep = struct { const build_output_dir = mem.trimRight(u8, output_dir_nl, "\r\n"); if (self.output_dir) |output_dir| { - var src_dir = try std.fs.cwd().openDirList(build_output_dir); + var src_dir = try std.fs.cwd().openDir(build_output_dir, .{ .iterate = true }); defer src_dir.close(); - var dest_dir = try std.fs.cwd().openDirTraverse(output_dir); + var dest_dir = try std.fs.cwd().openDir(output_dir, .{}); defer dest_dir.close(); var it = src_dir.iterate(); diff --git a/lib/std/build/write_file.zig b/lib/std/build/write_file.zig index 60c54336e0..0c3f628457 100644 --- a/lib/std/build/write_file.zig +++ b/lib/std/build/write_file.zig @@ -78,7 +78,7 @@ pub const WriteFileStep = struct { warn("unable to make path {}: {}\n", .{ self.output_dir, @errorName(err) }); return err; }; - var dir = try fs.cwd().openDirTraverse(self.output_dir); + var dir = try fs.cwd().openDir(self.output_dir, .{}); defer dir.close(); for (self.files.toSliceConst()) |file| { dir.writeFile(file.basename, file.bytes) catch |err| { diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 7e647230e6..e12c35ab8e 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -274,7 +274,7 @@ pub fn deleteTree(full_path: []const u8) !void { CannotDeleteRootDirectory, }.CannotDeleteRootDirectory; - var dir = try cwd().openDirList(dirname); + var dir = try cwd().openDir(dirname, .{}); defer dir.close(); return dir.deleteTree(path.basename(full_path)); @@ -339,7 +339,7 @@ pub const Dir = struct { if (rc == 0) return null; if (rc < 0) { switch (os.errno(rc)) { - os.EBADF => unreachable, + os.EBADF => unreachable, // Dir is invalid or was opened without iteration ability os.EFAULT => unreachable, os.ENOTDIR => unreachable, os.EINVAL => unreachable, @@ -388,7 +388,7 @@ pub const Dir = struct { ); switch (os.errno(rc)) { 0 => {}, - os.EBADF => unreachable, + os.EBADF => unreachable, // Dir is invalid or was opened without iteration ability os.EFAULT => unreachable, os.ENOTDIR => unreachable, os.EINVAL => unreachable, @@ -444,7 +444,7 @@ pub const Dir = struct { const rc = os.linux.getdents64(self.dir.fd, &self.buf, self.buf.len); switch (os.linux.getErrno(rc)) { 0 => {}, - os.EBADF => unreachable, + os.EBADF => unreachable, // Dir is invalid or was opened without iteration ability os.EFAULT => unreachable, os.ENOTDIR => unreachable, os.EINVAL => unreachable, @@ -555,36 +555,6 @@ pub const Dir = struct { }; pub fn iterate(self: Dir) Iterator { - // Make sure the directory was not open with openDirTraverse - if (std.debug.runtime_safety) { - var ok = true; - - if (builtin.os.tag == .windows) { - const w = os.windows; - - var io_status_block: w.IO_STATUS_BLOCK = undefined; - var info: w.FILE_ACCESS_INFORMATION = undefined; - - const rc = w.ntdll.NtQueryInformationFile( - self.fd, - &io_status_block, - &info, - @sizeOf(w.FILE_ACCESS_INFORMATION), - .FileAccessInformation, - ); - assert(rc == .SUCCESS); - - ok = (info.AccessFlags & w.FILE_LIST_DIRECTORY) != 0; - } else if (@hasDecl(os, "O_PATH")) { - const f = os.fcntl(self.fd, os.F_GETFL, 0) catch unreachable; - ok = (f & os.O_PATH) == 0; - } - - if (!ok) { - std.debug.panic("iterate() called on Dir open with openDirTraverse", .{}); - } - } - switch (builtin.os.tag) { .macosx, .ios, .freebsd, .netbsd, .dragonfly => return Iterator{ .dir = self, @@ -626,16 +596,6 @@ pub const Dir = struct { DeviceBusy, } || os.UnexpectedError; - /// Deprecated; call `cwd().openDirList` directly. - pub fn open(dir_path: []const u8) OpenError!Dir { - return cwd().openDirList(dir_path); - } - - /// Deprecated; call `cwd().openDirListC` directly. - pub fn openC(dir_path_c: [*:0]const u8) OpenError!Dir { - return cwd().openDirListC(dir_path_c); - } - pub fn close(self: *Dir) void { if (need_async_thread) { std.event.Loop.instance.?.close(self.fd); @@ -822,79 +782,61 @@ pub const Dir = struct { try os.fchdir(self.fd); } - /// Deprecated; call `openDirList` directly. - pub fn openDir(self: Dir, sub_path: []const u8) OpenError!Dir { - return self.openDirList(sub_path); - } + pub const OpenDirOptions = struct { + /// `true` means the opened directory can be used as the `Dir` parameter + /// for functions which operate based on an open directory handle. When `false`, + /// such operations are Illegal Behavior. + access_sub_paths: bool = true, - /// Deprecated; call `openDirListC` directly. - pub fn openDirC(self: Dir, sub_path_c: [*:0]const u8) OpenError!Dir { - return self.openDirListC(sub_path_c); - } + /// `true` means the opened directory can be scanned for the files and sub-directories + /// of the result. It means the `iterate` function can be called. + iterate: bool = false, + }; - /// 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. + /// Opens a directory at the given path. The directory is a system resource that remains + /// open until `close` is called on the result. /// /// Asserts that the path parameter has no null bytes. - /// TODO collapse this and `openDirList` into one function with an options parameter - pub fn openDirTraverse(self: Dir, sub_path: []const u8) OpenError!Dir { + pub fn openDir(self: Dir, sub_path: []const u8, args: OpenDirOptions) OpenError!Dir { if (builtin.os.tag == .windows) { const sub_path_w = try os.windows.sliceToPrefixedFileW(sub_path); - return self.openDirTraverseW(&sub_path_w); + return self.openDirW(&sub_path_w, args); + } else { + const sub_path_c = try os.toPosixPath(sub_path); + return self.openDirC(&sub_path_c, args); } - - const sub_path_c = try os.toPosixPath(sub_path); - return self.openDirTraverseC(&sub_path_c); } - /// 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. - /// - /// Asserts that the path parameter has no null bytes. - /// TODO collapse this and `openDirTraverse` into one function with an options parameter - pub fn openDirList(self: Dir, sub_path: []const u8) OpenError!Dir { - if (builtin.os.tag == .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 { + /// Same as `openDir` except the parameter is null-terminated. + pub fn openDirC(self: Dir, sub_path_c: [*:0]const u8, args: OpenDirOptions) OpenError!Dir { if (builtin.os.tag == .windows) { const sub_path_w = try os.windows.cStrToPrefixedFileW(sub_path_c); - return self.openDirTraverseW(&sub_path_w); - } else { + return self.openDirW(&sub_path_w, args); + } else if (!args.iterate) { const O_PATH = if (@hasDecl(os, "O_PATH")) os.O_PATH else 0; - return self.openDirFlagsC(sub_path_c, os.O_RDONLY | os.O_CLOEXEC | O_PATH); - } - } - - /// 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.tag == .windows) { - const sub_path_w = try os.windows.cStrToPrefixedFileW(sub_path_c); - return self.openDirListW(&sub_path_w); + return self.openDirFlagsC(sub_path_c, os.O_DIRECTORY | os.O_RDONLY | os.O_CLOEXEC | O_PATH); } else { - return self.openDirFlagsC(sub_path_c, os.O_RDONLY | os.O_CLOEXEC); + return self.openDirFlagsC(sub_path_c, os.O_DIRECTORY | os.O_RDONLY | os.O_CLOEXEC); } } + /// Same as `openDir` except the path parameter is WTF-16 encoded, NT-prefixed. + /// This function asserts the target OS is Windows. + pub fn openDirW(self: Dir, sub_path_w: [*:0]const u16, args: OpenDirOptions) OpenError!Dir { + const w = os.windows; + // TODO remove some of these flags if args.access_sub_paths is false + const base_flags = w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | + w.SYNCHRONIZE | w.FILE_TRAVERSE; + const flags: u32 = if (args.iterate) base_flags else base_flags | w.FILE_LIST_DIRECTORY; + return self.openDirAccessMaskW(sub_path_w, flags); + } + + /// `flags` must contain `os.O_DIRECTORY`. fn openDirFlagsC(self: Dir, sub_path_c: [*:0]const u8, flags: u32) OpenError!Dir { - const os_flags = flags | os.O_DIRECTORY; const result = if (need_async_thread) - std.event.Loop.instance.?.openatZ(self.fd, sub_path_c, os_flags, 0) + std.event.Loop.instance.?.openatZ(self.fd, sub_path_c, flags, 0) else - os.openatC(self.fd, sub_path_c, os_flags, 0); + os.openatC(self.fd, sub_path_c, flags, 0); const fd = result catch |err| switch (err) { error.FileTooBig => unreachable, // can't happen for directories error.IsDir => unreachable, // we're providing O_DIRECTORY @@ -905,22 +847,6 @@ pub const Dir = struct { return Dir{ .fd = fd }; } - /// 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); - } - fn openDirAccessMaskW(self: Dir, sub_path_w: [*:0]const u16, access_mask: u32) OpenError!Dir { const w = os.windows; @@ -1141,7 +1067,7 @@ pub const Dir = struct { error.Unexpected, => |e| return e, } - var dir = self.openDirList(sub_path) catch |err| switch (err) { + var dir = self.openDir(sub_path, .{ .iterate = true }) catch |err| switch (err) { error.NotDir => { if (got_access_denied) { return error.AccessDenied; @@ -1174,7 +1100,6 @@ pub const Dir = struct { var dir_name_buf: [MAX_PATH_BYTES]u8 = undefined; var dir_name: []const u8 = sub_path; - var parent_dir = self; // Here we must avoid recursion, in order to provide O(1) memory guarantee of this function. // Go through each entry and if it is not a directory, delete it. If it is a directory, @@ -1206,7 +1131,7 @@ pub const Dir = struct { => |e| return e, } - const new_dir = dir.openDirList(entry.name) catch |err| switch (err) { + const new_dir = dir.openDir(entry.name, .{ .iterate = true }) catch |err| switch (err) { error.NotDir => { if (got_access_denied) { return error.AccessDenied; @@ -1491,7 +1416,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.openDirList(base.name) catch |err| switch (err) { + var new_dir = top.dir_it.dir.openDir(base.name, .{ .iterate = true }) catch |err| switch (err) { error.NameTooLong => unreachable, // no path sep in base.name else => |e| return e, }; @@ -1529,7 +1454,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 cwd().openDirList(dir_path); + var dir = try cwd().openDir(dir_path, .{ .iterate = true }); 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 b0452f4d66..ae1df6802b 100644 --- a/lib/std/os/test.zig +++ b/lib/std/os/test.zig @@ -21,7 +21,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.cwd().openDirTraverse("os_test_tmp")) |dir| { + if (fs.cwd().openDir("os_test_tmp", .{})) |dir| { @panic("expected error"); } else |err| { expect(err == error.FileNotFound); @@ -49,7 +49,7 @@ test "sendfile" { try fs.cwd().makePath("os_test_tmp"); defer fs.deleteTree("os_test_tmp") catch {}; - var dir = try fs.cwd().openDirList("os_test_tmp"); + var dir = try fs.cwd().openDir("os_test_tmp", .{}); defer dir.close(); const line1 = "line1\n"; @@ -455,7 +455,10 @@ test "fcntl" { const test_out_file = "os_tmp_test"; const file = try fs.cwd().createFile(test_out_file, .{}); - defer file.close(); + defer { + file.close(); + fs.cwd().deleteFile(test_out_file) catch {}; + } // Note: The test assumes createFile opens the file with O_CLOEXEC { @@ -472,6 +475,4 @@ test "fcntl" { const flags = try os.fcntl(file.handle, os.F_GETFD, 0); expect((flags & os.FD_CLOEXEC) != 0); } - - try fs.cwd().deleteFile(test_out_file); } diff --git a/lib/std/zig/system.zig b/lib/std/zig/system.zig index 88b3022bb2..5c47b68838 100644 --- a/lib/std/zig/system.zig +++ b/lib/std/zig/system.zig @@ -754,7 +754,7 @@ pub const NativeTargetInfo = struct { const rpath_list = mem.toSliceConst(u8, @ptrCast([*:0]u8, strtab[rpoff..].ptr)); var it = mem.tokenize(rpath_list, ":"); while (it.next()) |rpath| { - var dir = fs.cwd().openDirList(rpath) catch |err| switch (err) { + var dir = fs.cwd().openDir(rpath, .{}) catch |err| switch (err) { error.NameTooLong => unreachable, error.InvalidUtf8 => unreachable, error.BadPathName => unreachable, diff --git a/src-self-hosted/libc_installation.zig b/src-self-hosted/libc_installation.zig index c07ef03b51..996f705060 100644 --- a/src-self-hosted/libc_installation.zig +++ b/src-self-hosted/libc_installation.zig @@ -280,7 +280,7 @@ pub const LibCInstallation = struct { // search in reverse order const search_path_untrimmed = search_paths.at(search_paths.len - path_i - 1); const search_path = std.mem.trimLeft(u8, search_path_untrimmed, " "); - var search_dir = fs.cwd().openDirList(search_path) catch |err| switch (err) { + var search_dir = fs.cwd().openDir(search_path, .{}) catch |err| switch (err) { error.FileNotFound, error.NotDir, error.NoDevice, @@ -335,7 +335,7 @@ pub const LibCInstallation = struct { const stream = result_buf.outStream(); try stream.print("{}\\Include\\{}\\ucrt", .{ search.path, search.version }); - var dir = fs.cwd().openDirList(result_buf.toSliceConst()) catch |err| switch (err) { + var dir = fs.cwd().openDir(result_buf.toSliceConst(), .{}) catch |err| switch (err) { error.FileNotFound, error.NotDir, error.NoDevice, @@ -382,7 +382,7 @@ pub const LibCInstallation = struct { const stream = result_buf.outStream(); try stream.print("{}\\Lib\\{}\\ucrt\\{}", .{ search.path, search.version, arch_sub_dir }); - var dir = fs.cwd().openDirList(result_buf.toSliceConst()) catch |err| switch (err) { + var dir = fs.cwd().openDir(result_buf.toSliceConst(), .{}) catch |err| switch (err) { error.FileNotFound, error.NotDir, error.NoDevice, @@ -437,7 +437,7 @@ pub const LibCInstallation = struct { const stream = result_buf.outStream(); try stream.print("{}\\Lib\\{}\\um\\{}", .{ search.path, search.version, arch_sub_dir }); - var dir = fs.cwd().openDirList(result_buf.toSliceConst()) catch |err| switch (err) { + var dir = fs.cwd().openDir(result_buf.toSliceConst(), .{}) catch |err| switch (err) { error.FileNotFound, error.NotDir, error.NoDevice, @@ -475,7 +475,7 @@ pub const LibCInstallation = struct { try result_buf.append("\\include"); - var dir = fs.cwd().openDirList(result_buf.toSliceConst()) catch |err| switch (err) { + var dir = fs.cwd().openDir(result_buf.toSliceConst(), .{}) catch |err| switch (err) { error.FileNotFound, error.NotDir, error.NoDevice, diff --git a/src-self-hosted/main.zig b/src-self-hosted/main.zig index 42a0873483..f5faa26615 100644 --- a/src-self-hosted/main.zig +++ b/src-self-hosted/main.zig @@ -734,7 +734,7 @@ async fn fmtPath(fmt: *Fmt, file_path_ref: []const u8, check_mode: bool) FmtErro max_src_size, ) catch |err| switch (err) { error.IsDir, error.AccessDenied => { - var dir = try fs.cwd().openDirList(file_path); + var dir = try fs.cwd().openDir(file_path, .{ .iterate = true }); defer dir.close(); var group = event.Group(FmtError!void).init(fmt.allocator); diff --git a/src-self-hosted/print_targets.zig b/src-self-hosted/print_targets.zig index ad506425d2..997cfcfddc 100644 --- a/src-self-hosted/print_targets.zig +++ b/src-self-hosted/print_targets.zig @@ -72,7 +72,7 @@ pub fn cmdTargets( }; defer allocator.free(zig_lib_dir); - var dir = try std.fs.cwd().openDirList(zig_lib_dir); + var dir = try std.fs.cwd().openDir(zig_lib_dir, .{}); defer dir.close(); const vers_txt = try dir.readFileAlloc(allocator, "libc/glibc/vers.txt", 10 * 1024); diff --git a/src-self-hosted/stage2.zig b/src-self-hosted/stage2.zig index 38dfbaf072..5393310f9e 100644 --- a/src-self-hosted/stage2.zig +++ b/src-self-hosted/stage2.zig @@ -319,7 +319,7 @@ fn fmtPath(fmt: *Fmt, file_path: []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.cwd().openDirList(file_path); + var dir = try fs.cwd().openDir(file_path, .{ .iterate = true }); defer dir.close(); var dir_it = dir.iterate(); From 46ffc798b6d9bb7be8b016ef7529092d647151ee Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 18 Mar 2020 16:09:06 -0400 Subject: [PATCH 5/5] fix swapped logic for Windows Remove `std.fs.deleteTree`. Callers instead should use `std.fs.cwd().deleteTree`. Add `std.fs.deleteTreeAbsolute` for when the caller has an absolute path. --- doc/docgen.zig | 2 +- lib/std/build.zig | 4 +-- lib/std/fs.zig | 48 ++++++++++++++++----------------- lib/std/fs/watch.zig | 2 +- lib/std/os/test.zig | 6 ++--- src-self-hosted/compilation.zig | 3 +-- src-self-hosted/test.zig | 4 +-- test/cli.zig | 2 +- 8 files changed, 34 insertions(+), 37 deletions(-) diff --git a/doc/docgen.zig b/doc/docgen.zig index 4b9b94dbe4..32ad0cdc5d 100644 --- a/doc/docgen.zig +++ b/doc/docgen.zig @@ -48,7 +48,7 @@ pub fn main() !void { var toc = try genToc(allocator, &tokenizer); try fs.cwd().makePath(tmp_dir_name); - defer fs.deleteTree(tmp_dir_name) catch {}; + defer fs.cwd().deleteTree(tmp_dir_name) catch {}; try genHtml(allocator, &tokenizer, &toc, buffered_out_stream.outStream(), zig_exe); try buffered_out_stream.flush(); diff --git a/lib/std/build.zig b/lib/std/build.zig index 7585fe20e2..fd727c321e 100644 --- a/lib/std/build.zig +++ b/lib/std/build.zig @@ -377,7 +377,7 @@ pub const Builder = struct { if (self.verbose) { warn("rm {}\n", .{full_path}); } - fs.deleteTree(full_path) catch {}; + fs.cwd().deleteTree(full_path) catch {}; } // TODO remove empty directories @@ -2365,7 +2365,7 @@ pub const RemoveDirStep = struct { const self = @fieldParentPtr(RemoveDirStep, "step", step); const full_path = self.builder.pathFromRoot(self.dir_path); - fs.deleteTree(full_path) catch |err| { + fs.cwd().deleteTree(full_path) catch |err| { warn("Unable to remove {}: {}\n", .{ full_path, @errorName(err) }); return err; }; diff --git a/lib/std/fs.zig b/lib/std/fs.zig index e12c35ab8e..bc02fd0c91 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -261,28 +261,6 @@ pub fn deleteDirW(dir_path: [*:0]const u16) !void { return os.rmdirW(dir_path); } -/// Removes a symlink, file, or directory. -/// If `full_path` is relative, this is equivalent to `Dir.deleteTree` with the -/// current working directory as the open directory handle. -/// If `full_path` is absolute, this is equivalent to `Dir.deleteTree` with the -/// base directory. -pub fn deleteTree(full_path: []const u8) !void { - if (path.isAbsolute(full_path)) { - const dirname = path.dirname(full_path) orelse return error{ - /// Attempt to remove the root file system path. - /// This error is unreachable if `full_path` is relative. - CannotDeleteRootDirectory, - }.CannotDeleteRootDirectory; - - var dir = try cwd().openDir(dirname, .{}); - defer dir.close(); - - return dir.deleteTree(path.basename(full_path)); - } else { - return cwd().deleteTree(full_path); - } -} - pub const Dir = struct { fd: os.fd_t, @@ -518,7 +496,8 @@ pub const Dir = struct { self.end_index = io.Information; switch (rc) { .SUCCESS => {}, - .ACCESS_DENIED => return error.AccessDenied, + .ACCESS_DENIED => return error.AccessDenied, // Double-check that the Dir was opened with iteration ability + else => return w.unexpectedStatus(rc), } } @@ -827,7 +806,7 @@ pub const Dir = struct { // TODO remove some of these flags if args.access_sub_paths is false const base_flags = w.STANDARD_RIGHTS_READ | w.FILE_READ_ATTRIBUTES | w.FILE_READ_EA | w.SYNCHRONIZE | w.FILE_TRAVERSE; - const flags: u32 = if (args.iterate) base_flags else base_flags | w.FILE_LIST_DIRECTORY; + const flags: u32 = if (args.iterate) base_flags | w.FILE_LIST_DIRECTORY else base_flags; return self.openDirAccessMaskW(sub_path_w, flags); } @@ -1304,7 +1283,7 @@ pub const Dir = struct { } }; -/// Returns an handle to the current working directory that is open for traversal. +/// Returns an handle to the current working directory. It is not opened with iteration capability. /// 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 { @@ -1382,6 +1361,25 @@ pub fn deleteFileAbsoluteW(absolute_path_w: [*:0]const u16) DeleteFileError!void return cwd().deleteFileW(absolute_path_w); } +/// Removes a symlink, file, or directory. +/// This is equivalent to `Dir.deleteTree` with the base directory. +/// Asserts that the path is absolute. See `Dir.deleteTree` for a function that +/// operates on both absolute and relative paths. +/// Asserts that the path parameter has no null bytes. +pub fn deleteTreeAbsolute(absolute_path: []const u8) !void { + assert(path.isAbsolute(absolute_path)); + const dirname = path.dirname(absolute_path) orelse return error{ + /// Attempt to remove the root file system path. + /// This error is unreachable if `absolute_path` is relative. + CannotDeleteRootDirectory, + }.CannotDeleteRootDirectory; + + var dir = try cwd().openDir(dirname, .{}); + defer dir.close(); + + return dir.deleteTree(path.basename(absolute_path)); +} + pub const Walker = struct { stack: std.ArrayList(StackItem), name_buffer: std.Buffer, diff --git a/lib/std/fs/watch.zig b/lib/std/fs/watch.zig index 3180240a72..96fe1538e4 100644 --- a/lib/std/fs/watch.zig +++ b/lib/std/fs/watch.zig @@ -619,7 +619,7 @@ test "write a file, watch it, write it again" { if (true) return error.SkipZigTest; try fs.cwd().makePath(test_tmp_dir); - defer os.deleteTree(test_tmp_dir) catch {}; + defer fs.cwd().deleteTree(test_tmp_dir) catch {}; const allocator = std.heap.page_allocator; return testFsWatch(&allocator); diff --git a/lib/std/os/test.zig b/lib/std/os/test.zig index ae1df6802b..14a865904b 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 fs.cwd().makePath("os_test_tmp" ++ fs.path.sep_str ++ "b" ++ fs.path.sep_str ++ "c"); 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"); + try fs.cwd().deleteTree("os_test_tmp"); if (fs.cwd().openDir("os_test_tmp", .{})) |dir| { @panic("expected error"); } else |err| { @@ -38,7 +38,7 @@ test "access file" { try io.writeFile("os_test_tmp" ++ fs.path.sep_str ++ "file.txt", ""); try os.access("os_test_tmp" ++ fs.path.sep_str ++ "file.txt", os.F_OK); - try fs.deleteTree("os_test_tmp"); + try fs.cwd().deleteTree("os_test_tmp"); } fn testThreadIdFn(thread_id: *Thread.Id) void { @@ -47,7 +47,7 @@ fn testThreadIdFn(thread_id: *Thread.Id) void { test "sendfile" { try fs.cwd().makePath("os_test_tmp"); - defer fs.deleteTree("os_test_tmp") catch {}; + defer fs.cwd().deleteTree("os_test_tmp") catch {}; var dir = try fs.cwd().openDir("os_test_tmp", .{}); defer dir.close(); diff --git a/src-self-hosted/compilation.zig b/src-self-hosted/compilation.zig index d9f635ebbc..03e71c102d 100644 --- a/src-self-hosted/compilation.zig +++ b/src-self-hosted/compilation.zig @@ -520,8 +520,7 @@ pub const Compilation = struct { if (comp.tmp_dir.getOrNull()) |tmp_dir_result| if (tmp_dir_result.*) |tmp_dir| { - // TODO evented I/O? - fs.deleteTree(tmp_dir) catch {}; + fs.cwd().deleteTree(tmp_dir) catch {}; } else |_| {}; } diff --git a/src-self-hosted/test.zig b/src-self-hosted/test.zig index e87164c9fb..b146d6607a 100644 --- a/src-self-hosted/test.zig +++ b/src-self-hosted/test.zig @@ -57,11 +57,11 @@ pub const TestContext = struct { errdefer allocator.free(self.zig_lib_dir); try std.fs.cwd().makePath(tmp_dir_name); - errdefer std.fs.deleteTree(tmp_dir_name) catch {}; + errdefer std.fs.cwd().deleteTree(tmp_dir_name) catch {}; } fn deinit(self: *TestContext) void { - std.fs.deleteTree(tmp_dir_name) catch {}; + std.fs.cwd().deleteTree(tmp_dir_name) catch {}; allocator.free(self.zig_lib_dir); self.zig_compiler.deinit(); } diff --git a/test/cli.zig b/test/cli.zig index 9bc4a21c90..4c067d16ae 100644 --- a/test/cli.zig +++ b/test/cli.zig @@ -36,7 +36,7 @@ pub fn main() !void { testMissingOutputPath, }; for (test_fns) |testFn| { - try fs.deleteTree(dir_path); + try fs.cwd().deleteTree(dir_path); try fs.cwd().makeDir(dir_path); try testFn(zig_exe, dir_path); }