Skip to content

uucore::pipes::pipe_with_size: return OwnedFd without conversion #12274

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.

Sign up for GitHub

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

Closed
oech3 wants to merge 1 commit into from
+22 −19
Closed

uucore::pipes::pipe_with_size: return OwnedFd without conversion #12274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 1 addition & 1 deletion src/uu/od/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ byteorder = { workspace = true }
clap = { workspace = true }
half = { workspace = true }
rustix = { workspace = true, features = ["stdio"] }
uucore = { workspace = true, features = ["parser-size"] }
uucore = { workspace = true, features = ["fs", "parser-size"] }
fluent = { workspace = true }
libc.workspace = true

Expand Down
10 changes: 1 addition & 9 deletions src/uu/od/src/multifile_reader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,7 @@ impl MultifileReader<'_> {
// limit.
#[cfg(any(unix, target_os = "wasi"))]
{
use std::os::fd::AsFd;
// todo: definition is generic enough to move to uucore::io::RawReader if useful
struct RawReader<T: AsFd>(pub T);
impl<T: AsFd> io::Read for RawReader<T> {
fn read(&mut self, b: &mut [u8]) -> io::Result<usize> {
rustix::io::read(&self.0, b).map_err(Into::into)
}
}
let stdin = RawReader(rustix::stdio::stdin());
let stdin = uucore::io::RawReader(rustix::stdio::stdin());
self.curr_file = Some(Box::new(stdin));
}

Expand Down
19 changes: 10 additions & 9 deletions src/uucore/src/lib/features/pipes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@
#[cfg(any(target_os = "linux", target_os = "android"))]
use rustix::pipe::{SpliceFlags, fcntl_setpipe_size};
#[cfg(any(target_os = "linux", target_os = "android"))]
use std::fs::File;
#[cfg(any(target_os = "linux", target_os = "android"))]
use std::{
fs::File,
io::{Read, Write},
os::fd::AsFd,
os::fd::{AsFd, OwnedFd},
sync::OnceLock,
};
#[cfg(any(target_os = "linux", target_os = "android"))]
Expand Down Expand Up @@ -40,13 +39,13 @@ pub fn pipe() -> std::io::Result<(File, File)> {
/// useful to save RAM usage
#[inline]
#[cfg(any(target_os = "linux", target_os = "android"))]
fn pipe_with_size(s: usize) -> std::io::Result<(File, File)> {
let (read, write) = rustix::pipe::pipe()?;
fn pipe_with_size(s: usize) -> std::io::Result<(OwnedFd, OwnedFd)> {
Copy link
Copy Markdown
Contributor

@xtqqczze xtqqczze May 13, 2026

Choose a reason for hiding this comment

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

Why do we need to change the types from File to OwnedFd?

Copy link
Copy Markdown
Contributor Author

@oech3 oech3 May 13, 2026

Choose a reason for hiding this comment

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

We can deffer conversion until when it is needed (which is cold code path at most cases). Also it is natural to just use rustix provided one.

Copy link
Copy Markdown
Contributor

@xtqqczze xtqqczze May 13, 2026

Choose a reason for hiding this comment

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

The conversion is zero-cost, see https://rust.godbolt.org/z/6sqafnM5Y

Copy link
Copy Markdown
Contributor Author

@oech3 oech3 May 13, 2026

Choose a reason for hiding this comment

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

But it is not clear from code and much simpler.

Copy link
Copy Markdown
Contributor Author

@oech3 oech3 May 13, 2026

Choose a reason for hiding this comment

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

I don't want to write many (*,*)s. But OK to close this.

Copy link
Copy Markdown
Contributor Author

@oech3 oech3 May 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This is low-priority, but it might useful if rustc reject trying to seeking (and other operations impossible) pipe as File at compile time.
OwnedFd satisfies it.

Copy link
Copy Markdown
Contributor

@xtqqczze xtqqczze May 13, 2026

Choose a reason for hiding this comment

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

The Seek trait does not promise that the object is actually seekable; std::io::Seek:seek returns a std::io::Result, see rust-lang/rust#72802.

let pair = rustix::pipe::pipe()?;
if s > KERNEL_DEFAULT_PIPE_SIZE {
let _ = fcntl_setpipe_size(&read, s);
let _ = fcntl_setpipe_size(&pair.0, s);
}

Ok((File::from(read), File::from(write)))
Ok(pair)
}

/// Less noisy wrapper around [`rustix::pipe::splice`].
Expand Down Expand Up @@ -166,7 +165,7 @@ pub fn send_n_bytes(
mut target: impl Write + AsFd,
n: u64,
) -> std::io::Result<u64> {
static PIPE_CACHE: OnceLock<Option<(File, File)>> = OnceLock::new();
static PIPE_CACHE: OnceLock<Option<(OwnedFd, OwnedFd)>> = OnceLock::new();
let pipe_size = MAX_ROOTLESS_PIPE_SIZE.min(n as usize);
let mut n = n;
let mut bytes_written: u64 = 0;
Expand Down Expand Up @@ -216,7 +215,9 @@ pub fn send_n_bytes(
debug_assert!(s <= MAX_ROOTLESS_PIPE_SIZE, "unexpected RAM usage");
// drain pipe before fallback to raw write
let mut drain = Vec::with_capacity(s);
broker_r.take(s as u64).read_to_end(&mut drain)?;
crate::io::RawReader(broker_r)
.take(s as u64)
.read_to_end(&mut drain)?;
crate::io::RawWriter(&target).write_all(&drain)?;
break true;
}
Expand Down
10 changes: 10 additions & 0 deletions src/uucore/src/lib/mods/io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,16 @@ type NativeType = OwnedHandle;
#[cfg(not(windows))]
type NativeType = OwnedFd;

// create reader without buffering
#[cfg(any(unix, target_os = "wasi"))]
pub struct RawReader<T: AsFd>(pub T);
#[cfg(any(unix, target_os = "wasi"))]
impl<T: AsFd> io::Read for RawReader<T> {
fn read(&mut self, b: &mut [u8]) -> io::Result<usize> {
rustix::io::read(&self.0, b).map_err(Into::into)
}
}

// create writer without buffering
#[cfg(any(unix, target_os = "wasi"))]
pub struct RawWriter<T: AsFd>(pub T);
Expand Down
Loading