Skip to content

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

Closed
oech3 wants to merge 1 commit into
uutils:mainfrom
oech3:pipe-dep
Closed

uucore::pipes::pipe_with_size: return OwnedFd without conversion#12274
oech3 wants to merge 1 commit into
uutils:mainfrom
oech3:pipe-dep

Conversation

@oech3
Copy link
Copy Markdown
Contributor

@oech3 oech3 commented May 13, 2026

  • make RawReader reusable
  • return OwnedFd itself at pipe_with_size

@github-actions
Copy link
Copy Markdown

GNU testsuite comparison:

Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/printf/printf-surprise is now passing!

@oech3 oech3 marked this pull request as ready for review May 13, 2026 08:18
#[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

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

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

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

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

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

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

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.

@oech3 oech3 closed this May 13, 2026
@oech3 oech3 deleted the pipe-dep branch May 13, 2026 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants