From 5d55d8f12f11b8db07210d4ccafc7de6136457fa Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Wed, 30 Nov 2022 15:11:05 -0700 Subject: [PATCH] Use AsFd in copy_file_range And expose it on FreeBSD --- CHANGELOG.md | 11 ++++++ src/fcntl.rs | 81 +++++++++++++++++++++++++++++--------------- test/test_fcntl.rs | 83 +++++++++++++++++++++++++--------------------- 3 files changed, 112 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 308f929025..44f9ccc1d7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,8 @@ This project adheres to [Semantic Versioning](https://semver.org/). - Added `SO_RTABLE` for OpenBSD and `SO_ACCEPTFILTER` for FreeBSD/NetBSD to `nix::sys::socket::sockopt`. ([#2085](https://github.com/nix-rust/nix/pull/2085)) - Removed `flock` from `::nix::fcntl` on Solaris. ([#2082](https://github.com/nix-rust/nix/pull/2082)) +- Use I/O safety with `copy_file_range`, and expose it on FreeBSD. + (#[1906](https://github.com/nix-rust/nix/pull/1906)) ### Changed @@ -44,6 +46,15 @@ This project adheres to [Semantic Versioning](https://semver.org/). - `nix::socket` and `nix::select` are now available on Redox. ([#2012](https://github.com/nix-rust/nix/pull/2012)) +- Implemented I/O safety. Many public functions argument and return types have + changed: + | Original Type | New Type | + | ------------- | --------------------- | + | AsRawFd | AsFd | + | RawFd | BorrowedFd or OwnedFd | + + (#[1906](https://github.com/nix-rust/nix/pull/1906)) + ### Fixed - Fix: send `ETH_P_ALL` in htons format ([#1925](https://github.com/nix-rust/nix/pull/1925)) diff --git a/src/fcntl.rs b/src/fcntl.rs index 85c8e1a49e..9bfecda5ac 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -5,11 +5,19 @@ use std::ffi::OsString; use std::os::raw; use std::os::unix::ffi::OsStringExt; use std::os::unix::io::RawFd; +// For splice and copy_file_range +#[cfg(any( + target_os = "android", + target_os = "freebsd", + target_os = "linux" +))] +use std::{ + os::unix::io::{AsFd, AsRawFd}, + ptr, +}; #[cfg(feature = "fs")] use crate::{sys::stat::Mode, NixPath, Result}; -#[cfg(any(target_os = "android", target_os = "linux"))] -use std::ptr; // For splice and copy_file_range #[cfg(any( target_os = "linux", @@ -612,44 +620,65 @@ feature! { /// /// The `copy_file_range` system call performs an in-kernel copy between /// file descriptors `fd_in` and `fd_out` without the additional cost of -/// transferring data from the kernel to user space and then back into the -/// kernel. It copies up to `len` bytes of data from file descriptor `fd_in` to -/// file descriptor `fd_out`, overwriting any data that exists within the -/// requested range of the target file. +/// transferring data from the kernel to user space and back again. There may be +/// additional optimizations for specific file systems. It copies up to `len` +/// bytes of data from file descriptor `fd_in` to file descriptor `fd_out`, +/// overwriting any data that exists within the requested range of the target +/// file. /// /// If the `off_in` and/or `off_out` arguments are used, the values /// will be mutated to reflect the new position within the file after -/// copying. If they are not used, the relevant filedescriptors will be seeked +/// copying. If they are not used, the relevant file descriptors will be seeked /// to the new position. /// /// On successful completion the number of bytes actually copied will be /// returned. -#[cfg(any(target_os = "android", target_os = "linux"))] -pub fn copy_file_range( - fd_in: RawFd, - off_in: Option<&mut libc::loff_t>, - fd_out: RawFd, - off_out: Option<&mut libc::loff_t>, +// Note: FreeBSD defines the offset argument as "off_t". Linux and Android +// define it as "loff_t". But on both OSes, on all supported platforms, those +// are 64 bits. So Nix uses i64 to make the docs simple and consistent. +#[cfg(any(target_os = "android", target_os = "freebsd", target_os = "linux"))] +pub fn copy_file_range( + fd_in: Fd1, + off_in: Option<&mut i64>, + fd_out: Fd2, + off_out: Option<&mut i64>, len: usize, ) -> Result { let off_in = off_in - .map(|offset| offset as *mut libc::loff_t) + .map(|offset| offset as *mut i64) .unwrap_or(ptr::null_mut()); let off_out = off_out - .map(|offset| offset as *mut libc::loff_t) + .map(|offset| offset as *mut i64) .unwrap_or(ptr::null_mut()); - let ret = unsafe { - libc::syscall( - libc::SYS_copy_file_range, - fd_in, - off_in, - fd_out, - off_out, - len, - 0, - ) - }; + cfg_if::cfg_if! { + if #[cfg(target_os = "freebsd")] { + let ret = unsafe { + libc::copy_file_range( + fd_in.as_fd().as_raw_fd(), + off_in, + fd_out.as_fd().as_raw_fd(), + off_out, + len, + 0, + ) + }; + } else { + // May Linux distros still don't include copy_file_range in their + // libc implementations, so we need to make a direct syscall. + let ret = unsafe { + libc::syscall( + libc::SYS_copy_file_range, + fd_in, + off_in, + fd_out.as_fd().as_raw_fd(), + off_out, + len, + 0, + ) + }; + } + } Errno::result(ret).map(|r| r as usize) } diff --git a/test/test_fcntl.rs b/test/test_fcntl.rs index 31df6d038f..5fef04ba9b 100644 --- a/test/test_fcntl.rs +++ b/test/test_fcntl.rs @@ -26,7 +26,7 @@ use std::io::prelude::*; #[cfg(not(target_os = "redox"))] use std::os::unix::fs; #[cfg(not(target_os = "redox"))] -use tempfile::{self, NamedTempFile}; +use tempfile::NamedTempFile; #[test] #[cfg(not(target_os = "redox"))] @@ -227,6 +227,51 @@ fn test_readlink() { ); } +/// This test creates a temporary file containing the contents +/// 'foobarbaz' and uses the `copy_file_range` call to transfer +/// 3 bytes at offset 3 (`bar`) to another empty file at offset 0. The +/// resulting file is read and should contain the contents `bar`. +/// The from_offset should be updated by the call to reflect +/// the 3 bytes read (6). +#[cfg(any( + target_os = "linux", + // Not available until FreeBSD 13.0 + all(target_os = "freebsd", fbsd14), + target_os = "android" +))] +#[test] +// QEMU does not support copy_file_range. Skip under qemu +#[cfg_attr(qemu, ignore)] +fn test_copy_file_range() { + use nix::fcntl::copy_file_range; + use std::os::unix::io::AsFd; + + const CONTENTS: &[u8] = b"foobarbaz"; + + let mut tmp1 = tempfile::tempfile().unwrap(); + let mut tmp2 = tempfile::tempfile().unwrap(); + + tmp1.write_all(CONTENTS).unwrap(); + tmp1.flush().unwrap(); + + let mut from_offset: i64 = 3; + copy_file_range( + tmp1.as_fd(), + Some(&mut from_offset), + tmp2.as_fd(), + None, + 3, + ) + .unwrap(); + + let mut res: String = String::new(); + tmp2.rewind().unwrap(); + tmp2.read_to_string(&mut res).unwrap(); + + assert_eq!(res, String::from("bar")); + assert_eq!(from_offset, 6); +} + #[cfg(any(target_os = "linux", target_os = "android"))] mod linux_android { use libc::loff_t; @@ -243,42 +288,6 @@ mod linux_android { use crate::*; - /// This test creates a temporary file containing the contents - /// 'foobarbaz' and uses the `copy_file_range` call to transfer - /// 3 bytes at offset 3 (`bar`) to another empty file at offset 0. The - /// resulting file is read and should contain the contents `bar`. - /// The from_offset should be updated by the call to reflect - /// the 3 bytes read (6). - #[test] - // QEMU does not support copy_file_range. Skip under qemu - #[cfg_attr(qemu, ignore)] - fn test_copy_file_range() { - const CONTENTS: &[u8] = b"foobarbaz"; - - let mut tmp1 = tempfile().unwrap(); - let mut tmp2 = tempfile().unwrap(); - - tmp1.write_all(CONTENTS).unwrap(); - tmp1.flush().unwrap(); - - let mut from_offset: i64 = 3; - copy_file_range( - tmp1.as_raw_fd(), - Some(&mut from_offset), - tmp2.as_raw_fd(), - None, - 3, - ) - .unwrap(); - - let mut res: String = String::new(); - tmp2.rewind().unwrap(); - tmp2.read_to_string(&mut res).unwrap(); - - assert_eq!(res, String::from("bar")); - assert_eq!(from_offset, 6); - } - #[test] fn test_splice() { const CONTENTS: &[u8] = b"abcdef123456";