Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std.os read/write functions + sendfile #4612

Merged
merged 5 commits into from
Mar 3, 2020
Merged

Conversation

andrewrk
Copy link
Member

@andrewrk andrewrk commented Mar 3, 2020

This merges #4131 and then does the following:

  • rework os.sendfile and add macosx support, and a fallback
    implementation for any OS.
  • fix sendto compile error
  • std.os write functions support partial writes. closes std.os.writev does not handle partial writes #3443.
  • std.os pread / pwrite functions can now return error.Unseekable.
  • std.fs.File read/write functions now have readAll/writeAll variants
    which loop to complete operations even when partial reads/writes
    happen.
  • Audit std.os read/write functions with respect to Linux returning
    EINVAL for lengths greater than 0x7fff0000.
  • std.os read/write shim functions do not unnecessarily loop. Since
    partial reads/writes are part of the API, the caller will be forced
    to loop anyway, and so that would just be code bloat.
  • Improve doc comments
  • Add a non-trivial test for std.os.sendfile
  • Fix std.os.pread on 32 bit Linux
  • Add missing SYS_sendfile bit on aarch64

terinjokes and others added 3 commits March 2, 2020 12:54
This changset adds a `sendfile(2)` syscall bindings to the linux bits
component. Where available, the `sendfile64(2)` syscall will be
transparently called.

A wrapping function has also been added to the std.os to transform
errno returns to Zig errors.

Change-Id: I86769fc4382c0771e3656e7b21137bafd99a4411
 * rework os.sendfile and add macosx support, and a fallback
   implementation for any OS.
 * fix sendto compile error
 * std.os write functions support partial writes. closes #3443.
 * std.os pread / pwrite functions can now return `error.Unseekable`.
 * std.fs.File read/write functions now have readAll/writeAll variants
   which loop to complete operations even when partial reads/writes
   happen.
 * Audit std.os read/write functions with respect to Linux returning
   EINVAL for lengths greater than 0x7fff0000.
 * std.os read/write shim functions do not unnecessarily loop. Since
   partial reads/writes are part of the API, the caller will be forced
   to loop anyway, and so that would just be code bloat.
 * Improve doc comments
 * Add a non-trivial test for std.os.sendfile
 * Fix std.os.pread on 32 bit Linux
 * Add missing SYS_sendfile bit on aarch64
@shawnl
Copy link
Contributor

shawnl commented Mar 3, 2020

What about copy_file_range(2) ?

@andrewrk
Copy link
Member Author

andrewrk commented Mar 3, 2020

What about copy_file_range(2) ?

That will be a nice separate enhancement

@@ -3745,7 +3746,7 @@ pub fn sendfile(
EPIPE => return error.BrokenPipe,

else => {
_ = unexpectedErrno(err);
const discard = unexpectedErrno(err);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need an assignment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it's an error

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hrm. why don't we support discarding error values (not error unions) via assignment to _?

@andrewrk andrewrk merged commit 226b801 into master Mar 3, 2020
@andrewrk andrewrk deleted the os-read-write-sendfile branch March 3, 2020 15:15
@andrewrk andrewrk added the breaking Implementing this issue could cause existing code to no longer compile or have different behavior. label Mar 3, 2020
@daurnimator
Copy link
Contributor

FYI this broke zig hello world examples all over the place as file.write now returns a !usize rather than a !void. Although streams got writeOnce+write, files didn't.

@EvanCarroll
Copy link

EvanCarroll commented Sep 18, 2023

Shouldn't this optimization also exist on fifo.pump?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

std.os.writev does not handle partial writes
5 participants