From 9009c5dd1f28cdab5f9f8fc8432fb31ac30539ba Mon Sep 17 00:00:00 2001 From: arbelonson-source <269032023+arbelonson-source@users.noreply.github.com> Date: Mon, 31 Aug 2026 00:39:01 +0300 Subject: [PATCH 1/3] cp: take the standard copy to any non-regular destination Closes #14283. The sparse paths call `ftruncate` and write at explicit offsets, which only a regular file supports. Only a fifo destination was excluded, so a character device still took them and failed: $ cp --sparse=always file /dev/stdout > /dev/null cp: 'file' -> '/dev/stdout': Invalid argument GNU copies the contents normally. Widen the check from "is a fifo" to "is not a regular file", which covers character devices, sockets and block devices as well. A destination that does not exist yet is about to be created as a regular file, so it is still eligible. --- src/uu/cp/src/platform/linux.rs | 25 ++++++++++++++++--------- tests/by-util/test_cp.rs | 13 +++++++++++++ 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index fc32a4be5da..1ede361d1e3 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -8,7 +8,6 @@ use rustix::fs::{SeekFrom, ftruncate, ioctl_ficlone, seek}; use std::fs::File; use std::io::{self, Read}; use std::os::unix::fs::FileExt; -use std::os::unix::fs::FileTypeExt; use std::os::unix::fs::MetadataExt; use std::path::Path; @@ -224,10 +223,18 @@ fn sparse_copy_fd(src_file: &mut File, dst_file: &File, context: &str) -> CopyRe Ok(()) } -/// Checks whether an existing destination is a fifo -fn check_dest_is_fifo(dest: &Path) -> bool { - // If our destination file exists and its a fifo , we do a standard copy . - std::fs::metadata(dest).is_ok_and(|f| f.file_type().is_fifo()) +/// Checks whether an existing destination cannot be written to sparsely. +/// +/// The sparse paths call `ftruncate` and write at explicit offsets, neither of +/// which anything but a regular file supports. A fifo, a socket, or a character +/// device such as the `/dev/null` that `/dev/stdout` may resolve to all reject +/// `ftruncate` with `EINVAL`, so those take the standard copy instead, which is +/// what GNU does with them. +/// +/// A destination that does not exist yet is about to be created as a regular +/// file, so it is not excluded here. +fn dest_cannot_be_sparse(dest: &Path) -> bool { + std::fs::metadata(dest).is_ok_and(|f| !f.file_type().is_file()) } /// Copy the contents of a stream from `source` to `dest`. @@ -485,7 +492,7 @@ fn handle_reflink_auto_sparse_always( (true, false, _) => copy_debug.sparse_detection = SparseDebug::SeekHole, (_, _, _) => (), } - if check_dest_is_fifo(dest) { + if dest_cannot_be_sparse(dest) { copy_method = CopyMethod::FSCopy; } Ok((copy_debug, copy_method)) @@ -571,7 +578,7 @@ fn handle_reflink_auto_sparse_auto( copy_debug.sparse_detection = SparseDebug::SeekHole; } - if check_dest_is_fifo(dest) { + if dest_cannot_be_sparse(dest) { copy_method = CopyMethod::FSCopy; } Ok((copy_debug, copy_method)) @@ -607,7 +614,7 @@ fn handle_reflink_never_sparse_auto( copy_debug.sparse_detection = SparseDebug::SeekHole; } - if check_dest_is_fifo(dest) { + if dest_cannot_be_sparse(dest) { copy_method = CopyMethod::FSCopy; } Ok((copy_debug, copy_method)) @@ -652,7 +659,7 @@ fn handle_reflink_never_sparse_always( (_, _, _) => (), } - if check_dest_is_fifo(dest) { + if dest_cannot_be_sparse(dest) { copy_method = CopyMethod::FSCopy; } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index f216f52a3be..893befa9d46 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -8937,3 +8937,16 @@ fn test_progressbar_inexistent_source() { .fails_with_code(1) .stderr_contains("cp: cannot stat 'inexistent1': No such file or directory"); } + +#[test] +#[cfg(target_os = "linux")] +fn test_cp_sparse_always_to_character_device() { + // The sparse paths use ftruncate and positional writes, which a character + // device rejects with EINVAL. Only a fifo destination was excluded, so + // `cp --sparse=always FILE /dev/null` failed where GNU copies normally. + let (at, mut ucmd) = at_and_ucmd!(); + at.write("src.txt", "hello world\n"); + ucmd.args(&["--sparse=always", "src.txt", "/dev/null"]) + .succeeds() + .no_stderr(); +} From e5f0caa690b1fe2f7328233bbaf5d5c7d35c2abb Mon Sep 17 00:00:00 2001 From: arbelonson-source <269032023+arbelonson-source@users.noreply.github.com> Date: Mon, 31 Aug 2026 06:03:47 +0300 Subject: [PATCH 2/3] cp: discover a non-sparse destination instead of checking for one @oech3: "Please never check file type. Fallback when ftruncate failed to remove unnecessary overhead." / "Also `not` in the name of fn should be avoided and bool should be flipped even you want to keep the hack." `dest_cannot_be_sparse` cost every destination -- overwhelmingly a regular file, for which it always returned false -- a `metadata` call before any copy started, just to rule out the rare fifo/socket/character- device case. Removed, along with the `dest: &Path` parameter it needed in all four planning functions that called it. In its place, the two sparse-copy functions now catch their own first `ftruncate` failing with EINVAL and fall back to `buf_copy::copy_fast` right there. Nothing has been read from the source or written to the destination yet at that point, so falling back there cannot duplicate or corrupt output -- the same guarantee the old pre-check gave, discovered instead of assumed. The negated boolean predicate this replaces is gone rather than renamed, since a fallible action controlling what happens next reads better here than a bool a caller re-interprets afterward. --- src/uu/cp/src/platform/linux.rs | 83 ++++++++++++++++----------------- 1 file changed, 40 insertions(+), 43 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 1ede361d1e3..4d6e6186336 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -153,7 +153,9 @@ fn sparse_copy_without_hole_fd(src_file: &File, dst_file: &File, context: &str) let ctx_err = |e: io::Error| CpError::IoErrContext(e, context.to_owned()); let size = src_file.metadata().map_err(&ctx_err)?.size(); - ftruncate(dst_file, size).map_err(|e| CpError::IoErrContext(e.into(), context.to_owned()))?; + if let Err(e) = ftruncate(dst_file, size) { + return fall_back_if_cannot_be_sparse(e, src_file, dst_file, context); + } let mut current_offset = 0; // Maximize the data read at once to 16 MiB to avoid memory hogging with large files // 16 MiB chunks should saturate an SSD @@ -193,7 +195,9 @@ fn sparse_copy_fd(src_file: &mut File, dst_file: &File, context: &str) -> CopyRe // Keep the size as u64: on 32-bit targets a usize conversion would // panic for sources of 4 GiB and more. let size = src_file.metadata().map_err(&ctx_err)?.size(); - ftruncate(dst_file, size).map_err(|e| CpError::IoErrContext(e.into(), context.to_owned()))?; + if let Err(e) = ftruncate(dst_file, size) { + return fall_back_if_cannot_be_sparse(e, src_file, dst_file, context); + } let blksize = dst_file.metadata().map_err(&ctx_err)?.blksize(); let mut buf: Vec = vec![0; blksize as usize]; @@ -223,18 +227,35 @@ fn sparse_copy_fd(src_file: &mut File, dst_file: &File, context: &str) -> CopyRe Ok(()) } -/// Checks whether an existing destination cannot be written to sparsely. +/// Falls back to a plain copy when `error` -- from the `ftruncate` a sparse +/// copy opens with -- shows the destination cannot be written to sparsely at +/// all, or reports `error` as the reason the copy failed. /// /// The sparse paths call `ftruncate` and write at explicit offsets, neither of -/// which anything but a regular file supports. A fifo, a socket, or a character -/// device such as the `/dev/null` that `/dev/stdout` may resolve to all reject -/// `ftruncate` with `EINVAL`, so those take the standard copy instead, which is -/// what GNU does with them. +/// which anything but a regular file supports. A fifo, a socket, or a +/// character device such as the `/dev/null` that `/dev/stdout` may resolve to +/// all reject `ftruncate` with `EINVAL`, so those take the standard copy +/// instead, which is what GNU does with them. /// -/// A destination that does not exist yet is about to be created as a regular -/// file, so it is not excluded here. -fn dest_cannot_be_sparse(dest: &Path) -> bool { - std::fs::metadata(dest).is_ok_and(|f| !f.file_type().is_file()) +/// This is discovered here rather than checked in advance, since checking +/// first would cost every ordinary destination -- overwhelmingly a regular +/// file, for which this never fires -- a `metadata` call for no benefit. +/// Nothing has been read from `src_file` or written to `dst_file` yet at the +/// point every caller reaches this, so falling back here cannot duplicate or +/// corrupt output. +fn fall_back_if_cannot_be_sparse( + error: rustix::io::Errno, + src_file: &File, + dst_file: &File, + context: &str, +) -> CopyResult<()> { + if error != rustix::io::Errno::INVAL { + return Err(CpError::IoErrContext(error.into(), context.to_owned())); + } + let mut src = src_file; + let mut dst = dst_file; + buf_copy::copy_fast(&mut src, &mut dst) + .map_err(|e| CpError::IoErrContext(e, context.to_owned())) } /// Copy the contents of a stream from `source` to `dest`. @@ -321,7 +342,7 @@ pub(crate) fn copy_on_write( let mut src_file = open_source(source, nofollow) .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_never_sparse_always(&mut src_file, dest); + let result = handle_reflink_never_sparse_always(&mut src_file); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; @@ -363,7 +384,7 @@ pub(crate) fn copy_on_write( let mut src_file = open_source(source, nofollow) .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_never_sparse_auto(&mut src_file, dest); + let result = handle_reflink_never_sparse_auto(&mut src_file); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; @@ -389,7 +410,7 @@ pub(crate) fn copy_on_write( let mut src_file = open_source(source, nofollow) .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_auto_sparse_always(&mut src_file, dest); + let result = handle_reflink_auto_sparse_always(&mut src_file); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; @@ -428,7 +449,7 @@ pub(crate) fn copy_on_write( let mut src_file = open_source(source, nofollow) .map_err(|e| CpError::IoErrContext(e, context.to_owned()))?; let mut copy_method = CopyMethod::Default; - let result = handle_reflink_auto_sparse_auto(&mut src_file, dest); + let result = handle_reflink_auto_sparse_auto(&mut src_file); if let Ok((debug, method)) = result { copy_debug = debug; copy_method = method; @@ -464,10 +485,7 @@ pub(crate) fn copy_on_write( /// Handles debug results when flags are "--reflink=auto" and "--sparse=always" and specifies what /// type of copy should be used -fn handle_reflink_auto_sparse_always( - src_file: &mut File, - dest: &Path, -) -> io::Result<(CopyDebug, CopyMethod)> { +fn handle_reflink_auto_sparse_always(src_file: &mut File) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::Unsupported, @@ -492,9 +510,6 @@ fn handle_reflink_auto_sparse_always( (true, false, _) => copy_debug.sparse_detection = SparseDebug::SeekHole, (_, _, _) => (), } - if dest_cannot_be_sparse(dest) { - copy_method = CopyMethod::FSCopy; - } Ok((copy_debug, copy_method)) } @@ -543,10 +558,7 @@ fn handle_reflink_auto_sparse_never(src_file: &mut File) -> io::Result io::Result<(CopyDebug, CopyMethod)> { +fn handle_reflink_auto_sparse_auto(src_file: &mut File) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::Unsupported, @@ -578,18 +590,12 @@ fn handle_reflink_auto_sparse_auto( copy_debug.sparse_detection = SparseDebug::SeekHole; } - if dest_cannot_be_sparse(dest) { - copy_method = CopyMethod::FSCopy; - } Ok((copy_debug, copy_method)) } /// Handles debug results when flags are "--reflink=never" and "--sparse=auto" and specifies what /// type of copy should be used -fn handle_reflink_never_sparse_auto( - src_file: &mut File, - dest: &Path, -) -> io::Result<(CopyDebug, CopyMethod)> { +fn handle_reflink_never_sparse_auto(src_file: &mut File) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, @@ -614,18 +620,12 @@ fn handle_reflink_never_sparse_auto( copy_debug.sparse_detection = SparseDebug::SeekHole; } - if dest_cannot_be_sparse(dest) { - copy_method = CopyMethod::FSCopy; - } Ok((copy_debug, copy_method)) } /// Handles debug results when flags are "--reflink=never" and "--sparse=always" and specifies what /// type of copy should be used -fn handle_reflink_never_sparse_always( - src_file: &mut File, - dest: &Path, -) -> io::Result<(CopyDebug, CopyMethod)> { +fn handle_reflink_never_sparse_always(src_file: &mut File) -> io::Result<(CopyDebug, CopyMethod)> { let mut copy_debug = CopyDebug { offload: OffloadReflinkDebug::Unknown, reflink: OffloadReflinkDebug::No, @@ -659,9 +659,6 @@ fn handle_reflink_never_sparse_always( (_, _, _) => (), } - if dest_cannot_be_sparse(dest) { - copy_method = CopyMethod::FSCopy; - } Ok((copy_debug, copy_method)) } From 41ef79da7e5140fb9e1dbc67c5f447cc17d2b5fa Mon Sep 17 00:00:00 2001 From: arbelonson-source <269032023+arbelonson-source@users.noreply.github.com> Date: Mon, 31 Aug 2026 06:57:41 +0300 Subject: [PATCH 3/3] cp: inline the sparse-copy EINVAL fallback at its two call sites fall_back_if_cannot_be_sparse was only ever called from sparse_copy_without_hole_fd and sparse_copy_fd, each already inside a match/if on ftruncate's result -- pull it into a match at each call site instead of a third, indirected function. AI-assisted-by: Claude Opus 5, via Claude Code --- src/uu/cp/src/platform/linux.rs | 62 ++++++++++++++------------------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/src/uu/cp/src/platform/linux.rs b/src/uu/cp/src/platform/linux.rs index 4d6e6186336..015d6dddf6e 100644 --- a/src/uu/cp/src/platform/linux.rs +++ b/src/uu/cp/src/platform/linux.rs @@ -153,8 +153,24 @@ fn sparse_copy_without_hole_fd(src_file: &File, dst_file: &File, context: &str) let ctx_err = |e: io::Error| CpError::IoErrContext(e, context.to_owned()); let size = src_file.metadata().map_err(&ctx_err)?.size(); - if let Err(e) = ftruncate(dst_file, size) { - return fall_back_if_cannot_be_sparse(e, src_file, dst_file, context); + // A fifo, a socket, or a character device such as the `/dev/null` that + // `/dev/stdout` may resolve to all reject `ftruncate` with `EINVAL` + // since they support neither it nor the writes at explicit offsets the + // rest of this function makes; take the standard copy instead, which is + // what GNU does with them. Discovered here rather than checked in + // advance, since checking first would cost every ordinary destination + // -- overwhelmingly a regular file, for which this never fires -- a + // `metadata` call for no benefit. Nothing has been read from `src_file` + // or written to `dst_file` yet, so falling back here cannot duplicate + // or corrupt output. + match ftruncate(dst_file, size) { + Ok(()) => {} + Err(rustix::io::Errno::INVAL) => { + let mut src = src_file; + let mut dst = dst_file; + return buf_copy::copy_fast(&mut src, &mut dst).map_err(&ctx_err); + } + Err(e) => return Err(CpError::IoErrContext(e.into(), context.to_owned())), } let mut current_offset = 0; // Maximize the data read at once to 16 MiB to avoid memory hogging with large files @@ -195,8 +211,15 @@ fn sparse_copy_fd(src_file: &mut File, dst_file: &File, context: &str) -> CopyRe // Keep the size as u64: on 32-bit targets a usize conversion would // panic for sources of 4 GiB and more. let size = src_file.metadata().map_err(&ctx_err)?.size(); - if let Err(e) = ftruncate(dst_file, size) { - return fall_back_if_cannot_be_sparse(e, src_file, dst_file, context); + // See the matching comment in `sparse_copy_without_hole_fd` for why an + // `EINVAL` here falls back to a plain copy instead of erroring. + match ftruncate(dst_file, size) { + Ok(()) => {} + Err(rustix::io::Errno::INVAL) => { + let mut dst = dst_file; + return buf_copy::copy_fast(src_file, &mut dst).map_err(&ctx_err); + } + Err(e) => return Err(CpError::IoErrContext(e.into(), context.to_owned())), } let blksize = dst_file.metadata().map_err(&ctx_err)?.blksize(); @@ -227,37 +250,6 @@ fn sparse_copy_fd(src_file: &mut File, dst_file: &File, context: &str) -> CopyRe Ok(()) } -/// Falls back to a plain copy when `error` -- from the `ftruncate` a sparse -/// copy opens with -- shows the destination cannot be written to sparsely at -/// all, or reports `error` as the reason the copy failed. -/// -/// The sparse paths call `ftruncate` and write at explicit offsets, neither of -/// which anything but a regular file supports. A fifo, a socket, or a -/// character device such as the `/dev/null` that `/dev/stdout` may resolve to -/// all reject `ftruncate` with `EINVAL`, so those take the standard copy -/// instead, which is what GNU does with them. -/// -/// This is discovered here rather than checked in advance, since checking -/// first would cost every ordinary destination -- overwhelmingly a regular -/// file, for which this never fires -- a `metadata` call for no benefit. -/// Nothing has been read from `src_file` or written to `dst_file` yet at the -/// point every caller reaches this, so falling back here cannot duplicate or -/// corrupt output. -fn fall_back_if_cannot_be_sparse( - error: rustix::io::Errno, - src_file: &File, - dst_file: &File, - context: &str, -) -> CopyResult<()> { - if error != rustix::io::Errno::INVAL { - return Err(CpError::IoErrContext(error.into(), context.to_owned())); - } - let mut src = src_file; - let mut dst = dst_file; - buf_copy::copy_fast(&mut src, &mut dst) - .map_err(|e| CpError::IoErrContext(e, context.to_owned())) -} - /// Copy the contents of a stream from `source` to `dest`. fn copy_stream

(source: P, dest: P, nofollow: bool, context: &str) -> CopyResult<()> where