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

Adds implementation of copy_file_range() #1008

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
([#1010](https://github.com/nix-rust/nix/pull/1010))
- Added `nix::sys::signal::signal`.
([#817](https://github.com/nix-rust/nix/pull/817))
- Added `copy_file_range` wrapper
([#1008](https://github.com/nix-rust/nix/pull/1008))

### Changed
### Fixed
Expand Down
36 changes: 34 additions & 2 deletions src/fcntl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,9 @@ pub enum FcntlArg<'a> {
}
pub use self::FcntlArg::*;

#[cfg(any(target_os = "android", target_os = "linux"))]
use std::ptr;

// TODO: Figure out how to handle value fcntl returns
pub fn fcntl(fd: RawFd, arg: FcntlArg) -> Result<c_int> {
let res = unsafe {
Expand Down Expand Up @@ -305,6 +308,36 @@ pub fn flock(fd: RawFd, arg: FlockArg) -> Result<()> {
Errno::result(res).map(drop)
}

/// Copy a range of data from one file to another
///
/// The `copy_file_range` system call performs an in-kernel copy between
/// file descriptor `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.
///
/// 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
/// to the new position.
///
/// On 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 i64>,
fd_out: RawFd, off_out: Option<&mut i64>,
len: usize) -> Result<usize> {

let off_in = off_in.map(|offset| offset as *mut _).unwrap_or(ptr::null_mut());
let off_out = off_out.map(|offset| offset as *mut _).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)
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the last argument be flags instead of 0?

Copy link
Author

Choose a reason for hiding this comment

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

Yes.

Copy link
Author

Choose a reason for hiding this comment

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

Done (flags removed)

};
Errno::result(ret).map(|r| r as usize)
}

#[cfg(any(target_os = "android", target_os = "linux"))]
libc_bitflags! {
/// Additional flags to `splice` and friends.
Expand All @@ -329,8 +362,7 @@ libc_bitflags! {
#[cfg(any(target_os = "linux", target_os = "android"))]
pub fn splice(fd_in: RawFd, off_in: Option<&mut libc::loff_t>,
fd_out: RawFd, off_out: Option<&mut libc::loff_t>,
len: usize, flags: SpliceFFlags) -> Result<usize> {
use std::ptr;
len: usize, flags: SpliceFFlags) -> Result<usize> {
let off_in = off_in.map(|offset| offset as *mut _).unwrap_or(ptr::null_mut());
let off_out = off_out.map(|offset| offset as *mut _).unwrap_or(ptr::null_mut());

Expand Down
36 changes: 35 additions & 1 deletion test/test_fcntl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,49 @@ fn test_readlink() {
mod linux_android {
use std::io::prelude::*;
use std::os::unix::prelude::*;
use std::io::SeekFrom;

use libc::loff_t;

use nix::fcntl::{SpliceFFlags, FallocateFlags, fallocate, splice, tee, vmsplice};
use nix::fcntl::{SpliceFFlags, FallocateFlags, fallocate, splice, tee, vmsplice, copy_file_range};
use nix::sys::uio::IoVec;
use nix::unistd::{close, pipe, read, write};

use tempfile::{tempfile, NamedTempFile};

/// 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).
///
/// Fix me: test is disabled for linux based builds, because Jenkins
/// Linux version is too old for `copy_file_range`. Should work
/// on netbsd build.
#[test]
#[cfg_attr(target_env = "linux", ignore)]
Copy link
Member

Choose a reason for hiding this comment

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

Linux is a target_os, not a target_env. Also, you should leave a message with the ignore attribute. And it's not going to work on a "netbsd build", because NetBSD doesn't have this syscall at all. Finally, we use Travis, not Jenkins.

Choose a reason for hiding this comment

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

Is there anything else that is holding this PR back?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. The OP needs to address my last comments, fix the CI failures, and rebase.

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.seek(SeekFrom::Start(0)).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";
Expand Down