Skip to content

Commit

Permalink
std.fs.rename: fix Windows implementation
Browse files Browse the repository at this point in the history
The semantics of this function are that it moves both files and
directories. Previously we had this `is_dir` boolean field of
`std.os.windows.OpenFile` which required the API user to choose: are we
opening a file or directory? And the other kind would either cause
error.IsDir or error.NotDir. But that is not a limitation of the Windows
file system API; it was self-imposed.

On Windows, rename is implemented internally with `NtCreateFile` so we
need to allow it to open either files or directories. This is now done
by `std.os.windows.OpenFile` accepting enum{file_only,dir_only,any}
instead of a boolean.
  • Loading branch information
andrewrk committed Jan 3, 2022
1 parent b4d6e85 commit d3f87f8
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 12 deletions.
2 changes: 1 addition & 1 deletion lib/std/fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1361,7 +1361,7 @@ pub const Dir = struct {
.share_access = share_access,
.creation = creation,
.io_mode = .blocking,
.open_dir = true,
.filter = .dir_only,
}) catch |er| switch (er) {
error.WouldBlock => unreachable,
else => |e2| return e2,
Expand Down
2 changes: 1 addition & 1 deletion lib/std/fs/watch.zig
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ pub fn Watch(comptime V: type) type {
.access_mask = windows.FILE_LIST_DIRECTORY,
.creation = windows.FILE_OPEN,
.io_mode = .evented,
.open_dir = true,
.filter = .dir_only,
});
errdefer windows.CloseHandle(dir_handle);

Expand Down
11 changes: 6 additions & 5 deletions lib/std/os.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1353,7 +1353,7 @@ fn openOptionsFromFlags(flags: u32) windows.OpenFileOptions {
access_mask |= w.GENERIC_READ | w.GENERIC_WRITE;
}

const open_dir: bool = flags & O.DIRECTORY != 0;
const filter: windows.OpenFileOptions.Filter = if (flags & O.DIRECTORY != 0) .dir_only else .file_only;
const follow_symlinks: bool = flags & O.NOFOLLOW == 0;

const creation: w.ULONG = blk: {
Expand All @@ -1369,7 +1369,7 @@ fn openOptionsFromFlags(flags: u32) windows.OpenFileOptions {
.access_mask = access_mask,
.io_mode = .blocking,
.creation = creation,
.open_dir = open_dir,
.filter = filter,
.follow_symlinks = follow_symlinks,
};
}
Expand Down Expand Up @@ -2324,6 +2324,7 @@ pub fn renameatW(
.access_mask = windows.SYNCHRONIZE | windows.GENERIC_WRITE | windows.DELETE,
.creation = windows.FILE_OPEN,
.io_mode = .blocking,
.filter = .any, // This function is supposed to rename both files and directories.
}) catch |err| switch (err) {
error.WouldBlock => unreachable, // Not possible without `.share_access_nonblocking = true`.
else => |e| return e,
Expand Down Expand Up @@ -2435,7 +2436,7 @@ pub fn mkdiratW(dir_fd: fd_t, sub_path_w: []const u16, mode: u32) MakeDirError!v
.access_mask = windows.GENERIC_READ | windows.SYNCHRONIZE,
.creation = windows.FILE_CREATE,
.io_mode = .blocking,
.open_dir = true,
.filter = .dir_only,
}) catch |err| switch (err) {
error.IsDir => unreachable,
error.PipeBusy => unreachable,
Expand Down Expand Up @@ -2511,7 +2512,7 @@ pub fn mkdirW(dir_path_w: []const u16, mode: u32) MakeDirError!void {
.access_mask = windows.GENERIC_READ | windows.SYNCHRONIZE,
.creation = windows.FILE_CREATE,
.io_mode = .blocking,
.open_dir = true,
.filter = .dir_only,
}) catch |err| switch (err) {
error.IsDir => unreachable,
error.PipeBusy => unreachable,
Expand Down Expand Up @@ -4693,7 +4694,7 @@ pub fn realpathW(pathname: []const u16, out_buffer: *[MAX_PATH_BYTES]u8) RealPat
.share_access = share_access,
.creation = creation,
.io_mode = .blocking,
.open_dir = true,
.filter = .dir_only,
}) catch |er| switch (er) {
error.WouldBlock => unreachable,
else => |e2| return e2,
Expand Down
23 changes: 18 additions & 5 deletions lib/std/os/windows.zig
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,26 @@ pub const OpenFileOptions = struct {
io_mode: std.io.ModeOverride,
/// If true, tries to open path as a directory.
/// Defaults to false.
open_dir: bool = false,
filter: Filter = .file_only,
/// If false, tries to open path as a reparse point without dereferencing it.
/// Defaults to true.
follow_symlinks: bool = true,

pub const Filter = enum {
/// Causes `OpenFile` to return `error.IsDir` if the opened handle would be a directory.
file_only,
/// Causes `OpenFile` to return `error.NotDir` if the opened handle would be a file.
dir_only,
/// `OpenFile` does not discriminate between opening files and directories.
any,
};
};

pub fn OpenFile(sub_path_w: []const u16, options: OpenFileOptions) OpenError!HANDLE {
if (mem.eql(u16, sub_path_w, &[_]u16{'.'}) and !options.open_dir) {
if (mem.eql(u16, sub_path_w, &[_]u16{'.'}) and options.filter == .file_only) {
return error.IsDir;
}
if (mem.eql(u16, sub_path_w, &[_]u16{ '.', '.' }) and !options.open_dir) {
if (mem.eql(u16, sub_path_w, &[_]u16{ '.', '.' }) and options.filter == .file_only) {
return error.IsDir;
}

Expand All @@ -87,7 +96,11 @@ pub fn OpenFile(sub_path_w: []const u16, options: OpenFileOptions) OpenError!HAN
};
var io: IO_STATUS_BLOCK = undefined;
const blocking_flag: ULONG = if (options.io_mode == .blocking) FILE_SYNCHRONOUS_IO_NONALERT else 0;
const file_or_dir_flag: ULONG = if (options.open_dir) FILE_DIRECTORY_FILE else FILE_NON_DIRECTORY_FILE;
const file_or_dir_flag: ULONG = switch (options.filter) {
.file_only => FILE_NON_DIRECTORY_FILE,
.dir_only => FILE_DIRECTORY_FILE,
.any => 0,
};
// If we're not following symlinks, we need to ensure we don't pass in any synchronization flags such as FILE_SYNCHRONOUS_IO_NONALERT.
const flags: ULONG = if (options.follow_symlinks) file_or_dir_flag | blocking_flag else file_or_dir_flag | FILE_OPEN_REPARSE_POINT;

Expand Down Expand Up @@ -695,7 +708,7 @@ pub fn CreateSymbolicLink(
.dir = dir,
.creation = FILE_CREATE,
.io_mode = .blocking,
.open_dir = is_directory,
.filter = if (is_directory) .dir_only else .file_only,
}) catch |err| switch (err) {
error.IsDir => return error.PathAlreadyExists,
error.NotDir => unreachable,
Expand Down

0 comments on commit d3f87f8

Please sign in to comment.