From 0f248e0988b24bf4fcdaadd0e47127462206711e Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 3 Oct 2020 12:31:17 +0200 Subject: [PATCH 1/5] std: Make file copy ops use zero-copy mechanisms Use copy_file_range on Linux (if available), fcopyfile on Darwin, sendfile on *BSDs (and on Linux kernels without copy_file_range). --- lib/std/c/darwin.zig | 10 +++++ lib/std/fs.zig | 2 +- lib/std/os.zig | 101 ++++++++++++++++++++++++++++++++++++++----- 3 files changed, 102 insertions(+), 11 deletions(-) diff --git a/lib/std/c/darwin.zig b/lib/std/c/darwin.zig index ed1ddb7d91b7..5b305d60e6d4 100644 --- a/lib/std/c/darwin.zig +++ b/lib/std/c/darwin.zig @@ -18,6 +18,16 @@ pub extern "c" fn _dyld_get_image_header(image_index: u32) ?*mach_header; pub extern "c" fn _dyld_get_image_vmaddr_slide(image_index: u32) usize; pub extern "c" fn _dyld_get_image_name(image_index: u32) [*:0]const u8; +pub const COPYFILE_ACL = 1 << 0; +pub const COPYFILE_STAT = 1 << 1; +pub const COPYFILE_XATTR = 1 << 2; +pub const COPYFILE_DATA = 1 << 3; + +pub const copyfile_state_t = *@Type(.Opaque); +pub extern "c" fn copyfile_state_alloc() copyfile_state_t; +pub extern "c" fn copyfile_state_free(state: copyfile_state_t) c_int; +pub extern "c" fn fcopyfile(from: fd_t, to: fd_t, state: ?copyfile_state_t, flags: u32) c_int; + pub extern "c" fn @"realpath$DARWIN_EXTSN"(noalias file_name: [*:0]const u8, noalias resolved_name: [*]u8) ?[*:0]u8; pub extern "c" fn __getdirentries64(fd: c_int, buf_ptr: [*]u8, buf_len: usize, basep: *i64) isize; diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 1890d7e1360c..75f0709df60c 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1823,7 +1823,7 @@ pub const Dir = struct { var atomic_file = try dest_dir.atomicFile(dest_path, .{ .mode = mode }); defer atomic_file.deinit(); - try atomic_file.file.writeFileAll(in_file, .{ .in_len = size }); + try os.copy_file(in_file.handle, atomic_file.file.handle, .{ .file_size = size }); return atomic_file.finish(); } diff --git a/lib/std/os.zig b/lib/std/os.zig index c06ce4ed005b..c8d27eaa85b6 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -4981,19 +4981,15 @@ pub const CopyFileRangeError = error{ pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len: usize, flags: u32) CopyFileRangeError!usize { const use_c = std.c.versionCheck(.{ .major = 2, .minor = 27, .patch = 0 }).ok; - // TODO support for other systems than linux - const try_syscall = comptime std.Target.current.os.isAtLeast(.linux, .{ .major = 4, .minor = 5 }) != false; - - if (use_c or try_syscall) { + if (std.Target.current.os.tag == .linux and + (use_c or has_copy_file_range_syscall.get() != 0)) + { const sys = if (use_c) std.c else linux; var off_in_copy = @bitCast(i64, off_in); var off_out_copy = @bitCast(i64, off_out); const rc = sys.copy_file_range(fd_in, &off_in_copy, fd_out, &off_out_copy, len, flags); - - // TODO avoid wasting a syscall every time if kernel is too old and returns ENOSYS https://github.com/ziglang/zig/issues/1018 - switch (sys.getErrno(rc)) { 0 => return @intCast(usize, rc), EBADF => unreachable, @@ -5005,9 +5001,14 @@ pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len EOVERFLOW => return error.Unseekable, EPERM => return error.PermissionDenied, ETXTBSY => return error.FileBusy, - EINVAL => {}, // these may not be regular files, try fallback - EXDEV => {}, // support for cross-filesystem copy added in Linux 5.3, use fallback - ENOSYS => {}, // syscall added in Linux 4.5, use fallback + // these may not be regular files, try fallback + EINVAL => {}, + // support for cross-filesystem copy added in Linux 5.3, use fallback + EXDEV => {}, + // syscall added in Linux 4.5, use fallback + ENOSYS => { + has_copy_file_range_syscall.set(0); + }, else => |err| return unexpectedErrno(err), } } @@ -5021,6 +5022,86 @@ pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len return pwrite(fd_out, buf[0..amt_read], off_out); } +var has_copy_file_range_syscall = std.atomic.Int(u1).init(1); + +pub const CopyFileOptions = struct { + /// Size in bytes of the source files, if available saves a call to stat(). + file_size: ?u64 = null, +}; + +pub const CopyFileError = error{ + SystemResources, + FileTooBig, + InputOutput, + IsDir, + OutOfMemory, + NoSpaceLeft, + Unseekable, + PermissionDenied, + FileBusy, +} || FStatError || SendFileError; + +/// Transfer all the data between two file descriptors in the most efficient way. +/// No metadata is transferred over. +pub fn copy_file(fd_in: fd_t, fd_out: fd_t, options: CopyFileOptions) CopyFileError!void { + if (comptime std.Target.current.isDarwin()) { + const rc = system.fcopyfile(fd_in, fd_out, null, system.COPYFILE_DATA); + switch (errno(rc)) { + 0 => return, + EINVAL => unreachable, + ENOMEM => return error.SystemResources, + // The source file was not a directory, symbolic link, or regular file. + // Try with the fallback path before giving up. + ENOTSUP => {}, + else => |err| return unexpectedErrno(err), + } + } + + const src_file_size = options.file_size orelse + @bitCast(u64, (try fstat(fd_in)).size); + var remaining = src_file_size; + + if (std.Target.current.os.tag == .linux) { + // Try copy_file_range first as that works at the FS level and is the + // most efficient method (if available). + if (has_copy_file_range_syscall.get() != 0) { + cfr_loop: while (remaining > 0) { + const copy_amt = math.cast(usize, remaining) catch math.maxInt(usize); + const rc = linux.copy_file_range(fd_in, null, fd_out, null, copy_amt, 0); + switch (errno(rc)) { + 0 => {}, + EBADF => unreachable, + EFBIG => return error.FileTooBig, + EIO => return error.InputOutput, + EISDIR => return error.IsDir, + ENOMEM => return error.OutOfMemory, + ENOSPC => return error.NoSpaceLeft, + EOVERFLOW => return error.Unseekable, + EPERM => return error.PermissionDenied, + ETXTBSY => return error.FileBusy, + // these may not be regular files, try fallback + EINVAL => break :cfr_loop, + // support for cross-filesystem copy added in Linux 5.3, use fallback + EXDEV => break :cfr_loop, + // syscall added in Linux 4.5, use fallback + ENOSYS => { + has_copy_file_range_syscall.set(0); + break :cfr_loop; + }, + else => |err| return unexpectedErrno(err), + } + remaining -= rc; + } + return; + } + } + + // Sendfile is a zero-copy mechanism iff the OS supports it, otherwise the + // fallback code will copy the contents chunk by chunk. + const empty_iovec = [0]iovec_const{}; + _ = try sendfile(fd_out, fd_in, 0, remaining, &empty_iovec, &empty_iovec, 0); +} + pub const PollError = error{ /// The kernel had no space to allocate file descriptor tables. SystemResources, From 8b4f5f039df65980d0a0ec6add1caf4c9bf2468c Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Sat, 3 Oct 2020 19:51:22 +0200 Subject: [PATCH 2/5] Alternative strategy to avoid calling stat() This is an optimization as it avoids an extra syscall, but it's also a workaround for fstat being not available on Windows. --- lib/std/fs.zig | 2 +- lib/std/os.zig | 39 +++++++++++++++++++++------------------ 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 75f0709df60c..c00976e01c30 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1823,7 +1823,7 @@ pub const Dir = struct { var atomic_file = try dest_dir.atomicFile(dest_path, .{ .mode = mode }); defer atomic_file.deinit(); - try os.copy_file(in_file.handle, atomic_file.file.handle, .{ .file_size = size }); + try os.copy_file(in_file.handle, atomic_file.file.handle, .{}); return atomic_file.finish(); } diff --git a/lib/std/os.zig b/lib/std/os.zig index c8d27eaa85b6..60845a3b87d9 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -5024,12 +5024,10 @@ pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len var has_copy_file_range_syscall = std.atomic.Int(u1).init(1); -pub const CopyFileOptions = struct { - /// Size in bytes of the source files, if available saves a call to stat(). - file_size: ?u64 = null, -}; +pub const CopyFileOptions = struct {}; pub const CopyFileError = error{ + BadFileHandle, SystemResources, FileTooBig, InputOutput, @@ -5057,20 +5055,18 @@ pub fn copy_file(fd_in: fd_t, fd_out: fd_t, options: CopyFileOptions) CopyFileEr } } - const src_file_size = options.file_size orelse - @bitCast(u64, (try fstat(fd_in)).size); - var remaining = src_file_size; - if (std.Target.current.os.tag == .linux) { // Try copy_file_range first as that works at the FS level and is the // most efficient method (if available). if (has_copy_file_range_syscall.get() != 0) { - cfr_loop: while (remaining > 0) { - const copy_amt = math.cast(usize, remaining) catch math.maxInt(usize); - const rc = linux.copy_file_range(fd_in, null, fd_out, null, copy_amt, 0); + cfr_loop: while (true) { + // The kernel checks `file_pos+count` for overflow, use a 32 bit + // value so that the syscall won't return EINVAL except for + // impossibly large files. + const rc = linux.copy_file_range(fd_in, null, fd_out, null, math.maxInt(u32), 0); switch (errno(rc)) { 0 => {}, - EBADF => unreachable, + EBADF => return error.BadFileHandle, EFBIG => return error.FileTooBig, EIO => return error.InputOutput, EISDIR => return error.IsDir, @@ -5079,27 +5075,34 @@ pub fn copy_file(fd_in: fd_t, fd_out: fd_t, options: CopyFileOptions) CopyFileEr EOVERFLOW => return error.Unseekable, EPERM => return error.PermissionDenied, ETXTBSY => return error.FileBusy, - // these may not be regular files, try fallback + // These may not be regular files, try fallback EINVAL => break :cfr_loop, - // support for cross-filesystem copy added in Linux 5.3, use fallback + // Support for cross-filesystem copy added in Linux 5.3, use fallback EXDEV => break :cfr_loop, - // syscall added in Linux 4.5, use fallback + // Syscall added in Linux 4.5, use fallback ENOSYS => { has_copy_file_range_syscall.set(0); break :cfr_loop; }, else => |err| return unexpectedErrno(err), } - remaining -= rc; + // Terminate when no data was copied + if (rc == 0) return; } - return; + // This point is reached when an error occurred, hopefully no data + // was transferred yet } } // Sendfile is a zero-copy mechanism iff the OS supports it, otherwise the // fallback code will copy the contents chunk by chunk. const empty_iovec = [0]iovec_const{}; - _ = try sendfile(fd_out, fd_in, 0, remaining, &empty_iovec, &empty_iovec, 0); + var offset: u64 = 0; + sendfile_loop: while (true) { + const amt = try sendfile(fd_out, fd_in, offset, 0, &empty_iovec, &empty_iovec, 0); + if (amt == 0) break :sendfile_loop; + offset += amt; + } } pub const PollError = error{ From a419a1aabce63fedcc77a1118d596864fcff46e3 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Tue, 6 Oct 2020 09:38:59 +0200 Subject: [PATCH 3/5] Move copy_file to fs namespace Now it is a private API. Also handle short writes in copy_file_range fallback implementation. --- lib/std/fs.zig | 48 ++++++++++++++++++++- lib/std/os.zig | 110 ++++++++++--------------------------------------- 2 files changed, 69 insertions(+), 89 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index c00976e01c30..92f68590f95b 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -1823,7 +1823,7 @@ pub const Dir = struct { var atomic_file = try dest_dir.atomicFile(dest_path, .{ .mode = mode }); defer atomic_file.deinit(); - try os.copy_file(in_file.handle, atomic_file.file.handle, .{}); + try copy_file(in_file.handle, atomic_file.file.handle); return atomic_file.finish(); } @@ -2263,6 +2263,52 @@ pub fn realpathAlloc(allocator: *Allocator, pathname: []const u8) ![]u8 { return allocator.dupe(u8, try os.realpath(pathname, &buf)); } +const CopyFileError = error{SystemResources} || os.CopyFileRangeError || os.SendFileError; + +/// Transfer all the data between two file descriptors in the most efficient way. +/// No metadata is transferred over. +fn copy_file(fd_in: os.fd_t, fd_out: os.fd_t) CopyFileError!void { + if (comptime std.Target.current.isDarwin()) { + const rc = os.system.fcopyfile(fd_in, fd_out, null, os.system.COPYFILE_DATA); + switch (errno(rc)) { + 0 => return, + EINVAL => unreachable, + ENOMEM => return error.SystemResources, + // The source file was not a directory, symbolic link, or regular file. + // Try with the fallback path before giving up. + ENOTSUP => {}, + else => |err| return unexpectedErrno(err), + } + } + + if (std.Target.current.os.tag == .linux) { + // Try copy_file_range first as that works at the FS level and is the + // most efficient method (if available). + var offset: u64 = 0; + cfr_loop: while (true) { + // The kernel checks `offset+count` for overflow, use a 32 bit + // value so that the syscall won't return EINVAL except for + // impossibly large files. + const amt = try os.copy_file_range(fd_in, offset, fd_out, offset, math.maxInt(u32), 0); + // Terminate when no data was copied + if (amt == 0) break :cfr_loop; + offset += amt; + } + return; + } + + // Sendfile is a zero-copy mechanism iff the OS supports it, otherwise the + // fallback code will copy the contents chunk by chunk. + const empty_iovec = [0]os.iovec_const{}; + var offset: u64 = 0; + sendfile_loop: while (true) { + const amt = try os.sendfile(fd_out, fd_in, offset, 0, &empty_iovec, &empty_iovec, 0); + // Terminate when no data was copied + if (amt == 0) break :sendfile_loop; + offset += amt; + } +} + test "" { if (builtin.os.tag != .wasi) { _ = makeDirAbsolute; diff --git a/lib/std/os.zig b/lib/std/os.zig index 60845a3b87d9..e4e0b1d4a14f 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -4945,6 +4945,7 @@ pub fn sendfile( pub const CopyFileRangeError = error{ FileTooBig, InputOutput, + InvalidFileDescriptor, IsDir, OutOfMemory, NoSpaceLeft, @@ -4978,6 +4979,11 @@ pub const CopyFileRangeError = error{ /// Other systems fall back to calling `pread` / `pwrite`. /// /// Maximum offsets on Linux are `math.maxInt(i64)`. +var has_copy_file_range_syscall = init: { + const kernel_has_syscall = comptime std.Target.current.os.isAtLeast(.linux, .{ .major = 4, .minor = 5 }) orelse true; + break :init std.atomic.Int(u1).init(@boolToInt(kernel_has_syscall)); +}; + pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len: usize, flags: u32) CopyFileRangeError!usize { const use_c = std.c.versionCheck(.{ .major = 2, .minor = 27, .patch = 0 }).ok; @@ -4992,7 +4998,7 @@ pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len const rc = sys.copy_file_range(fd_in, &off_in_copy, fd_out, &off_out_copy, len, flags); switch (sys.getErrno(rc)) { 0 => return @intCast(usize, rc), - EBADF => unreachable, + EBADF => return error.InvalidFileDescriptor, EFBIG => return error.FileTooBig, EIO => return error.InputOutput, EISDIR => return error.IsDir, @@ -5013,96 +5019,24 @@ pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len } } - var buf: [8 * 4096]u8 = undefined; - const adjusted_count = math.min(buf.len, len); - const amt_read = try pread(fd_in, buf[0..adjusted_count], off_in); - // TODO without @as the line below fails to compile for wasm32-wasi: - // error: integer value 0 cannot be coerced to type 'os.PWriteError!usize' - if (amt_read == 0) return @as(usize, 0); - return pwrite(fd_out, buf[0..amt_read], off_out); -} - -var has_copy_file_range_syscall = std.atomic.Int(u1).init(1); - -pub const CopyFileOptions = struct {}; - -pub const CopyFileError = error{ - BadFileHandle, - SystemResources, - FileTooBig, - InputOutput, - IsDir, - OutOfMemory, - NoSpaceLeft, - Unseekable, - PermissionDenied, - FileBusy, -} || FStatError || SendFileError; + var buf: [2 * 4096]u8 = undefined; -/// Transfer all the data between two file descriptors in the most efficient way. -/// No metadata is transferred over. -pub fn copy_file(fd_in: fd_t, fd_out: fd_t, options: CopyFileOptions) CopyFileError!void { - if (comptime std.Target.current.isDarwin()) { - const rc = system.fcopyfile(fd_in, fd_out, null, system.COPYFILE_DATA); - switch (errno(rc)) { - 0 => return, - EINVAL => unreachable, - ENOMEM => return error.SystemResources, - // The source file was not a directory, symbolic link, or regular file. - // Try with the fallback path before giving up. - ENOTSUP => {}, - else => |err| return unexpectedErrno(err), - } - } - - if (std.Target.current.os.tag == .linux) { - // Try copy_file_range first as that works at the FS level and is the - // most efficient method (if available). - if (has_copy_file_range_syscall.get() != 0) { - cfr_loop: while (true) { - // The kernel checks `file_pos+count` for overflow, use a 32 bit - // value so that the syscall won't return EINVAL except for - // impossibly large files. - const rc = linux.copy_file_range(fd_in, null, fd_out, null, math.maxInt(u32), 0); - switch (errno(rc)) { - 0 => {}, - EBADF => return error.BadFileHandle, - EFBIG => return error.FileTooBig, - EIO => return error.InputOutput, - EISDIR => return error.IsDir, - ENOMEM => return error.OutOfMemory, - ENOSPC => return error.NoSpaceLeft, - EOVERFLOW => return error.Unseekable, - EPERM => return error.PermissionDenied, - ETXTBSY => return error.FileBusy, - // These may not be regular files, try fallback - EINVAL => break :cfr_loop, - // Support for cross-filesystem copy added in Linux 5.3, use fallback - EXDEV => break :cfr_loop, - // Syscall added in Linux 4.5, use fallback - ENOSYS => { - has_copy_file_range_syscall.set(0); - break :cfr_loop; - }, - else => |err| return unexpectedErrno(err), - } - // Terminate when no data was copied - if (rc == 0) return; - } - // This point is reached when an error occurred, hopefully no data - // was transferred yet - } + var total_copied: usize = 0; + var read_off = off_in; + var write_off = off_out; + while (total_copied < len) { + const adjusted_count = math.min(buf.len, len - total_copied); + const amt_read = try pread(fd_in, buf[0..adjusted_count], read_off); + if (amt_read == 0) break; + const amt_written = try pwrite(fd_out, buf[0..amt_read], write_off); + // pwrite may write less than the specified amount, handle the remaining + // chunk of data in the next iteration + read_off += amt_written; + write_off += amt_written; + total_copied += amt_written; } - // Sendfile is a zero-copy mechanism iff the OS supports it, otherwise the - // fallback code will copy the contents chunk by chunk. - const empty_iovec = [0]iovec_const{}; - var offset: u64 = 0; - sendfile_loop: while (true) { - const amt = try sendfile(fd_out, fd_in, offset, 0, &empty_iovec, &empty_iovec, 0); - if (amt == 0) break :sendfile_loop; - offset += amt; - } + return total_copied; } pub const PollError = error{ From 1f7ec0de706eded887bc8dbcf5f45a5546d1be5c Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Tue, 6 Oct 2020 11:57:23 +0200 Subject: [PATCH 4/5] Address review comments & fix compilation errors --- lib/std/fs.zig | 18 +++++++++--------- lib/std/os.zig | 14 +++++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 92f68590f95b..6b4f9384dd17 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -2270,14 +2270,14 @@ const CopyFileError = error{SystemResources} || os.CopyFileRangeError || os.Send fn copy_file(fd_in: os.fd_t, fd_out: os.fd_t) CopyFileError!void { if (comptime std.Target.current.isDarwin()) { const rc = os.system.fcopyfile(fd_in, fd_out, null, os.system.COPYFILE_DATA); - switch (errno(rc)) { + switch (os.errno(rc)) { 0 => return, - EINVAL => unreachable, - ENOMEM => return error.SystemResources, - // The source file was not a directory, symbolic link, or regular file. + os.EINVAL => unreachable, + os.ENOMEM => return error.SystemResources, + // The source file is not a directory, symbolic link, or regular file. // Try with the fallback path before giving up. - ENOTSUP => {}, - else => |err| return unexpectedErrno(err), + os.ENOTSUP => {}, + else => |err| return os.unexpectedErrno(err), } } @@ -2286,9 +2286,9 @@ fn copy_file(fd_in: os.fd_t, fd_out: os.fd_t) CopyFileError!void { // most efficient method (if available). var offset: u64 = 0; cfr_loop: while (true) { - // The kernel checks `offset+count` for overflow, use a 32 bit - // value so that the syscall won't return EINVAL except for - // impossibly large files. + // The kernel checks the u64 value `offset+count` for overflow, use + // a 32 bit value so that the syscall won't return EINVAL except for + // impossibly large files (> 2^64-1 - 2^32-1). const amt = try os.copy_file_range(fd_in, offset, fd_out, offset, math.maxInt(u32), 0); // Terminate when no data was copied if (amt == 0) break :cfr_loop; diff --git a/lib/std/os.zig b/lib/std/os.zig index e4e0b1d4a14f..0e81b73f8c00 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -4954,6 +4954,11 @@ pub const CopyFileRangeError = error{ FileBusy, } || PReadError || PWriteError || UnexpectedError; +var has_copy_file_range_syscall = init: { + const kernel_has_syscall = comptime std.Target.current.os.isAtLeast(.linux, .{ .major = 4, .minor = 5 }) orelse true; + break :init std.atomic.Int(bool).init(kernel_has_syscall); +}; + /// Transfer data between file descriptors at specified offsets. /// Returns the number of bytes written, which can less than requested. /// @@ -4979,16 +4984,11 @@ pub const CopyFileRangeError = error{ /// Other systems fall back to calling `pread` / `pwrite`. /// /// Maximum offsets on Linux are `math.maxInt(i64)`. -var has_copy_file_range_syscall = init: { - const kernel_has_syscall = comptime std.Target.current.os.isAtLeast(.linux, .{ .major = 4, .minor = 5 }) orelse true; - break :init std.atomic.Int(u1).init(@boolToInt(kernel_has_syscall)); -}; - pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len: usize, flags: u32) CopyFileRangeError!usize { const use_c = std.c.versionCheck(.{ .major = 2, .minor = 27, .patch = 0 }).ok; if (std.Target.current.os.tag == .linux and - (use_c or has_copy_file_range_syscall.get() != 0)) + (use_c or has_copy_file_range_syscall.get())) { const sys = if (use_c) std.c else linux; @@ -5013,7 +5013,7 @@ pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len EXDEV => {}, // syscall added in Linux 4.5, use fallback ENOSYS => { - has_copy_file_range_syscall.set(0); + has_copy_file_range_syscall.set(false); }, else => |err| return unexpectedErrno(err), } From 03762da2af8753ecf7f4bc2005dd00d570c51ae7 Mon Sep 17 00:00:00 2001 From: LemonBoy Date: Wed, 7 Oct 2020 11:13:26 +0200 Subject: [PATCH 5/5] New review round --- lib/std/c/darwin.zig | 2 -- lib/std/fs.zig | 5 +++-- lib/std/os.zig | 33 ++++++++++++--------------------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/lib/std/c/darwin.zig b/lib/std/c/darwin.zig index 5b305d60e6d4..93efdd061cfb 100644 --- a/lib/std/c/darwin.zig +++ b/lib/std/c/darwin.zig @@ -24,8 +24,6 @@ pub const COPYFILE_XATTR = 1 << 2; pub const COPYFILE_DATA = 1 << 3; pub const copyfile_state_t = *@Type(.Opaque); -pub extern "c" fn copyfile_state_alloc() copyfile_state_t; -pub extern "c" fn copyfile_state_free(state: copyfile_state_t) c_int; pub extern "c" fn fcopyfile(from: fd_t, to: fd_t, state: ?copyfile_state_t, flags: u32) c_int; pub extern "c" fn @"realpath$DARWIN_EXTSN"(noalias file_name: [*:0]const u8, noalias resolved_name: [*]u8) ?[*:0]u8; diff --git a/lib/std/fs.zig b/lib/std/fs.zig index 6b4f9384dd17..5f49bb376682 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -2265,8 +2265,9 @@ pub fn realpathAlloc(allocator: *Allocator, pathname: []const u8) ![]u8 { const CopyFileError = error{SystemResources} || os.CopyFileRangeError || os.SendFileError; -/// Transfer all the data between two file descriptors in the most efficient way. -/// No metadata is transferred over. +// Transfer all the data between two file descriptors in the most efficient way. +// The copy starts at offset 0, the initial offsets are preserved. +// No metadata is transferred over. fn copy_file(fd_in: os.fd_t, fd_out: os.fd_t) CopyFileError!void { if (comptime std.Target.current.isDarwin()) { const rc = os.system.fcopyfile(fd_in, fd_out, null, os.system.COPYFILE_DATA); diff --git a/lib/std/os.zig b/lib/std/os.zig index 0e81b73f8c00..6e254518798d 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -4945,7 +4945,9 @@ pub fn sendfile( pub const CopyFileRangeError = error{ FileTooBig, InputOutput, - InvalidFileDescriptor, + /// `fd_in` is not open for reading; or `fd_out` is not open for writing; + /// or the `O_APPEND` flag is set for `fd_out`. + FilesOpenedWithWrongFlags, IsDir, OutOfMemory, NoSpaceLeft, @@ -4955,7 +4957,7 @@ pub const CopyFileRangeError = error{ } || PReadError || PWriteError || UnexpectedError; var has_copy_file_range_syscall = init: { - const kernel_has_syscall = comptime std.Target.current.os.isAtLeast(.linux, .{ .major = 4, .minor = 5 }) orelse true; + const kernel_has_syscall = std.Target.current.os.isAtLeast(.linux, .{ .major = 4, .minor = 5 }) orelse true; break :init std.atomic.Int(bool).init(kernel_has_syscall); }; @@ -4998,7 +5000,7 @@ pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len const rc = sys.copy_file_range(fd_in, &off_in_copy, fd_out, &off_out_copy, len, flags); switch (sys.getErrno(rc)) { 0 => return @intCast(usize, rc), - EBADF => return error.InvalidFileDescriptor, + EBADF => return error.FilesOpenedWithWrongFlags, EFBIG => return error.FileTooBig, EIO => return error.InputOutput, EISDIR => return error.IsDir, @@ -5019,24 +5021,13 @@ pub fn copy_file_range(fd_in: fd_t, off_in: u64, fd_out: fd_t, off_out: u64, len } } - var buf: [2 * 4096]u8 = undefined; - - var total_copied: usize = 0; - var read_off = off_in; - var write_off = off_out; - while (total_copied < len) { - const adjusted_count = math.min(buf.len, len - total_copied); - const amt_read = try pread(fd_in, buf[0..adjusted_count], read_off); - if (amt_read == 0) break; - const amt_written = try pwrite(fd_out, buf[0..amt_read], write_off); - // pwrite may write less than the specified amount, handle the remaining - // chunk of data in the next iteration - read_off += amt_written; - write_off += amt_written; - total_copied += amt_written; - } - - return total_copied; + var buf: [8 * 4096]u8 = undefined; + const adjusted_count = math.min(buf.len, len); + const amt_read = try pread(fd_in, buf[0..adjusted_count], off_in); + // TODO without @as the line below fails to compile for wasm32-wasi: + // error: integer value 0 cannot be coerced to type 'os.PWriteError!usize' + if (amt_read == 0) return @as(usize, 0); + return pwrite(fd_out, buf[0..amt_read], off_out); } pub const PollError = error{