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

Expose copy_file_range on FreeBSD and use I/O Safety #1906

Merged
merged 1 commit into from
Aug 11, 2023

Conversation

asomers
Copy link
Member

@asomers asomers commented Dec 4, 2022

Depends on #1862

@asomers asomers force-pushed the copy_file_range branch 4 times, most recently from 66a79a1 to fbf6470 Compare December 4, 2022 23:25
@asomers
Copy link
Member Author

asomers commented Dec 4, 2022

@rtzoeller suddenly I'm getting a bunch of errors on musl, android, and qemu. But I can't figure out why they didn't happen before this PR. Any guesses?

  • On android, libc::copy_file_range doesn't exist. But why didn't that cause problems before?
  • Same thing on musl.
  • On QEMU, undefined reference to copy_file_range'` at link time. But again, what changed?

Even before this PR, copy_file_range should've been enabled for all of those targets.

@asomers
Copy link
Member Author

asomers commented Dec 4, 2022

Oh, I see. The old version used libc::syscall(libc::SYS_copy_file_range, ...). Now I'm trying to call libc::copy_file_range(...). Silly Linux. I wonder if it actually worked before?

@asomers asomers marked this pull request as ready for review February 10, 2023 03:43
@asomers asomers force-pushed the copy_file_range branch 2 times, most recently from 63fa5ae to 9a59112 Compare August 7, 2023 02:25
@asomers asomers added this pull request to the merge queue Aug 7, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Aug 7, 2023
And expose it on FreeBSD
@asomers asomers added this pull request to the merge queue Aug 11, 2023
Merged via the queue into nix-rust:master with commit 9304788 Aug 11, 2023
39 checks passed
rkuris referenced this pull request in ava-labs/firewood Aug 28, 2023
Two major changes:
 - features must be included; we only need two (this will speed up
   compiles!)
 - FD safety; we now need an OwnedFd, so attempting to use the fd after
   the object is dropped is no longer valid. Additionally, we don't need
   to implement Drop, as OwnedFd already has a drop which calls close

References:

 * [https://github.com/nix-rust/nix/pull/2091](nix PR for features drop)
 * [https://doc.rust-lang.org/stable/src/std/os/fd/owned.rs.html#170-182](OwnedFd docs)
 * [https://github.com/nix-rust/nix/pull/1906](nix PR for FD I/O safety)
rkuris added a commit to ava-labs/firewood that referenced this pull request Aug 28, 2023
Two major changes:
 - features must be included; we only need two (this will speed up
   compiles!)
 - FD safety; we now need an OwnedFd, so attempting to use the fd after
   the object is dropped is no longer valid. Additionally, we don't need
   to implement Drop, as OwnedFd already has a drop which calls close

References:

 * nix-rust/nix#2091 - nix PR for features drop
 * https://doc.rust-lang.org/stable/src/std/os/fd/owned.rs.html#170-182 - OwnedFd docs
 * nix-rust/nix#1906 - nix PR for FD I/O safety
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant