Merge pull request #1906 from asomers/copy_file_range

Expose copy_file_range on FreeBSD and use I/O Safety

Author: asomers

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))

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<Fd1: AsFd, Fd2: AsFd>(
+    fd_in: Fd1,
+    off_in: Option<&mut i64>,
+    fd_out: Fd2,
+    off_out: Option<&mut i64>,
     len: usize,
 ) -> Result<usize> {
     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)
 }
 

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";