From 6f3ad0a40bf6bd80610008bfe48d3977e921d09d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 27 Jun 2024 15:27:52 -0400 Subject: [PATCH 01/21] Only require symbol name for @feature_gate --- compiler/rustc_lint/src/impl_trait_overcaptures.rs | 6 +++--- .../src/multiple_supertrait_upcastable.rs | 3 +-- compiler/rustc_lint_defs/src/builtin.rs | 13 ++++++------- compiler/rustc_lint_defs/src/lib.rs | 12 ++++++------ src/librustdoc/lint.rs | 4 ++-- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index d4f6d388d9fe3..0860413190c3d 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -11,7 +11,7 @@ use rustc_middle::ty::{ self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, }; use rustc_session::{declare_lint, declare_lint_pass}; -use rustc_span::{sym, Span}; +use rustc_span::Span; use crate::fluent_generated as fluent; use crate::{LateContext, LateLintPass}; @@ -57,7 +57,7 @@ declare_lint! { pub IMPL_TRAIT_OVERCAPTURES, Allow, "`impl Trait` will capture more lifetimes than possibly intended in edition 2024", - @feature_gate = sym::precise_capturing; + @feature_gate = precise_capturing; //@future_incompatible = FutureIncompatibleInfo { // reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024), // reference: "", @@ -91,7 +91,7 @@ declare_lint! { pub IMPL_TRAIT_REDUNDANT_CAPTURES, Warn, "redundant precise-capturing `use<...>` syntax on an `impl Trait`", - @feature_gate = sym::precise_capturing; + @feature_gate = precise_capturing; } declare_lint_pass!( diff --git a/compiler/rustc_lint/src/multiple_supertrait_upcastable.rs b/compiler/rustc_lint/src/multiple_supertrait_upcastable.rs index aa1d94228ea22..b9ed648b246fe 100644 --- a/compiler/rustc_lint/src/multiple_supertrait_upcastable.rs +++ b/compiler/rustc_lint/src/multiple_supertrait_upcastable.rs @@ -2,7 +2,6 @@ use crate::{LateContext, LateLintPass, LintContext}; use rustc_hir as hir; use rustc_session::{declare_lint, declare_lint_pass}; -use rustc_span::sym; declare_lint! { /// The `multiple_supertrait_upcastable` lint detects when an object-safe trait has multiple @@ -30,7 +29,7 @@ declare_lint! { pub MULTIPLE_SUPERTRAIT_UPCASTABLE, Allow, "detect when an object-safe trait has multiple supertraits", - @feature_gate = sym::multiple_supertrait_upcastable; + @feature_gate = multiple_supertrait_upcastable; } declare_lint_pass!(MultipleSupertraitUpcastable => [MULTIPLE_SUPERTRAIT_UPCASTABLE]); diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 472e93d202d76..186be98332b36 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -9,7 +9,6 @@ use crate::{declare_lint, declare_lint_pass, FutureIncompatibilityReason}; use rustc_span::edition::Edition; -use rustc_span::symbol::sym; declare_lint_pass! { /// Does nothing as a lint pass, but registers some `Lint`s @@ -461,7 +460,7 @@ declare_lint! { pub MUST_NOT_SUSPEND, Allow, "use of a `#[must_not_suspend]` value across a yield point", - @feature_gate = rustc_span::symbol::sym::must_not_suspend; + @feature_gate = must_not_suspend; } declare_lint! { @@ -1645,7 +1644,7 @@ declare_lint! { pub RUST_2024_INCOMPATIBLE_PAT, Allow, "detects patterns whose meaning will change in Rust 2024", - @feature_gate = sym::ref_pat_eat_one_layer_2024; + @feature_gate = ref_pat_eat_one_layer_2024; // FIXME uncomment below upon stabilization /*@future_incompatible = FutureIncompatibleInfo { reason: FutureIncompatibilityReason::EditionSemanticsChange(Edition::Edition2024), @@ -2693,7 +2692,7 @@ declare_lint! { pub FUZZY_PROVENANCE_CASTS, Allow, "a fuzzy integer to pointer cast is used", - @feature_gate = sym::strict_provenance; + @feature_gate = strict_provenance; } declare_lint! { @@ -2739,7 +2738,7 @@ declare_lint! { pub LOSSY_PROVENANCE_CASTS, Allow, "a lossy pointer to integer cast is used", - @feature_gate = sym::strict_provenance; + @feature_gate = strict_provenance; } declare_lint! { @@ -3923,7 +3922,7 @@ declare_lint! { pub NON_EXHAUSTIVE_OMITTED_PATTERNS, Allow, "detect when patterns of types marked `non_exhaustive` are missed", - @feature_gate = sym::non_exhaustive_omitted_patterns_lint; + @feature_gate = non_exhaustive_omitted_patterns_lint; } declare_lint! { @@ -4043,7 +4042,7 @@ declare_lint! { pub TEST_UNSTABLE_LINT, Deny, "this unstable lint is only for testing", - @feature_gate = sym::test_unstable_lint; + @feature_gate = test_unstable_lint; } declare_lint! { diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index b44eb25216770..f87f19e170005 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/compiler/rustc_lint_defs/src/lib.rs @@ -865,7 +865,7 @@ macro_rules! declare_lint { ); ); ($(#[$attr:meta])* $vis: vis $NAME: ident, $Level: ident, $desc: expr, - $(@feature_gate = $gate:expr;)? + $(@feature_gate = $gate:ident;)? $(@future_incompatible = FutureIncompatibleInfo { reason: $reason:expr, $($field:ident : $val:expr),* $(,)* @@ -879,7 +879,7 @@ macro_rules! declare_lint { desc: $desc, is_externally_loaded: false, $($v: true,)* - $(feature_gate: Some($gate),)? + $(feature_gate: Some(rustc_span::symbol::sym::$gate),)? $(future_incompatible: Some($crate::FutureIncompatibleInfo { reason: $reason, $($field: $val,)* @@ -895,21 +895,21 @@ macro_rules! declare_lint { macro_rules! declare_tool_lint { ( $(#[$attr:meta])* $vis:vis $tool:ident ::$NAME:ident, $Level: ident, $desc: expr - $(, @feature_gate = $gate:expr;)? + $(, @feature_gate = $gate:ident;)? ) => ( $crate::declare_tool_lint!{$(#[$attr])* $vis $tool::$NAME, $Level, $desc, false $(, @feature_gate = $gate;)?} ); ( $(#[$attr:meta])* $vis:vis $tool:ident ::$NAME:ident, $Level:ident, $desc:expr, report_in_external_macro: $rep:expr - $(, @feature_gate = $gate:expr;)? + $(, @feature_gate = $gate:ident;)? ) => ( $crate::declare_tool_lint!{$(#[$attr])* $vis $tool::$NAME, $Level, $desc, $rep $(, @feature_gate = $gate;)?} ); ( $(#[$attr:meta])* $vis:vis $tool:ident ::$NAME:ident, $Level:ident, $desc:expr, $external:expr - $(, @feature_gate = $gate:expr;)? + $(, @feature_gate = $gate:ident;)? ) => ( $(#[$attr])* $vis static $NAME: &$crate::Lint = &$crate::Lint { @@ -920,7 +920,7 @@ macro_rules! declare_tool_lint { report_in_external_macro: $external, future_incompatible: None, is_externally_loaded: true, - $(feature_gate: Some($gate),)? + $(feature_gate: Some(rustc_span::symbol::sym::$gate),)? crate_level_only: false, ..$crate::Lint::default_fields_for_macro() }; diff --git a/src/librustdoc/lint.rs b/src/librustdoc/lint.rs index dd2bb47e5926b..b570b73bf08df 100644 --- a/src/librustdoc/lint.rs +++ b/src/librustdoc/lint.rs @@ -66,7 +66,7 @@ where macro_rules! declare_rustdoc_lint { ( $(#[$attr:meta])* $name: ident, $level: ident, $descr: literal $(,)? - $(@feature_gate = $gate:expr;)? + $(@feature_gate = $gate:ident;)? ) => { declare_tool_lint! { $(#[$attr])* pub rustdoc::$name, $level, $descr @@ -128,7 +128,7 @@ declare_rustdoc_lint! { MISSING_DOC_CODE_EXAMPLES, Allow, "detects publicly-exported items without code samples in their documentation", - @feature_gate = rustc_span::symbol::sym::rustdoc_missing_doc_code_examples; + @feature_gate = rustdoc_missing_doc_code_examples; } declare_rustdoc_lint! { From d526adad2580d20c6f01c4c285e699492dbd45f7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 27 Jun 2024 15:32:29 -0400 Subject: [PATCH 02/21] Basic lint detecting closure-returning-async-block --- compiler/rustc_lint/messages.ftl | 3 + compiler/rustc_lint/src/async_closures.rs | 110 ++++++++++++++++++++++ compiler/rustc_lint/src/lib.rs | 3 + 3 files changed, 116 insertions(+) create mode 100644 compiler/rustc_lint/src/async_closures.rs diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 46cf87d1e3c17..8c4cfe9b87e60 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -187,6 +187,9 @@ lint_cfg_attr_no_attributes = lint_check_name_unknown_tool = unknown lint tool: `{$tool_name}` +lint_closure_returning_async_block = closure returning async block can be made into an async closure + .label = this async block can be removed, and the closure can be turned into an async closure + lint_command_line_source = `forbid` lint level was set on command line lint_confusable_identifier_pair = found both `{$existing_sym}` and `{$sym}` as identifiers, which look alike diff --git a/compiler/rustc_lint/src/async_closures.rs b/compiler/rustc_lint/src/async_closures.rs new file mode 100644 index 0000000000000..8a72b1d153b0b --- /dev/null +++ b/compiler/rustc_lint/src/async_closures.rs @@ -0,0 +1,110 @@ +use rustc_hir as hir; +use rustc_macros::LintDiagnostic; +use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_span::Span; + +use crate::{LateContext, LateLintPass}; + +declare_lint! { + /// The `closure_returning_async_block` lint detects cases where users + /// write a closure that returns an async block. + /// + /// ### Example + /// + /// ```rust + /// #![warn(closure_returning_async_block)] + /// let c = |x: &str| async {}; + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Using an async closure is preferable over a closure that returns an + /// async block, since async closures are less restrictive in how its + /// captures are allowed to be used. + /// + /// For example, this code does not work with a closure returning an async + /// block: + /// + /// ```rust,compile_fail + /// async fn callback(x: &str) {} + /// + /// let captured_str = String::new(); + /// let c = move || async { + /// callback(&captured_str).await; + /// }; + /// ``` + /// + /// But it does work with async closures: + /// + /// ```rust + /// #![feature(async_closure)] + /// + /// async fn callback(x: &str) {} + /// + /// let captured_str = String::new(); + /// let c = async move || { + /// callback(&captured_str).await; + /// }; + /// ``` + pub CLOSURE_RETURNING_ASYNC_BLOCK, + Allow, + "closure that returns `async {}` could be rewritten as an async closure", + @feature_gate = async_closure; +} + +declare_lint_pass!( + /// Lint for potential usages of async closures and async fn trait bounds. + AsyncClosureUsage => [CLOSURE_RETURNING_ASYNC_BLOCK] +); + +impl<'tcx> LateLintPass<'tcx> for AsyncClosureUsage { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { + let hir::ExprKind::Closure(&hir::Closure { + body, + kind: hir::ClosureKind::Closure, + fn_decl_span, + .. + }) = expr.kind + else { + return; + }; + + let mut body = cx.tcx.hir().body(body).value; + + // Only peel blocks that have no expressions. + while let hir::ExprKind::Block(&hir::Block { stmts: [], expr: Some(tail), .. }, None) = + body.kind + { + body = tail; + } + + let hir::ExprKind::Closure(&hir::Closure { + kind: + hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared( + hir::CoroutineDesugaring::Async, + hir::CoroutineSource::Block, + )), + fn_decl_span: async_decl_span, + .. + }) = body.kind + else { + return; + }; + + cx.tcx.emit_node_span_lint( + CLOSURE_RETURNING_ASYNC_BLOCK, + expr.hir_id, + fn_decl_span, + ClosureReturningAsyncBlock { async_decl_span }, + ); + } +} + +#[derive(LintDiagnostic)] +#[diag(lint_closure_returning_async_block)] +struct ClosureReturningAsyncBlock { + #[label] + async_decl_span: Span, +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 7dae2de7bfb58..43e6e6498b3a1 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -41,6 +41,7 @@ #![feature(trait_upcasting)] // tidy-alphabetical-end +mod async_closures; mod async_fn_in_trait; pub mod builtin; mod context; @@ -86,6 +87,7 @@ use rustc_hir::def_id::LocalModDefId; use rustc_middle::query::Providers; use rustc_middle::ty::TyCtxt; +use async_closures::AsyncClosureUsage; use async_fn_in_trait::AsyncFnInTrait; use builtin::*; use deref_into_dyn_supertrait::*; @@ -227,6 +229,7 @@ late_lint_methods!( MapUnitFn: MapUnitFn, MissingDebugImplementations: MissingDebugImplementations, MissingDoc: MissingDoc, + AsyncClosureUsage: AsyncClosureUsage, AsyncFnInTrait: AsyncFnInTrait, NonLocalDefinitions: NonLocalDefinitions::default(), ImplTraitOvercaptures: ImplTraitOvercaptures, From acc13e29d1f42805ea0701c05f59beb646978c1d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Fri, 28 Jun 2024 15:56:06 -0400 Subject: [PATCH 03/21] Make it into a structured suggestion, maybe-incorrect --- compiler/rustc_lint/messages.ftl | 1 + compiler/rustc_lint/src/async_closures.rs | 23 ++++++- .../lint-closure-returning-async-block.rs | 21 ++++++ .../lint-closure-returning-async-block.stderr | 67 +++++++++++++++++++ 4 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 tests/ui/async-await/async-closures/lint-closure-returning-async-block.rs create mode 100644 tests/ui/async-await/async-closures/lint-closure-returning-async-block.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 8c4cfe9b87e60..88f01c158b560 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -189,6 +189,7 @@ lint_check_name_unknown_tool = unknown lint tool: `{$tool_name}` lint_closure_returning_async_block = closure returning async block can be made into an async closure .label = this async block can be removed, and the closure can be turned into an async closure + .suggestion = turn this into an async closure lint_command_line_source = `forbid` lint level was set on command line diff --git a/compiler/rustc_lint/src/async_closures.rs b/compiler/rustc_lint/src/async_closures.rs index 8a72b1d153b0b..33cc5738262f6 100644 --- a/compiler/rustc_lint/src/async_closures.rs +++ b/compiler/rustc_lint/src/async_closures.rs @@ -1,5 +1,5 @@ use rustc_hir as hir; -use rustc_macros::LintDiagnostic; +use rustc_macros::{LintDiagnostic, Subdiagnostic}; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::Span; @@ -93,11 +93,19 @@ impl<'tcx> LateLintPass<'tcx> for AsyncClosureUsage { return; }; + let deletion_span = cx.tcx.sess.source_map().span_extend_while_whitespace(async_decl_span); + cx.tcx.emit_node_span_lint( CLOSURE_RETURNING_ASYNC_BLOCK, expr.hir_id, fn_decl_span, - ClosureReturningAsyncBlock { async_decl_span }, + ClosureReturningAsyncBlock { + async_decl_span, + sugg: AsyncClosureSugg { + deletion_span, + insertion_span: fn_decl_span.shrink_to_lo(), + }, + }, ); } } @@ -107,4 +115,15 @@ impl<'tcx> LateLintPass<'tcx> for AsyncClosureUsage { struct ClosureReturningAsyncBlock { #[label] async_decl_span: Span, + #[subdiagnostic] + sugg: AsyncClosureSugg, +} + +#[derive(Subdiagnostic)] +#[multipart_suggestion(lint_suggestion, applicability = "maybe-incorrect")] +struct AsyncClosureSugg { + #[suggestion_part(code = "")] + deletion_span: Span, + #[suggestion_part(code = "async ")] + insertion_span: Span, } diff --git a/tests/ui/async-await/async-closures/lint-closure-returning-async-block.rs b/tests/ui/async-await/async-closures/lint-closure-returning-async-block.rs new file mode 100644 index 0000000000000..3e2ab8321a890 --- /dev/null +++ b/tests/ui/async-await/async-closures/lint-closure-returning-async-block.rs @@ -0,0 +1,21 @@ +//@ edition: 2021 + +#![feature(async_closure)] +#![deny(closure_returning_async_block)] + +fn main() { + let x = || async {}; + //~^ ERROR closure returning async block can be made into an async closure + + let x = || async move {}; + //~^ ERROR closure returning async block can be made into an async closure + + let x = move || async move {}; + //~^ ERROR closure returning async block can be made into an async closure + + let x = move || async {}; + //~^ ERROR closure returning async block can be made into an async closure + + let x = || {{ async {} }}; + //~^ ERROR closure returning async block can be made into an async closure +} diff --git a/tests/ui/async-await/async-closures/lint-closure-returning-async-block.stderr b/tests/ui/async-await/async-closures/lint-closure-returning-async-block.stderr new file mode 100644 index 0000000000000..4c0c4d797d8ef --- /dev/null +++ b/tests/ui/async-await/async-closures/lint-closure-returning-async-block.stderr @@ -0,0 +1,67 @@ +error: closure returning async block can be made into an async closure + --> $DIR/lint-closure-returning-async-block.rs:7:13 + | +LL | let x = || async {}; + | ^^ ----- this async block can be removed, and the closure can be turned into an async closure + | +note: the lint level is defined here + --> $DIR/lint-closure-returning-async-block.rs:4:9 + | +LL | #![deny(closure_returning_async_block)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: turn this into an async closure + | +LL - let x = || async {}; +LL + let x = async || {}; + | + +error: closure returning async block can be made into an async closure + --> $DIR/lint-closure-returning-async-block.rs:10:13 + | +LL | let x = || async move {}; + | ^^ ---------- this async block can be removed, and the closure can be turned into an async closure + | +help: turn this into an async closure + | +LL - let x = || async move {}; +LL + let x = async || {}; + | + +error: closure returning async block can be made into an async closure + --> $DIR/lint-closure-returning-async-block.rs:13:13 + | +LL | let x = move || async move {}; + | ^^^^^^^ ---------- this async block can be removed, and the closure can be turned into an async closure + | +help: turn this into an async closure + | +LL - let x = move || async move {}; +LL + let x = async move || {}; + | + +error: closure returning async block can be made into an async closure + --> $DIR/lint-closure-returning-async-block.rs:16:13 + | +LL | let x = move || async {}; + | ^^^^^^^ ----- this async block can be removed, and the closure can be turned into an async closure + | +help: turn this into an async closure + | +LL - let x = move || async {}; +LL + let x = async move || {}; + | + +error: closure returning async block can be made into an async closure + --> $DIR/lint-closure-returning-async-block.rs:19:13 + | +LL | let x = || {{ async {} }}; + | ^^ ----- this async block can be removed, and the closure can be turned into an async closure + | +help: turn this into an async closure + | +LL - let x = || {{ async {} }}; +LL + let x = async || {{ {} }}; + | + +error: aborting due to 5 previous errors + From 04eed9bc2412b2a5e90bf6924e71c5113f58901e Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 30 Jun 2024 18:23:07 +1000 Subject: [PATCH 04/21] Initial implementation of annoymous_pipe Signed-off-by: Jiahao XU --- library/std/src/io/mod.rs | 2 + library/std/src/io/pipe/mod.rs | 238 ++++++++++++++++++++++ library/std/src/sys/pal/unix/pipe.rs | 5 + library/std/src/sys/pal/windows/handle.rs | 1 + library/std/src/sys/pal/windows/pipe.rs | 6 +- 5 files changed, 250 insertions(+), 2 deletions(-) create mode 100644 library/std/src/io/pipe/mod.rs diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 97b72f9664bb9..1e8ed2bb5bf13 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -338,6 +338,8 @@ pub(crate) mod copy; mod cursor; mod error; mod impls; +#[unstable(feature = "annoymous_pipe", issue = "")] +pub mod pipe; pub mod prelude; mod stdio; mod util; diff --git a/library/std/src/io/pipe/mod.rs b/library/std/src/io/pipe/mod.rs new file mode 100644 index 0000000000000..1919707a31fda --- /dev/null +++ b/library/std/src/io/pipe/mod.rs @@ -0,0 +1,238 @@ +use crate::{io, process::Stdio, sys::pipe::AnonPipe}; + +/// Create annoymous pipe that is close-on-exec and blocking. +#[inline] +pub fn pipe() -> io::Result<(PipeReader, PipeWriter)> { + cfg_if::cfg_if! { + if #[cfg(unix)] { + unix::pipe() + } else { + windows::pipe() + } + } +} + +#[derive(Debug)] +pub struct PipeReader(AnonPipe); + +#[derive(Debug)] +pub struct PipeWriter(AnonPipe); + +pub struct NotAPipeError; + +impl PipeReader { + /// Create a new [`PipeReader`] instance that shares the same underlying file description. + pub fn try_clone(&self) -> io::Result { + self.0.try_clone().map(Self) + } +} + +impl PipeWriter { + /// Create a new [`PipeWriter`] instance that shares the same underlying file description. + pub fn try_clone(&self) -> io::Result { + self.0.try_clone().map(Self) + } +} + +macro_rules! forward_io_read_traits { + ($name:ty) => { + impl io::Read for $name { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + self.0.read(buf) + } + fn read_vectored(&mut self, bufs: &mut [io::IoSliceMut<'_>]) -> io::Result { + self.0.read_vectored(bufs) + } + fn is_read_vectored(&self) -> bool { + self.0.is_read_vectored() + } + fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { + self.0.read_to_end(buf) + } + fn read_buf(&mut self, buf: io::BorrowedCursor<'_>) -> io::Result<()> { + self.0.read_buf(buf) + } + } + }; +} +forward_io_read_traits!(PipeReader); +forward_io_read_traits!(&PipeReader); + +macro_rules! forward_io_write_traits { + ($name:ty) => { + impl io::Write for $name { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.write(buf) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + + fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result { + self.0.write_vectored(bufs) + } + fn is_write_vectored(&self) -> bool { + self.0.is_write_vectored() + } + } + }; +} +forward_io_write_traits!(PipeWriter); +forward_io_write_traits!(&PipeWriter); + +#[cfg(unix)] +mod unix { + use super::*; + + use crate::{ + os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}, + sys::{ + fd::FileDesc, + pipe::{anon_pipe, AnonPipe}, + }, + sys_common::{FromInner, IntoInner}, + }; + + #[inline] + pub(super) fn pipe() -> io::Result<(PipeReader, PipeWriter)> { + anon_pipe().map(|(rx, tx)| (PipeReader(rx), PipeWriter(tx))) + } + + macro_rules! impl_traits { + ($name:ty) => { + impl AsFd for $name { + fn as_fd(&self) -> BorrowedFd<'_> { + self.0.as_fd() + } + } + impl AsRawFd for $name { + fn as_raw_fd(&self) -> RawFd { + self.0.as_raw_fd() + } + } + impl From<$name> for OwnedFd { + fn from(pipe: $name) -> Self { + FileDesc::into_inner(AnonPipe::into_inner(pipe.0)) + } + } + impl FromRawFd for $name { + unsafe fn from_raw_fd(raw_fd: RawFd) -> Self { + Self(AnonPipe::from_raw_fd(raw_fd)) + } + } + impl IntoRawFd for $name { + fn into_raw_fd(self) -> RawFd { + self.0.into_raw_fd() + } + } + impl From<$name> for Stdio { + fn from(pipe: $name) -> Self { + Self::from(OwnedFd::from(pipe)) + } + } + }; + } + impl_traits!(PipeReader); + impl_traits!(PipeWriter); + + fn owned_fd_to_anon_pipe(owned_fd: OwnedFd) -> AnonPipe { + AnonPipe::from_inner(FileDesc::from_inner(owned_fd)) + } + + impl TryFrom for PipeReader { + type Error = NotAPipeError; + + fn try_from(owned_fd: OwnedFd) -> Result { + Ok(Self(owned_fd_to_anon_pipe(owned_fd))) + } + } + + impl TryFrom for PipeWriter { + type Error = NotAPipeError; + + fn try_from(owned_fd: OwnedFd) -> Result { + Ok(Self(owned_fd_to_anon_pipe(owned_fd))) + } + } +} + +#[cfg(windows)] +mod windows { + use super::*; + + use crate::{ + os::windows::io::{ + AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, + RawHandle, + }, + sys::{ + handle::Handle, + pipe::{anon_pipe, AnonPipe, Pipes}, + }, + sys_common::{FromInner, IntoInner}, + }; + + #[inline] + pub(super) fn pipe() -> io::Result<(PipeReader, PipeWriter)> { + anon_pipe(true, false).map(|Pipes { ours, their }| (PipeReader(ours), PipeWrite(theirs))) + } + + macro_rules! impl_traits { + ($name:ty) => { + impl AsHandle for $name { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.0.handle().as_handle() + } + } + impl AsRawHandle for $name { + fn as_raw_handle(&self) -> RawHandle { + self.0.handle().as_raw_handle() + } + } + + impl FromRawHandle for $name { + unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { + Self(AnonPipe::from_inner(Handle::from_raw_handle(raw_handle))) + } + } + impl IntoRawHandle for $name { + fn into_raw_handle(self) -> RawHandle { + self.0.into_handle().into_raw_handle() + } + } + + impl From<$name> for OwnedHandle { + fn from(pipe: $name) -> Self { + Handle::into_inner(AnonPipe::into_inner(pipe.0)) + } + } + impl From<$name> for Stdio { + fn from(pipe: $name) -> Self { + Self::from(OwnedHandle::from(pipe)) + } + } + }; + } + impl_traits!(PipeReader); + impl_traits!(PipeWriter); + + fn owned_handle_to_anon_pipe(owned_handle: OwnedHandle) -> AnonPipe { + AnonPipe::from_inner(Handle::from_inner(owned_handle)) + } + + impl TryFrom for PipeReader { + type Error = NotAPipeError; + + fn try_from(owned_handle: OwnedHandle) -> Result { + Ok(Self(owned_handle_to_anon_pipe(owned_handle))) + } + } + + impl TryFrom for PipeWriter { + type Error = NotAPipeError; + + fn try_from(owned_handle: OwnedHandle) -> Result { + Ok(Self(owned_handle_to_anon_pipe(owned_handle))) + } + } +} diff --git a/library/std/src/sys/pal/unix/pipe.rs b/library/std/src/sys/pal/unix/pipe.rs index 33db24e77e4da..c2fb9c449cf08 100644 --- a/library/std/src/sys/pal/unix/pipe.rs +++ b/library/std/src/sys/pal/unix/pipe.rs @@ -9,6 +9,7 @@ use crate::sys_common::{FromInner, IntoInner}; // Anonymous pipes //////////////////////////////////////////////////////////////////////////////// +#[derive(Debug)] pub struct AnonPipe(FileDesc); pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> { @@ -46,6 +47,10 @@ pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> { } impl AnonPipe { + pub fn try_clone(&self) -> io::Result { + self.0.duplicate().map(Self) + } + pub fn read(&self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } diff --git a/library/std/src/sys/pal/windows/handle.rs b/library/std/src/sys/pal/windows/handle.rs index 3f85bb0a099a9..5aa2141571953 100644 --- a/library/std/src/sys/pal/windows/handle.rs +++ b/library/std/src/sys/pal/windows/handle.rs @@ -17,6 +17,7 @@ use crate::sys_common::{AsInner, FromInner, IntoInner}; /// An owned container for `HANDLE` object, closing them on Drop. /// /// All methods are inherited through a `Deref` impl to `RawHandle` +#[derive(Debug)] pub struct Handle(OwnedHandle); impl Handle { diff --git a/library/std/src/sys/pal/windows/pipe.rs b/library/std/src/sys/pal/windows/pipe.rs index 67ef3ca82da02..6e6ba721d00fa 100644 --- a/library/std/src/sys/pal/windows/pipe.rs +++ b/library/std/src/sys/pal/windows/pipe.rs @@ -19,6 +19,7 @@ use crate::sys_common::{FromInner, IntoInner}; // Anonymous pipes //////////////////////////////////////////////////////////////////////////////// +#[derive(Debug)] pub struct AnonPipe { inner: Handle, } @@ -182,7 +183,7 @@ pub fn spawn_pipe_relay( their_handle_inheritable: bool, ) -> io::Result { // We need this handle to live for the lifetime of the thread spawned below. - let source = source.duplicate()?; + let source = source.try_clone()?; // create a new pair of anon pipes. let Pipes { theirs, ours } = anon_pipe(ours_readable, their_handle_inheritable)?; @@ -238,7 +239,8 @@ impl AnonPipe { pub fn into_handle(self) -> Handle { self.inner } - fn duplicate(&self) -> io::Result { + + pub fn try_clone(&self) -> io::Result { self.inner.duplicate(0, false, c::DUPLICATE_SAME_ACCESS).map(|inner| AnonPipe { inner }) } From 72bda33a5b94a57b6fc262657899fd90a61931a7 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 30 Jun 2024 18:35:38 +1000 Subject: [PATCH 05/21] Fix compilation errors Signed-off-by: Jiahao XU --- library/std/src/io/mod.rs | 3 ++- library/std/src/io/pipe/mod.rs | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 1e8ed2bb5bf13..0e95ae8151c90 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -338,7 +338,8 @@ pub(crate) mod copy; mod cursor; mod error; mod impls; -#[unstable(feature = "annoymous_pipe", issue = "")] +/// Annoymous pipe implementation +#[unstable(feature = "annoymous_pipe", issue = "127154")] pub mod pipe; pub mod prelude; mod stdio; diff --git a/library/std/src/io/pipe/mod.rs b/library/std/src/io/pipe/mod.rs index 1919707a31fda..b5fa4b0bfc90d 100644 --- a/library/std/src/io/pipe/mod.rs +++ b/library/std/src/io/pipe/mod.rs @@ -12,12 +12,16 @@ pub fn pipe() -> io::Result<(PipeReader, PipeWriter)> { } } +/// Read end of the annoymous pipe. #[derive(Debug)] pub struct PipeReader(AnonPipe); +/// Write end of the annoymous pipe. #[derive(Debug)] pub struct PipeWriter(AnonPipe); +/// The owned fd provided is not a pipe. +#[derive(Debug)] pub struct NotAPipeError; impl PipeReader { @@ -174,7 +178,7 @@ mod windows { #[inline] pub(super) fn pipe() -> io::Result<(PipeReader, PipeWriter)> { - anon_pipe(true, false).map(|Pipes { ours, their }| (PipeReader(ours), PipeWrite(theirs))) + anon_pipe(true, false).map(|Pipes { ours, theirs }| (PipeReader(ours), PipeWriter(theirs))) } macro_rules! impl_traits { From 42e8beb64cac0124491fb86e2c42ee32d0f6c0f5 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 30 Jun 2024 19:19:11 +1000 Subject: [PATCH 06/21] Implement validation in `TryFrom for PIpe*` on unix Signed-off-by: Jiahao XU --- library/std/src/io/pipe/mod.rs | 64 +++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 13 deletions(-) diff --git a/library/std/src/io/pipe/mod.rs b/library/std/src/io/pipe/mod.rs index b5fa4b0bfc90d..7f07126e412f4 100644 --- a/library/std/src/io/pipe/mod.rs +++ b/library/std/src/io/pipe/mod.rs @@ -20,10 +20,6 @@ pub struct PipeReader(AnonPipe); #[derive(Debug)] pub struct PipeWriter(AnonPipe); -/// The owned fd provided is not a pipe. -#[derive(Debug)] -pub struct NotAPipeError; - impl PipeReader { /// Create a new [`PipeReader`] instance that shares the same underlying file description. pub fn try_clone(&self) -> io::Result { @@ -89,7 +85,11 @@ mod unix { use super::*; use crate::{ - os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}, + fs::File, + os::{ + fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}, + unix::fs::FileTypeExt, + }, sys::{ fd::FileDesc, pipe::{anon_pipe, AnonPipe}, @@ -139,23 +139,61 @@ mod unix { impl_traits!(PipeReader); impl_traits!(PipeWriter); - fn owned_fd_to_anon_pipe(owned_fd: OwnedFd) -> AnonPipe { - AnonPipe::from_inner(FileDesc::from_inner(owned_fd)) + fn convert_to_pipe(owned_fd: OwnedFd) -> io::Result { + let file = File::from(owned_fd); + if file.metadata()?.file_type().is_fifo() { + Ok(AnonPipe::from_inner(FileDesc::from_inner(OwnedFd::from(file)))) + } else { + Err(io::Error::new(io::ErrorKind::InvalidInput, "Not a pipe")) + } + } + + enum AccessMode { + Readable, + Writable, + } + + fn check_access_mode(pipe: AnonPipe, expected_access_mode: AccessMode) -> io::Result { + let ret = unsafe { libc::fcntl(pipe.as_raw_fd(), libc::F_GETFL) }; + let access_mode = ret & libc::O_ACCMODE; + let expected_access_mode_str = match expected_access_mode { + AccessMode::Readable => "readable", + AccessMode::Writable => "writable", + }; + let expected_access_mode = match expected_access_mode { + AccessMode::Readable => libc::O_RDONLY, + AccessMode::Writable => libc::O_WRONLY, + }; + + if ret == -1 { + Err(io::Error::last_os_error()) + } else if access_mode == libc::O_RDWR && access_mode == expected_access_mode { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Pipe {} is not {}", pipe.as_raw_fd(), expected_access_mode_str), + )) + } else { + Ok(pipe) + } } impl TryFrom for PipeReader { - type Error = NotAPipeError; + type Error = io::Error; fn try_from(owned_fd: OwnedFd) -> Result { - Ok(Self(owned_fd_to_anon_pipe(owned_fd))) + convert_to_pipe(owned_fd) + .and_then(|pipe| check_access_mode(pipe, AccessMode::Readable)) + .map(Self) } } impl TryFrom for PipeWriter { - type Error = NotAPipeError; + type Error = io::Error; fn try_from(owned_fd: OwnedFd) -> Result { - Ok(Self(owned_fd_to_anon_pipe(owned_fd))) + convert_to_pipe(owned_fd) + .and_then(|pipe| check_access_mode(pipe, AccessMode::Writable)) + .map(Self) } } } @@ -225,7 +263,7 @@ mod windows { } impl TryFrom for PipeReader { - type Error = NotAPipeError; + type Error = io::Error; fn try_from(owned_handle: OwnedHandle) -> Result { Ok(Self(owned_handle_to_anon_pipe(owned_handle))) @@ -233,7 +271,7 @@ mod windows { } impl TryFrom for PipeWriter { - type Error = NotAPipeError; + type Error = io::Error; fn try_from(owned_handle: OwnedHandle) -> Result { Ok(Self(owned_handle_to_anon_pipe(owned_handle))) From e170c7841613163383b323e88e282a3aeccaebb0 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 30 Jun 2024 19:21:48 +1000 Subject: [PATCH 07/21] Move the mod `pipe` to `std::net` Signed-off-by: Jiahao XU --- library/std/src/io/mod.rs | 3 --- library/std/src/net/mod.rs | 4 ++++ library/std/src/{io => net}/pipe/mod.rs | 0 3 files changed, 4 insertions(+), 3 deletions(-) rename library/std/src/{io => net}/pipe/mod.rs (100%) diff --git a/library/std/src/io/mod.rs b/library/std/src/io/mod.rs index 0e95ae8151c90..97b72f9664bb9 100644 --- a/library/std/src/io/mod.rs +++ b/library/std/src/io/mod.rs @@ -338,9 +338,6 @@ pub(crate) mod copy; mod cursor; mod error; mod impls; -/// Annoymous pipe implementation -#[unstable(feature = "annoymous_pipe", issue = "127154")] -pub mod pipe; pub mod prelude; mod stdio; mod util; diff --git a/library/std/src/net/mod.rs b/library/std/src/net/mod.rs index 858776f14466a..7e9a89916cf09 100644 --- a/library/std/src/net/mod.rs +++ b/library/std/src/net/mod.rs @@ -43,6 +43,10 @@ mod tcp; pub(crate) mod test; mod udp; +/// Annoymous pipe implementation +#[unstable(feature = "annoymous_pipe", issue = "127154")] +pub mod pipe; + /// Possible values which can be passed to the [`TcpStream::shutdown`] method. #[derive(Copy, Clone, PartialEq, Eq, Debug)] #[stable(feature = "rust1", since = "1.0.0")] diff --git a/library/std/src/io/pipe/mod.rs b/library/std/src/net/pipe/mod.rs similarity index 100% rename from library/std/src/io/pipe/mod.rs rename to library/std/src/net/pipe/mod.rs From 473fbce50c970175e14c37b72e9b925c440c65a8 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 30 Jun 2024 23:05:33 +1000 Subject: [PATCH 08/21] Fix typo Signed-off-by: Jiahao XU --- library/std/src/net/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/net/mod.rs b/library/std/src/net/mod.rs index 7e9a89916cf09..1fef8fc604cd0 100644 --- a/library/std/src/net/mod.rs +++ b/library/std/src/net/mod.rs @@ -43,8 +43,8 @@ mod tcp; pub(crate) mod test; mod udp; -/// Annoymous pipe implementation -#[unstable(feature = "annoymous_pipe", issue = "127154")] +/// Anonymous pipe implementation +#[unstable(feature = "anonymous_pipe", issue = "127154")] pub mod pipe; /// Possible values which can be passed to the [`TcpStream::shutdown`] method. From 97626b6ee3eb53b206d36900dc1d017744951963 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Sun, 30 Jun 2024 23:47:57 +1000 Subject: [PATCH 09/21] Fix tidy errors Signed-off-by: Jiahao XU --- library/std/src/net/mod.rs | 6 ++--- .../pipe/mod.rs => sys/anonymous_pipe.rs} | 23 +++++++++++++++++++ library/std/src/sys/mod.rs | 1 + 3 files changed, 26 insertions(+), 4 deletions(-) rename library/std/src/{net/pipe/mod.rs => sys/anonymous_pipe.rs} (84%) diff --git a/library/std/src/net/mod.rs b/library/std/src/net/mod.rs index 1fef8fc604cd0..cacf8c01f672d 100644 --- a/library/std/src/net/mod.rs +++ b/library/std/src/net/mod.rs @@ -33,6 +33,8 @@ pub use self::tcp::IntoIncoming; pub use self::tcp::{Incoming, TcpListener, TcpStream}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::udp::UdpSocket; +#[unstable(feature = "anonymous_pipe", issue = "127154")] +pub use crate::sys::anonymous_pipe::{pipe, PipeReader, PipeWriter}; #[stable(feature = "rust1", since = "1.0.0")] pub use core::net::AddrParseError; @@ -43,10 +45,6 @@ mod tcp; pub(crate) mod test; mod udp; -/// Anonymous pipe implementation -#[unstable(feature = "anonymous_pipe", issue = "127154")] -pub mod pipe; - /// Possible values which can be passed to the [`TcpStream::shutdown`] method. #[derive(Copy, Clone, PartialEq, Eq, Debug)] #[stable(feature = "rust1", since = "1.0.0")] diff --git a/library/std/src/net/pipe/mod.rs b/library/std/src/sys/anonymous_pipe.rs similarity index 84% rename from library/std/src/net/pipe/mod.rs rename to library/std/src/sys/anonymous_pipe.rs index 7f07126e412f4..aa7cad2a7de19 100644 --- a/library/std/src/net/pipe/mod.rs +++ b/library/std/src/sys/anonymous_pipe.rs @@ -1,6 +1,7 @@ use crate::{io, process::Stdio, sys::pipe::AnonPipe}; /// Create annoymous pipe that is close-on-exec and blocking. +#[unstable(feature = "anonymous_pipe", issue = "127154")] #[inline] pub fn pipe() -> io::Result<(PipeReader, PipeWriter)> { cfg_if::cfg_if! { @@ -13,15 +14,18 @@ pub fn pipe() -> io::Result<(PipeReader, PipeWriter)> { } /// Read end of the annoymous pipe. +#[unstable(feature = "anonymous_pipe", issue = "127154")] #[derive(Debug)] pub struct PipeReader(AnonPipe); /// Write end of the annoymous pipe. +#[unstable(feature = "anonymous_pipe", issue = "127154")] #[derive(Debug)] pub struct PipeWriter(AnonPipe); impl PipeReader { /// Create a new [`PipeReader`] instance that shares the same underlying file description. + #[unstable(feature = "anonymous_pipe", issue = "127154")] pub fn try_clone(&self) -> io::Result { self.0.try_clone().map(Self) } @@ -29,6 +33,7 @@ impl PipeReader { impl PipeWriter { /// Create a new [`PipeWriter`] instance that shares the same underlying file description. + #[unstable(feature = "anonymous_pipe", issue = "127154")] pub fn try_clone(&self) -> io::Result { self.0.try_clone().map(Self) } @@ -36,6 +41,7 @@ impl PipeWriter { macro_rules! forward_io_read_traits { ($name:ty) => { + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl io::Read for $name { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) @@ -60,6 +66,7 @@ forward_io_read_traits!(&PipeReader); macro_rules! forward_io_write_traits { ($name:ty) => { + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl io::Write for $name { fn write(&mut self, buf: &[u8]) -> io::Result { self.0.write(buf) @@ -104,31 +111,37 @@ mod unix { macro_rules! impl_traits { ($name:ty) => { + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl AsFd for $name { fn as_fd(&self) -> BorrowedFd<'_> { self.0.as_fd() } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl AsRawFd for $name { fn as_raw_fd(&self) -> RawFd { self.0.as_raw_fd() } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl From<$name> for OwnedFd { fn from(pipe: $name) -> Self { FileDesc::into_inner(AnonPipe::into_inner(pipe.0)) } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl FromRawFd for $name { unsafe fn from_raw_fd(raw_fd: RawFd) -> Self { Self(AnonPipe::from_raw_fd(raw_fd)) } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl IntoRawFd for $name { fn into_raw_fd(self) -> RawFd { self.0.into_raw_fd() } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl From<$name> for Stdio { fn from(pipe: $name) -> Self { Self::from(OwnedFd::from(pipe)) @@ -177,6 +190,7 @@ mod unix { } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl TryFrom for PipeReader { type Error = io::Error; @@ -187,6 +201,7 @@ mod unix { } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl TryFrom for PipeWriter { type Error = io::Error; @@ -221,33 +236,39 @@ mod windows { macro_rules! impl_traits { ($name:ty) => { + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl AsHandle for $name { fn as_handle(&self) -> BorrowedHandle<'_> { self.0.handle().as_handle() } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl AsRawHandle for $name { fn as_raw_handle(&self) -> RawHandle { self.0.handle().as_raw_handle() } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl FromRawHandle for $name { unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { Self(AnonPipe::from_inner(Handle::from_raw_handle(raw_handle))) } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl IntoRawHandle for $name { fn into_raw_handle(self) -> RawHandle { self.0.into_handle().into_raw_handle() } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl From<$name> for OwnedHandle { fn from(pipe: $name) -> Self { Handle::into_inner(AnonPipe::into_inner(pipe.0)) } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl From<$name> for Stdio { fn from(pipe: $name) -> Self { Self::from(OwnedHandle::from(pipe)) @@ -262,6 +283,7 @@ mod windows { AnonPipe::from_inner(Handle::from_inner(owned_handle)) } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl TryFrom for PipeReader { type Error = io::Error; @@ -270,6 +292,7 @@ mod windows { } } + #[unstable(feature = "anonymous_pipe", issue = "127154")] impl TryFrom for PipeWriter { type Error = io::Error; diff --git a/library/std/src/sys/mod.rs b/library/std/src/sys/mod.rs index 8aa35c40fe052..fad01f581577d 100644 --- a/library/std/src/sys/mod.rs +++ b/library/std/src/sys/mod.rs @@ -5,6 +5,7 @@ mod pal; mod personality; +pub mod anonymous_pipe; pub mod backtrace; pub mod cmath; pub mod os_str; From d60438fa2207490fdbe50d13b61db3b9af9befa4 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 1 Jul 2024 00:37:26 +1000 Subject: [PATCH 10/21] Validate pipe in `TryFrom for Pipe*` Signed-off-by: Jiahao XU --- library/std/src/sys/anonymous_pipe.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/library/std/src/sys/anonymous_pipe.rs b/library/std/src/sys/anonymous_pipe.rs index aa7cad2a7de19..230f56c2001e8 100644 --- a/library/std/src/sys/anonymous_pipe.rs +++ b/library/std/src/sys/anonymous_pipe.rs @@ -223,6 +223,7 @@ mod windows { RawHandle, }, sys::{ + c::{GetFileType, FILE_TYPE_PIPE}, handle::Handle, pipe::{anon_pipe, AnonPipe, Pipes}, }, @@ -279,8 +280,12 @@ mod windows { impl_traits!(PipeReader); impl_traits!(PipeWriter); - fn owned_handle_to_anon_pipe(owned_handle: OwnedHandle) -> AnonPipe { - AnonPipe::from_inner(Handle::from_inner(owned_handle)) + fn convert_to_pipe(owned_handle: OwnedHandle) -> io::Result { + if unsafe { GetFileType(owned_handle.as_raw_handle()) } == FILE_TYPE_PIPE { + Ok(AnonPipe::from_inner(Handle::from_inner(owned_handle))) + } else { + Err(io::Error::new(io::ErrorKind::InvalidInput, "Not a pipe")) + } } #[unstable(feature = "anonymous_pipe", issue = "127154")] @@ -288,7 +293,7 @@ mod windows { type Error = io::Error; fn try_from(owned_handle: OwnedHandle) -> Result { - Ok(Self(owned_handle_to_anon_pipe(owned_handle))) + convert_to_pipe(owned_handle).map(Self) } } @@ -297,7 +302,7 @@ mod windows { type Error = io::Error; fn try_from(owned_handle: OwnedHandle) -> Result { - Ok(Self(owned_handle_to_anon_pipe(owned_handle))) + convert_to_pipe(owned_handle).map(Self) } } } From b7af685cc2c50371cc687704a566a665f09ea215 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 1 Jul 2024 00:51:00 +1000 Subject: [PATCH 11/21] Refactor: Extract new method `FileDesc::get_access_mode` ALso fixed a bug in the original implementation Signed-off-by: Jiahao XU --- library/std/src/sys/anonymous_pipe.rs | 51 +++++++++++---------------- library/std/src/sys/pal/unix/fd.rs | 34 ++++++++++++------ library/std/src/sys/pal/unix/pipe.rs | 4 +++ 3 files changed, 47 insertions(+), 42 deletions(-) diff --git a/library/std/src/sys/anonymous_pipe.rs b/library/std/src/sys/anonymous_pipe.rs index 230f56c2001e8..1481fcf30e4b9 100644 --- a/library/std/src/sys/anonymous_pipe.rs +++ b/library/std/src/sys/anonymous_pipe.rs @@ -161,42 +161,22 @@ mod unix { } } - enum AccessMode { - Readable, - Writable, - } - - fn check_access_mode(pipe: AnonPipe, expected_access_mode: AccessMode) -> io::Result { - let ret = unsafe { libc::fcntl(pipe.as_raw_fd(), libc::F_GETFL) }; - let access_mode = ret & libc::O_ACCMODE; - let expected_access_mode_str = match expected_access_mode { - AccessMode::Readable => "readable", - AccessMode::Writable => "writable", - }; - let expected_access_mode = match expected_access_mode { - AccessMode::Readable => libc::O_RDONLY, - AccessMode::Writable => libc::O_WRONLY, - }; - - if ret == -1 { - Err(io::Error::last_os_error()) - } else if access_mode == libc::O_RDWR && access_mode == expected_access_mode { - Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("Pipe {} is not {}", pipe.as_raw_fd(), expected_access_mode_str), - )) - } else { - Ok(pipe) - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] impl TryFrom for PipeReader { type Error = io::Error; fn try_from(owned_fd: OwnedFd) -> Result { convert_to_pipe(owned_fd) - .and_then(|pipe| check_access_mode(pipe, AccessMode::Readable)) + .and_then(|pipe| { + if pipe.as_file_desc().get_access_mode()?.readable { + Ok(pipe) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Pipe {} is not readable", pipe.as_raw_fd()), + )) + } + }) .map(Self) } } @@ -207,7 +187,16 @@ mod unix { fn try_from(owned_fd: OwnedFd) -> Result { convert_to_pipe(owned_fd) - .and_then(|pipe| check_access_mode(pipe, AccessMode::Writable)) + .and_then(|pipe| { + if pipe.as_file_desc().get_access_mode()?.writable { + Ok(pipe) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Pipe {} is not writable", pipe.as_raw_fd()), + )) + } + }) .map(Self) } } diff --git a/library/std/src/sys/pal/unix/fd.rs b/library/std/src/sys/pal/unix/fd.rs index 1701717db597c..d889a95f891a7 100644 --- a/library/std/src/sys/pal/unix/fd.rs +++ b/library/std/src/sys/pal/unix/fd.rs @@ -26,6 +26,11 @@ use libc::off64_t; )))] use libc::off_t as off64_t; +pub struct AccessMode { + pub readable: bool, + pub writable: bool, +} + #[derive(Debug)] pub struct FileDesc(OwnedFd); @@ -518,20 +523,27 @@ impl FileDesc { } } + fn get_flags(&self) -> io::Result { + unsafe { cvt(libc::fcntl(self.as_raw_fd(), libc::F_GETFL)) } + } + + pub fn get_access_mode(&self) -> io::Result { + let access_mode = self.get_flags()? & libc::O_ACCMODE; + Ok(AccessMode { + readable: access_mode == libc::O_RDWR || access_mode == libc::O_RDONLY, + writable: access_mode == libc::O_RDWR || access_mode == libc::O_WRONLY, + }) + } + #[cfg(not(target_os = "linux"))] pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { - unsafe { - let previous = cvt(libc::fcntl(self.as_raw_fd(), libc::F_GETFL))?; - let new = if nonblocking { - previous | libc::O_NONBLOCK - } else { - previous & !libc::O_NONBLOCK - }; - if new != previous { - cvt(libc::fcntl(self.as_raw_fd(), libc::F_SETFL, new))?; - } - Ok(()) + let previous = self.get_flags()?; + let new = + if nonblocking { previous | libc::O_NONBLOCK } else { previous & !libc::O_NONBLOCK }; + if new != previous { + unsafe { cvt(libc::fcntl(self.as_raw_fd(), libc::F_SETFL, new)) }?; } + Ok(()) } #[inline] diff --git a/library/std/src/sys/pal/unix/pipe.rs b/library/std/src/sys/pal/unix/pipe.rs index c2fb9c449cf08..8762af614f17e 100644 --- a/library/std/src/sys/pal/unix/pipe.rs +++ b/library/std/src/sys/pal/unix/pipe.rs @@ -84,6 +84,10 @@ impl AnonPipe { pub fn is_write_vectored(&self) -> bool { self.0.is_write_vectored() } + + pub fn as_file_desc(&self) -> &FileDesc { + &self.0 + } } impl IntoInner for AnonPipe { From 4c6b6bbb850627ab22f2866b750770bd4e90a183 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Jul 2024 00:15:31 +1000 Subject: [PATCH 12/21] Add testing for anonymous pipe Signed-off-by: Jiahao XU --- .../mod.rs} | 3 + library/std/src/sys/anonymous_pipe/tests.rs | 128 ++++++++++++++++++ 2 files changed, 131 insertions(+) rename library/std/src/sys/{anonymous_pipe.rs => anonymous_pipe/mod.rs} (99%) create mode 100644 library/std/src/sys/anonymous_pipe/tests.rs diff --git a/library/std/src/sys/anonymous_pipe.rs b/library/std/src/sys/anonymous_pipe/mod.rs similarity index 99% rename from library/std/src/sys/anonymous_pipe.rs rename to library/std/src/sys/anonymous_pipe/mod.rs index 1481fcf30e4b9..9f2483ad290c2 100644 --- a/library/std/src/sys/anonymous_pipe.rs +++ b/library/std/src/sys/anonymous_pipe/mod.rs @@ -295,3 +295,6 @@ mod windows { } } } + +#[cfg(test)] +mod tests; diff --git a/library/std/src/sys/anonymous_pipe/tests.rs b/library/std/src/sys/anonymous_pipe/tests.rs new file mode 100644 index 0000000000000..048e011fc5eaf --- /dev/null +++ b/library/std/src/sys/anonymous_pipe/tests.rs @@ -0,0 +1,128 @@ +use super::*; +use crate::io::{Read, Write}; + +#[test] +fn pipe_creation_and_rw() { + let (mut rx, mut tx) = pipe().unwrap(); + tx.write_all(b"12345").unwrap(); + drop(tx); + + let mut s = String::new(); + rx.read_to_string(&mut s).unwrap(); + assert_eq!(s, "12345"); +} + +#[test] +fn pipe_try_clone_and_rw() { + let (mut rx, mut tx) = pipe().unwrap(); + tx.try_clone().unwrap().write_all(b"12").unwrap(); + tx.write_all(b"345").unwrap(); + drop(tx); + + let mut s = String::new(); + rx.try_clone().unwrap().take(3).read_to_string(&mut s).unwrap(); + assert_eq!(s, "123"); + + s.clear(); + rx.read_to_string(&mut s).unwrap(); + assert_eq!(s, "45"); +} + +#[cfg(unix)] +mod unix_specific { + use super::*; + + use crate::{ + fs::File, + io, + os::fd::{AsRawFd, OwnedFd}, + }; + + #[test] + fn pipe_owned_fd_round_trip_conversion() { + let (rx, tx) = pipe().unwrap(); + let raw_fds = (rx.as_raw_fd(), tx.as_raw_fd()); + let (rx_owned_fd, tx_owned_fd) = (OwnedFd::from(rx), OwnedFd::from(tx)); + + let rx = PipeReader::try_from(rx_owned_fd).unwrap(); + let tx = PipeWriter::try_from(tx_owned_fd).unwrap(); + assert_eq!(raw_fds, (rx.as_raw_fd(), tx.as_raw_fd())); + } + + #[test] + fn convert_from_non_pipe_to_pipe_reader_shall_fail() { + let file = File::open("/dev/zero").unwrap(); + let err = PipeReader::try_from(OwnedFd::from(file)).unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert_eq!(format!("{}", err.get_ref().unwrap()), "Not a pipe"); + } + + #[test] + fn convert_from_non_pipe_to_pipe_writer_shall_fail() { + let file = File::options().write(true).open("/dev/null").unwrap(); + let err = PipeWriter::try_from(OwnedFd::from(file)).unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert_eq!(format!("{}", err.get_ref().unwrap()), "Not a pipe"); + } + + #[test] + fn convert_pipe_writer_to_pipe_reader_shall_fail() { + let (_, tx) = pipe().unwrap(); + let fd = tx.as_raw_fd(); + let err = PipeReader::try_from(OwnedFd::from(tx)).unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert_eq!(format!("{}", err.get_ref().unwrap()), format!("Pipe {fd} is not readable")); + } + + #[test] + fn convert_pipe_reader_to_pipe_writer_shall_fail() { + let (rx, _) = pipe().unwrap(); + let fd = rx.as_raw_fd(); + let err = PipeWriter::try_from(OwnedFd::from(rx)).unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert_eq!(format!("{}", err.get_ref().unwrap()), format!("Pipe {fd} is not writable")); + } +} + +#[cfg(windows)] +mod windows_specific { + use super::*; + + use crate::{ + io, + os::windows::io::{AsHandle, AsRawHandle, OwnedHandle}, + }; + + #[test] + fn pipe_owned_handle_round_trip_conversion() { + let (rx, tx) = pipe().unwrap(); + let raw_handles = (rx.as_raw_handle(), tx.as_raw_handle()); + let (rx_owned_handle, tx_owned_handle) = (OwnedHandle::from(rx), OwnedHandle::from(tx)); + + let rx = PipeReader::try_from(rx_owned_handle).unwrap(); + let tx = PipeWriter::try_from(tx_owned_handle).unwrap(); + assert_eq!(raw_handles, (rx.as_raw_handle(), tx.as_raw_handle())); + } + + #[test] + fn convert_from_non_pipe_to_pipe_reader_shall_fail() { + let file = io::stdin().as_handle().try_clone_to_owned().unwrap(); + let err = PipeReader::try_from(OwnedHandle::from(file)).unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert_eq!(format!("{}", err.get_ref().unwrap()), "Not a pipe"); + } + + #[test] + fn convert_from_non_pipe_to_pipe_writer_shall_fail() { + let file = io::stdout().as_handle().try_clone_to_owned().unwrap(); + let err = PipeWriter::try_from(OwnedHandle::from(file)).unwrap_err(); + + assert_eq!(err.kind(), io::ErrorKind::InvalidInput); + assert_eq!(format!("{}", err.get_ref().unwrap()), "Not a pipe"); + } +} From 594abecd54ba08b766b2410c53872165055c7d62 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Jul 2024 19:14:56 +1000 Subject: [PATCH 13/21] Refactor: Put mod `unix` & `windows` into separate files Signed-off-by: Jiahao XU --- library/std/src/sys/anonymous_pipe/mod.rs | 207 +----------------- library/std/src/sys/anonymous_pipe/unix.rs | 111 ++++++++++ library/std/src/sys/anonymous_pipe/windows.rs | 89 ++++++++ 3 files changed, 202 insertions(+), 205 deletions(-) create mode 100644 library/std/src/sys/anonymous_pipe/unix.rs create mode 100644 library/std/src/sys/anonymous_pipe/windows.rs diff --git a/library/std/src/sys/anonymous_pipe/mod.rs b/library/std/src/sys/anonymous_pipe/mod.rs index 9f2483ad290c2..76c4a7c060224 100644 --- a/library/std/src/sys/anonymous_pipe/mod.rs +++ b/library/std/src/sys/anonymous_pipe/mod.rs @@ -88,213 +88,10 @@ forward_io_write_traits!(PipeWriter); forward_io_write_traits!(&PipeWriter); #[cfg(unix)] -mod unix { - use super::*; - - use crate::{ - fs::File, - os::{ - fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}, - unix::fs::FileTypeExt, - }, - sys::{ - fd::FileDesc, - pipe::{anon_pipe, AnonPipe}, - }, - sys_common::{FromInner, IntoInner}, - }; - - #[inline] - pub(super) fn pipe() -> io::Result<(PipeReader, PipeWriter)> { - anon_pipe().map(|(rx, tx)| (PipeReader(rx), PipeWriter(tx))) - } - - macro_rules! impl_traits { - ($name:ty) => { - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl AsFd for $name { - fn as_fd(&self) -> BorrowedFd<'_> { - self.0.as_fd() - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl AsRawFd for $name { - fn as_raw_fd(&self) -> RawFd { - self.0.as_raw_fd() - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl From<$name> for OwnedFd { - fn from(pipe: $name) -> Self { - FileDesc::into_inner(AnonPipe::into_inner(pipe.0)) - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl FromRawFd for $name { - unsafe fn from_raw_fd(raw_fd: RawFd) -> Self { - Self(AnonPipe::from_raw_fd(raw_fd)) - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl IntoRawFd for $name { - fn into_raw_fd(self) -> RawFd { - self.0.into_raw_fd() - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl From<$name> for Stdio { - fn from(pipe: $name) -> Self { - Self::from(OwnedFd::from(pipe)) - } - } - }; - } - impl_traits!(PipeReader); - impl_traits!(PipeWriter); - - fn convert_to_pipe(owned_fd: OwnedFd) -> io::Result { - let file = File::from(owned_fd); - if file.metadata()?.file_type().is_fifo() { - Ok(AnonPipe::from_inner(FileDesc::from_inner(OwnedFd::from(file)))) - } else { - Err(io::Error::new(io::ErrorKind::InvalidInput, "Not a pipe")) - } - } - - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl TryFrom for PipeReader { - type Error = io::Error; - - fn try_from(owned_fd: OwnedFd) -> Result { - convert_to_pipe(owned_fd) - .and_then(|pipe| { - if pipe.as_file_desc().get_access_mode()?.readable { - Ok(pipe) - } else { - Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("Pipe {} is not readable", pipe.as_raw_fd()), - )) - } - }) - .map(Self) - } - } - - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl TryFrom for PipeWriter { - type Error = io::Error; - - fn try_from(owned_fd: OwnedFd) -> Result { - convert_to_pipe(owned_fd) - .and_then(|pipe| { - if pipe.as_file_desc().get_access_mode()?.writable { - Ok(pipe) - } else { - Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("Pipe {} is not writable", pipe.as_raw_fd()), - )) - } - }) - .map(Self) - } - } -} +mod unix; #[cfg(windows)] -mod windows { - use super::*; - - use crate::{ - os::windows::io::{ - AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, - RawHandle, - }, - sys::{ - c::{GetFileType, FILE_TYPE_PIPE}, - handle::Handle, - pipe::{anon_pipe, AnonPipe, Pipes}, - }, - sys_common::{FromInner, IntoInner}, - }; - - #[inline] - pub(super) fn pipe() -> io::Result<(PipeReader, PipeWriter)> { - anon_pipe(true, false).map(|Pipes { ours, theirs }| (PipeReader(ours), PipeWriter(theirs))) - } - - macro_rules! impl_traits { - ($name:ty) => { - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl AsHandle for $name { - fn as_handle(&self) -> BorrowedHandle<'_> { - self.0.handle().as_handle() - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl AsRawHandle for $name { - fn as_raw_handle(&self) -> RawHandle { - self.0.handle().as_raw_handle() - } - } - - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl FromRawHandle for $name { - unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { - Self(AnonPipe::from_inner(Handle::from_raw_handle(raw_handle))) - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl IntoRawHandle for $name { - fn into_raw_handle(self) -> RawHandle { - self.0.into_handle().into_raw_handle() - } - } - - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl From<$name> for OwnedHandle { - fn from(pipe: $name) -> Self { - Handle::into_inner(AnonPipe::into_inner(pipe.0)) - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl From<$name> for Stdio { - fn from(pipe: $name) -> Self { - Self::from(OwnedHandle::from(pipe)) - } - } - }; - } - impl_traits!(PipeReader); - impl_traits!(PipeWriter); - - fn convert_to_pipe(owned_handle: OwnedHandle) -> io::Result { - if unsafe { GetFileType(owned_handle.as_raw_handle()) } == FILE_TYPE_PIPE { - Ok(AnonPipe::from_inner(Handle::from_inner(owned_handle))) - } else { - Err(io::Error::new(io::ErrorKind::InvalidInput, "Not a pipe")) - } - } - - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl TryFrom for PipeReader { - type Error = io::Error; - - fn try_from(owned_handle: OwnedHandle) -> Result { - convert_to_pipe(owned_handle).map(Self) - } - } - - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl TryFrom for PipeWriter { - type Error = io::Error; - - fn try_from(owned_handle: OwnedHandle) -> Result { - convert_to_pipe(owned_handle).map(Self) - } - } -} +mod windows; #[cfg(test)] mod tests; diff --git a/library/std/src/sys/anonymous_pipe/unix.rs b/library/std/src/sys/anonymous_pipe/unix.rs new file mode 100644 index 0000000000000..0438d933a06c0 --- /dev/null +++ b/library/std/src/sys/anonymous_pipe/unix.rs @@ -0,0 +1,111 @@ +use super::*; + +use crate::{ + fs::File, + os::{ + fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}, + unix::fs::FileTypeExt, + }, + sys::{ + fd::FileDesc, + pipe::{anon_pipe, AnonPipe}, + }, + sys_common::{FromInner, IntoInner}, +}; + +#[inline] +pub(super) fn pipe() -> io::Result<(PipeReader, PipeWriter)> { + anon_pipe().map(|(rx, tx)| (PipeReader(rx), PipeWriter(tx))) +} + +macro_rules! impl_traits { + ($name:ty) => { + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl AsFd for $name { + fn as_fd(&self) -> BorrowedFd<'_> { + self.0.as_fd() + } + } + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl AsRawFd for $name { + fn as_raw_fd(&self) -> RawFd { + self.0.as_raw_fd() + } + } + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl From<$name> for OwnedFd { + fn from(pipe: $name) -> Self { + FileDesc::into_inner(AnonPipe::into_inner(pipe.0)) + } + } + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl FromRawFd for $name { + unsafe fn from_raw_fd(raw_fd: RawFd) -> Self { + Self(AnonPipe::from_raw_fd(raw_fd)) + } + } + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl IntoRawFd for $name { + fn into_raw_fd(self) -> RawFd { + self.0.into_raw_fd() + } + } + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl From<$name> for Stdio { + fn from(pipe: $name) -> Self { + Self::from(OwnedFd::from(pipe)) + } + } + }; +} +impl_traits!(PipeReader); +impl_traits!(PipeWriter); + +fn convert_to_pipe(owned_fd: OwnedFd) -> io::Result { + let file = File::from(owned_fd); + if file.metadata()?.file_type().is_fifo() { + Ok(AnonPipe::from_inner(FileDesc::from_inner(OwnedFd::from(file)))) + } else { + Err(io::Error::new(io::ErrorKind::InvalidInput, "Not a pipe")) + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl TryFrom for PipeReader { + type Error = io::Error; + + fn try_from(owned_fd: OwnedFd) -> Result { + convert_to_pipe(owned_fd) + .and_then(|pipe| { + if pipe.as_file_desc().get_access_mode()?.readable { + Ok(pipe) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Pipe {} is not readable", pipe.as_raw_fd()), + )) + } + }) + .map(Self) + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl TryFrom for PipeWriter { + type Error = io::Error; + + fn try_from(owned_fd: OwnedFd) -> Result { + convert_to_pipe(owned_fd) + .and_then(|pipe| { + if pipe.as_file_desc().get_access_mode()?.writable { + Ok(pipe) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("Pipe {} is not writable", pipe.as_raw_fd()), + )) + } + }) + .map(Self) + } +} diff --git a/library/std/src/sys/anonymous_pipe/windows.rs b/library/std/src/sys/anonymous_pipe/windows.rs new file mode 100644 index 0000000000000..cd132c5a60f41 --- /dev/null +++ b/library/std/src/sys/anonymous_pipe/windows.rs @@ -0,0 +1,89 @@ +use super::*; + +use crate::{ + os::windows::io::{ + AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, + }, + sys::{ + c::{GetFileType, FILE_TYPE_PIPE}, + handle::Handle, + pipe::{anon_pipe, AnonPipe, Pipes}, + }, + sys_common::{FromInner, IntoInner}, +}; + +#[inline] +pub(super) fn pipe() -> io::Result<(PipeReader, PipeWriter)> { + anon_pipe(true, false).map(|Pipes { ours, theirs }| (PipeReader(ours), PipeWriter(theirs))) +} + +macro_rules! impl_traits { + ($name:ty) => { + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl AsHandle for $name { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.0.handle().as_handle() + } + } + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl AsRawHandle for $name { + fn as_raw_handle(&self) -> RawHandle { + self.0.handle().as_raw_handle() + } + } + + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl FromRawHandle for $name { + unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { + Self(AnonPipe::from_inner(Handle::from_raw_handle(raw_handle))) + } + } + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl IntoRawHandle for $name { + fn into_raw_handle(self) -> RawHandle { + self.0.into_handle().into_raw_handle() + } + } + + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl From<$name> for OwnedHandle { + fn from(pipe: $name) -> Self { + Handle::into_inner(AnonPipe::into_inner(pipe.0)) + } + } + #[unstable(feature = "anonymous_pipe", issue = "127154")] + impl From<$name> for Stdio { + fn from(pipe: $name) -> Self { + Self::from(OwnedHandle::from(pipe)) + } + } + }; +} +impl_traits!(PipeReader); +impl_traits!(PipeWriter); + +fn convert_to_pipe(owned_handle: OwnedHandle) -> io::Result { + if unsafe { GetFileType(owned_handle.as_raw_handle()) } == FILE_TYPE_PIPE { + Ok(AnonPipe::from_inner(Handle::from_inner(owned_handle))) + } else { + Err(io::Error::new(io::ErrorKind::InvalidInput, "Not a pipe")) + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl TryFrom for PipeReader { + type Error = io::Error; + + fn try_from(owned_handle: OwnedHandle) -> Result { + convert_to_pipe(owned_handle).map(Self) + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl TryFrom for PipeWriter { + type Error = io::Error; + + fn try_from(owned_handle: OwnedHandle) -> Result { + convert_to_pipe(owned_handle).map(Self) + } +} From d9f09809ce4a22a0e44f197c9dc435a1c7f86113 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Jul 2024 19:22:10 +1000 Subject: [PATCH 14/21] Fix `anonymous_pipe` impl for not supported targets Signed-off-by: Jiahao XU --- library/std/src/sys/anonymous_pipe/mod.rs | 4 +++- library/std/src/sys/pal/unsupported/pipe.rs | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/library/std/src/sys/anonymous_pipe/mod.rs b/library/std/src/sys/anonymous_pipe/mod.rs index 76c4a7c060224..afd07c23ad075 100644 --- a/library/std/src/sys/anonymous_pipe/mod.rs +++ b/library/std/src/sys/anonymous_pipe/mod.rs @@ -7,8 +7,10 @@ pub fn pipe() -> io::Result<(PipeReader, PipeWriter)> { cfg_if::cfg_if! { if #[cfg(unix)] { unix::pipe() - } else { + } else if #[cfg(windows)] { windows::pipe() + } else { + panic!("Anonymous pipe is not supported on this target!") } } } diff --git a/library/std/src/sys/pal/unsupported/pipe.rs b/library/std/src/sys/pal/unsupported/pipe.rs index d7d8f297ae586..eaa95594db0b3 100644 --- a/library/std/src/sys/pal/unsupported/pipe.rs +++ b/library/std/src/sys/pal/unsupported/pipe.rs @@ -3,6 +3,10 @@ use crate::io::{self, BorrowedCursor, IoSlice, IoSliceMut}; pub struct AnonPipe(!); impl AnonPipe { + pub fn try_clone(&self) -> io::Result { + self.0 + } + pub fn read(&self, _buf: &mut [u8]) -> io::Result { self.0 } From 6c755a32a96c9d49c42d477b5cd5c3ffa82aa8df Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 2 Jul 2024 19:23:34 +1000 Subject: [PATCH 15/21] Optimize: Add `#[inline]` to very simple function Signed-off-by: Jiahao XU --- library/std/src/sys/anonymous_pipe/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/std/src/sys/anonymous_pipe/mod.rs b/library/std/src/sys/anonymous_pipe/mod.rs index afd07c23ad075..f2e0ebc15b1dd 100644 --- a/library/std/src/sys/anonymous_pipe/mod.rs +++ b/library/std/src/sys/anonymous_pipe/mod.rs @@ -51,6 +51,7 @@ macro_rules! forward_io_read_traits { fn read_vectored(&mut self, bufs: &mut [io::IoSliceMut<'_>]) -> io::Result { self.0.read_vectored(bufs) } + #[inline] fn is_read_vectored(&self) -> bool { self.0.is_read_vectored() } @@ -73,6 +74,7 @@ macro_rules! forward_io_write_traits { fn write(&mut self, buf: &[u8]) -> io::Result { self.0.write(buf) } + #[inline] fn flush(&mut self) -> io::Result<()> { Ok(()) } @@ -80,6 +82,8 @@ macro_rules! forward_io_write_traits { fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result { self.0.write_vectored(bufs) } + + #[inline] fn is_write_vectored(&self) -> bool { self.0.is_write_vectored() } From d15cee549cafe4b47669bc722c2beec959ce73c8 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 9 Jul 2024 22:46:41 +1000 Subject: [PATCH 16/21] Refactor: Make `AcessMode` an enum` Signed-off-by: Jiahao XU --- library/std/src/sys/anonymous_pipe/unix.rs | 4 ++-- library/std/src/sys/pal/unix/fd.rs | 28 +++++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/library/std/src/sys/anonymous_pipe/unix.rs b/library/std/src/sys/anonymous_pipe/unix.rs index 0438d933a06c0..2e1d8b0094638 100644 --- a/library/std/src/sys/anonymous_pipe/unix.rs +++ b/library/std/src/sys/anonymous_pipe/unix.rs @@ -77,7 +77,7 @@ impl TryFrom for PipeReader { fn try_from(owned_fd: OwnedFd) -> Result { convert_to_pipe(owned_fd) .and_then(|pipe| { - if pipe.as_file_desc().get_access_mode()?.readable { + if pipe.as_file_desc().get_access_mode()?.is_readable() { Ok(pipe) } else { Err(io::Error::new( @@ -97,7 +97,7 @@ impl TryFrom for PipeWriter { fn try_from(owned_fd: OwnedFd) -> Result { convert_to_pipe(owned_fd) .and_then(|pipe| { - if pipe.as_file_desc().get_access_mode()?.writable { + if pipe.as_file_desc().get_access_mode()?.is_writable() { Ok(pipe) } else { Err(io::Error::new( diff --git a/library/std/src/sys/pal/unix/fd.rs b/library/std/src/sys/pal/unix/fd.rs index d889a95f891a7..2536dd845548d 100644 --- a/library/std/src/sys/pal/unix/fd.rs +++ b/library/std/src/sys/pal/unix/fd.rs @@ -26,9 +26,22 @@ use libc::off64_t; )))] use libc::off_t as off64_t; -pub struct AccessMode { - pub readable: bool, - pub writable: bool, +#[derive(Copy, Clone)] +pub enum AccessMode { + ReadOnly, + WriteOnly, + ReadWrite, + None, +} + +impl AccessMode { + pub fn is_readable(self) -> bool { + matches!(self, AccessMode::ReadOnly | AccessMode::ReadWrite) + } + + pub fn is_writable(self) -> bool { + matches!(self, AccessMode::WriteOnly | AccessMode::ReadWrite) + } } #[derive(Debug)] @@ -529,9 +542,12 @@ impl FileDesc { pub fn get_access_mode(&self) -> io::Result { let access_mode = self.get_flags()? & libc::O_ACCMODE; - Ok(AccessMode { - readable: access_mode == libc::O_RDWR || access_mode == libc::O_RDONLY, - writable: access_mode == libc::O_RDWR || access_mode == libc::O_WRONLY, + Ok(match access_mode { + libc::O_RDWR => AccessMode::ReadWrite, + libc::O_RDONLY => AccessMode::ReadOnly, + libc::O_WRONLY => AccessMode::WriteOnly, + + _ => AccessMode::None, }) } From e22dd1adc2b250f927b666d0260bc23008cbcdcf Mon Sep 17 00:00:00 2001 From: Jiahao XU <30436523+NobodyXu@users.noreply.github.com> Date: Wed, 10 Jul 2024 22:27:58 +1000 Subject: [PATCH 17/21] Update mod.rs Co-authored-by: Alphyr <47725341+a1phyr@users.noreply.github.com> --- library/std/src/sys/anonymous_pipe/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/anonymous_pipe/mod.rs b/library/std/src/sys/anonymous_pipe/mod.rs index f2e0ebc15b1dd..5b60e6b51d6b6 100644 --- a/library/std/src/sys/anonymous_pipe/mod.rs +++ b/library/std/src/sys/anonymous_pipe/mod.rs @@ -10,7 +10,7 @@ pub fn pipe() -> io::Result<(PipeReader, PipeWriter)> { } else if #[cfg(windows)] { windows::pipe() } else { - panic!("Anonymous pipe is not supported on this target!") + Err(io::Error::UNSUPPORTED_PLATFORM) } } } From 100fe5cbd323062f6a8cf95100b7fb9e9cd00dea Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 10 Jul 2024 23:20:12 +1000 Subject: [PATCH 18/21] Move `std::net::pip*` to a new mod `std::pipe` Signed-off-by: Jiahao XU --- library/std/src/lib.rs | 3 +++ library/std/src/net/mod.rs | 2 -- library/std/src/sys/anonymous_pipe/mod.rs | 10 ++++++++++ library/std/src/sys/mod.rs | 1 + 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/library/std/src/lib.rs b/library/std/src/lib.rs index 27ed2e4137c7e..b08d903fefda4 100644 --- a/library/std/src/lib.rs +++ b/library/std/src/lib.rs @@ -685,6 +685,9 @@ pub use core::{ module_path, option_env, stringify, trace_macros, }; +#[unstable(feature = "anonymous_pipe", issue = "127154")] +pub use crate::sys::anonymous_pipe as pipe; + #[unstable( feature = "concat_bytes", issue = "87555", diff --git a/library/std/src/net/mod.rs b/library/std/src/net/mod.rs index cacf8c01f672d..858776f14466a 100644 --- a/library/std/src/net/mod.rs +++ b/library/std/src/net/mod.rs @@ -33,8 +33,6 @@ pub use self::tcp::IntoIncoming; pub use self::tcp::{Incoming, TcpListener, TcpStream}; #[stable(feature = "rust1", since = "1.0.0")] pub use self::udp::UdpSocket; -#[unstable(feature = "anonymous_pipe", issue = "127154")] -pub use crate::sys::anonymous_pipe::{pipe, PipeReader, PipeWriter}; #[stable(feature = "rust1", since = "1.0.0")] pub use core::net::AddrParseError; diff --git a/library/std/src/sys/anonymous_pipe/mod.rs b/library/std/src/sys/anonymous_pipe/mod.rs index 5b60e6b51d6b6..3f660b8b21995 100644 --- a/library/std/src/sys/anonymous_pipe/mod.rs +++ b/library/std/src/sys/anonymous_pipe/mod.rs @@ -1,3 +1,13 @@ +//! Module for annoymous pipe +//! +//! ``` +//! #![feature(anonymous_pipe)] +//! # fn main() -> std::io::Result<()> { +//! let (reader, writer) = std::pipe::pipe()?; +//! # Ok(()) +//! # } +//! ``` + use crate::{io, process::Stdio, sys::pipe::AnonPipe}; /// Create annoymous pipe that is close-on-exec and blocking. diff --git a/library/std/src/sys/mod.rs b/library/std/src/sys/mod.rs index fad01f581577d..f7b41a2d4064a 100644 --- a/library/std/src/sys/mod.rs +++ b/library/std/src/sys/mod.rs @@ -5,6 +5,7 @@ mod pal; mod personality; +#[unstable(feature = "anonymous_pipe", issue = "127154")] pub mod anonymous_pipe; pub mod backtrace; pub mod cmath; From 45ad522e8745b9d2da00c56e93cca4d2f8c8fe7c Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Wed, 10 Jul 2024 15:47:24 +0200 Subject: [PATCH 19/21] Don't mark `DEBUG_EVENT` struct as `repr(packed)` That would give it alignment of 1 which is ABI-incompatible with its C definition. --- library/std/src/process/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/process/tests.rs b/library/std/src/process/tests.rs index 63455a274faaa..b4f6bb71c79c4 100644 --- a/library/std/src/process/tests.rs +++ b/library/std/src/process/tests.rs @@ -386,7 +386,7 @@ fn test_interior_nul_in_env_value_is_error() { fn test_creation_flags() { use crate::os::windows::process::CommandExt; use crate::sys::c::{BOOL, DWORD, INFINITE}; - #[repr(C, packed)] + #[repr(C)] struct DEBUG_EVENT { pub event_code: DWORD, pub process_id: DWORD, From 62b846ebe420574febdceb01ede043fd959818c6 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Wed, 10 Jul 2024 23:49:53 +1000 Subject: [PATCH 20/21] Remove use of `macro_rules!` Signed-off-by: Jiahao XU --- library/std/src/sys/anonymous_pipe/mod.rs | 126 +++++++++++------- library/std/src/sys/anonymous_pipe/unix.rs | 115 ++++++++++------ library/std/src/sys/anonymous_pipe/windows.rs | 117 ++++++++++------ 3 files changed, 225 insertions(+), 133 deletions(-) diff --git a/library/std/src/sys/anonymous_pipe/mod.rs b/library/std/src/sys/anonymous_pipe/mod.rs index 3f660b8b21995..118bcbaa75bf7 100644 --- a/library/std/src/sys/anonymous_pipe/mod.rs +++ b/library/std/src/sys/anonymous_pipe/mod.rs @@ -51,57 +51,85 @@ impl PipeWriter { } } -macro_rules! forward_io_read_traits { - ($name:ty) => { - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl io::Read for $name { - fn read(&mut self, buf: &mut [u8]) -> io::Result { - self.0.read(buf) - } - fn read_vectored(&mut self, bufs: &mut [io::IoSliceMut<'_>]) -> io::Result { - self.0.read_vectored(bufs) - } - #[inline] - fn is_read_vectored(&self) -> bool { - self.0.is_read_vectored() - } - fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { - self.0.read_to_end(buf) - } - fn read_buf(&mut self, buf: io::BorrowedCursor<'_>) -> io::Result<()> { - self.0.read_buf(buf) - } - } - }; +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl io::Read for &PipeReader { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + self.0.read(buf) + } + fn read_vectored(&mut self, bufs: &mut [io::IoSliceMut<'_>]) -> io::Result { + self.0.read_vectored(bufs) + } + #[inline] + fn is_read_vectored(&self) -> bool { + self.0.is_read_vectored() + } + fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { + self.0.read_to_end(buf) + } + fn read_buf(&mut self, buf: io::BorrowedCursor<'_>) -> io::Result<()> { + self.0.read_buf(buf) + } } -forward_io_read_traits!(PipeReader); -forward_io_read_traits!(&PipeReader); - -macro_rules! forward_io_write_traits { - ($name:ty) => { - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl io::Write for $name { - fn write(&mut self, buf: &[u8]) -> io::Result { - self.0.write(buf) - } - #[inline] - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } - - fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result { - self.0.write_vectored(bufs) - } - - #[inline] - fn is_write_vectored(&self) -> bool { - self.0.is_write_vectored() - } - } - }; + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl io::Read for PipeReader { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + self.0.read(buf) + } + fn read_vectored(&mut self, bufs: &mut [io::IoSliceMut<'_>]) -> io::Result { + self.0.read_vectored(bufs) + } + #[inline] + fn is_read_vectored(&self) -> bool { + self.0.is_read_vectored() + } + fn read_to_end(&mut self, buf: &mut Vec) -> io::Result { + self.0.read_to_end(buf) + } + fn read_buf(&mut self, buf: io::BorrowedCursor<'_>) -> io::Result<()> { + self.0.read_buf(buf) + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl io::Write for &PipeWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.write(buf) + } + #[inline] + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + + fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result { + self.0.write_vectored(bufs) + } + + #[inline] + fn is_write_vectored(&self) -> bool { + self.0.is_write_vectored() + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl io::Write for PipeWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.0.write(buf) + } + #[inline] + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + + fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result { + self.0.write_vectored(bufs) + } + + #[inline] + fn is_write_vectored(&self) -> bool { + self.0.is_write_vectored() + } } -forward_io_write_traits!(PipeWriter); -forward_io_write_traits!(&PipeWriter); #[cfg(unix)] mod unix; diff --git a/library/std/src/sys/anonymous_pipe/unix.rs b/library/std/src/sys/anonymous_pipe/unix.rs index 2e1d8b0094638..8ab36dea4cdd2 100644 --- a/library/std/src/sys/anonymous_pipe/unix.rs +++ b/library/std/src/sys/anonymous_pipe/unix.rs @@ -18,48 +18,79 @@ pub(super) fn pipe() -> io::Result<(PipeReader, PipeWriter)> { anon_pipe().map(|(rx, tx)| (PipeReader(rx), PipeWriter(tx))) } -macro_rules! impl_traits { - ($name:ty) => { - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl AsFd for $name { - fn as_fd(&self) -> BorrowedFd<'_> { - self.0.as_fd() - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl AsRawFd for $name { - fn as_raw_fd(&self) -> RawFd { - self.0.as_raw_fd() - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl From<$name> for OwnedFd { - fn from(pipe: $name) -> Self { - FileDesc::into_inner(AnonPipe::into_inner(pipe.0)) - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl FromRawFd for $name { - unsafe fn from_raw_fd(raw_fd: RawFd) -> Self { - Self(AnonPipe::from_raw_fd(raw_fd)) - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl IntoRawFd for $name { - fn into_raw_fd(self) -> RawFd { - self.0.into_raw_fd() - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl From<$name> for Stdio { - fn from(pipe: $name) -> Self { - Self::from(OwnedFd::from(pipe)) - } - } - }; -} -impl_traits!(PipeReader); -impl_traits!(PipeWriter); +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl AsFd for PipeReader { + fn as_fd(&self) -> BorrowedFd<'_> { + self.0.as_fd() + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl AsRawFd for PipeReader { + fn as_raw_fd(&self) -> RawFd { + self.0.as_raw_fd() + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl From for OwnedFd { + fn from(pipe: PipeReader) -> Self { + FileDesc::into_inner(AnonPipe::into_inner(pipe.0)) + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl FromRawFd for PipeReader { + unsafe fn from_raw_fd(raw_fd: RawFd) -> Self { + Self(AnonPipe::from_raw_fd(raw_fd)) + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl IntoRawFd for PipeReader { + fn into_raw_fd(self) -> RawFd { + self.0.into_raw_fd() + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl From for Stdio { + fn from(pipe: PipeReader) -> Self { + Self::from(OwnedFd::from(pipe)) + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl AsFd for PipeWriter { + fn as_fd(&self) -> BorrowedFd<'_> { + self.0.as_fd() + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl AsRawFd for PipeWriter { + fn as_raw_fd(&self) -> RawFd { + self.0.as_raw_fd() + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl From for OwnedFd { + fn from(pipe: PipeWriter) -> Self { + FileDesc::into_inner(AnonPipe::into_inner(pipe.0)) + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl FromRawFd for PipeWriter { + unsafe fn from_raw_fd(raw_fd: RawFd) -> Self { + Self(AnonPipe::from_raw_fd(raw_fd)) + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl IntoRawFd for PipeWriter { + fn into_raw_fd(self) -> RawFd { + self.0.into_raw_fd() + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl From for Stdio { + fn from(pipe: PipeWriter) -> Self { + Self::from(OwnedFd::from(pipe)) + } +} fn convert_to_pipe(owned_fd: OwnedFd) -> io::Result { let file = File::from(owned_fd); diff --git a/library/std/src/sys/anonymous_pipe/windows.rs b/library/std/src/sys/anonymous_pipe/windows.rs index cd132c5a60f41..f107258ff8ddb 100644 --- a/library/std/src/sys/anonymous_pipe/windows.rs +++ b/library/std/src/sys/anonymous_pipe/windows.rs @@ -17,50 +17,83 @@ pub(super) fn pipe() -> io::Result<(PipeReader, PipeWriter)> { anon_pipe(true, false).map(|Pipes { ours, theirs }| (PipeReader(ours), PipeWriter(theirs))) } -macro_rules! impl_traits { - ($name:ty) => { - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl AsHandle for $name { - fn as_handle(&self) -> BorrowedHandle<'_> { - self.0.handle().as_handle() - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl AsRawHandle for $name { - fn as_raw_handle(&self) -> RawHandle { - self.0.handle().as_raw_handle() - } - } +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl AsHandle for PipeReader { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.0.handle().as_handle() + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl AsRawHandle for PipeReader { + fn as_raw_handle(&self) -> RawHandle { + self.0.handle().as_raw_handle() + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl FromRawHandle for PipeReader { + unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { + Self(AnonPipe::from_inner(Handle::from_raw_handle(raw_handle))) + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl IntoRawHandle for PipeReader { + fn into_raw_handle(self) -> RawHandle { + self.0.into_handle().into_raw_handle() + } +} - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl FromRawHandle for $name { - unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { - Self(AnonPipe::from_inner(Handle::from_raw_handle(raw_handle))) - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl IntoRawHandle for $name { - fn into_raw_handle(self) -> RawHandle { - self.0.into_handle().into_raw_handle() - } - } +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl From for OwnedHandle { + fn from(pipe: PipeReader) -> Self { + Handle::into_inner(AnonPipe::into_inner(pipe.0)) + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl From for Stdio { + fn from(pipe: PipeReader) -> Self { + Self::from(OwnedHandle::from(pipe)) + } +} - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl From<$name> for OwnedHandle { - fn from(pipe: $name) -> Self { - Handle::into_inner(AnonPipe::into_inner(pipe.0)) - } - } - #[unstable(feature = "anonymous_pipe", issue = "127154")] - impl From<$name> for Stdio { - fn from(pipe: $name) -> Self { - Self::from(OwnedHandle::from(pipe)) - } - } - }; -} -impl_traits!(PipeReader); -impl_traits!(PipeWriter); +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl AsHandle for PipeWriter { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.0.handle().as_handle() + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl AsRawHandle for PipeWriter { + fn as_raw_handle(&self) -> RawHandle { + self.0.handle().as_raw_handle() + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl FromRawHandle for PipeWriter { + unsafe fn from_raw_handle(raw_handle: RawHandle) -> Self { + Self(AnonPipe::from_inner(Handle::from_raw_handle(raw_handle))) + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl IntoRawHandle for PipeWriter { + fn into_raw_handle(self) -> RawHandle { + self.0.into_handle().into_raw_handle() + } +} + +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl From for OwnedHandle { + fn from(pipe: PipeWriter) -> Self { + Handle::into_inner(AnonPipe::into_inner(pipe.0)) + } +} +#[unstable(feature = "anonymous_pipe", issue = "127154")] +impl From for Stdio { + fn from(pipe: PipeWriter) -> Self { + Self::from(OwnedHandle::from(pipe)) + } +} fn convert_to_pipe(owned_handle: OwnedHandle) -> io::Result { if unsafe { GetFileType(owned_handle.as_raw_handle()) } == FILE_TYPE_PIPE { From 4547b30b4a2751ac857dd2da5ce758ea520df243 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 11 Jul 2024 00:08:34 +1000 Subject: [PATCH 21/21] Replace `TryFrom` with `From` Signed-off-by: Jiahao XU --- library/std/src/sys/anonymous_pipe/tests.rs | 99 ------------------- library/std/src/sys/anonymous_pipe/unix.rs | 53 ++-------- library/std/src/sys/anonymous_pipe/windows.rs | 25 ++--- library/std/src/sys/pal/unix/fd.rs | 50 +++------- 4 files changed, 28 insertions(+), 199 deletions(-) diff --git a/library/std/src/sys/anonymous_pipe/tests.rs b/library/std/src/sys/anonymous_pipe/tests.rs index 048e011fc5eaf..8f17422901d62 100644 --- a/library/std/src/sys/anonymous_pipe/tests.rs +++ b/library/std/src/sys/anonymous_pipe/tests.rs @@ -27,102 +27,3 @@ fn pipe_try_clone_and_rw() { rx.read_to_string(&mut s).unwrap(); assert_eq!(s, "45"); } - -#[cfg(unix)] -mod unix_specific { - use super::*; - - use crate::{ - fs::File, - io, - os::fd::{AsRawFd, OwnedFd}, - }; - - #[test] - fn pipe_owned_fd_round_trip_conversion() { - let (rx, tx) = pipe().unwrap(); - let raw_fds = (rx.as_raw_fd(), tx.as_raw_fd()); - let (rx_owned_fd, tx_owned_fd) = (OwnedFd::from(rx), OwnedFd::from(tx)); - - let rx = PipeReader::try_from(rx_owned_fd).unwrap(); - let tx = PipeWriter::try_from(tx_owned_fd).unwrap(); - assert_eq!(raw_fds, (rx.as_raw_fd(), tx.as_raw_fd())); - } - - #[test] - fn convert_from_non_pipe_to_pipe_reader_shall_fail() { - let file = File::open("/dev/zero").unwrap(); - let err = PipeReader::try_from(OwnedFd::from(file)).unwrap_err(); - - assert_eq!(err.kind(), io::ErrorKind::InvalidInput); - assert_eq!(format!("{}", err.get_ref().unwrap()), "Not a pipe"); - } - - #[test] - fn convert_from_non_pipe_to_pipe_writer_shall_fail() { - let file = File::options().write(true).open("/dev/null").unwrap(); - let err = PipeWriter::try_from(OwnedFd::from(file)).unwrap_err(); - - assert_eq!(err.kind(), io::ErrorKind::InvalidInput); - assert_eq!(format!("{}", err.get_ref().unwrap()), "Not a pipe"); - } - - #[test] - fn convert_pipe_writer_to_pipe_reader_shall_fail() { - let (_, tx) = pipe().unwrap(); - let fd = tx.as_raw_fd(); - let err = PipeReader::try_from(OwnedFd::from(tx)).unwrap_err(); - - assert_eq!(err.kind(), io::ErrorKind::InvalidInput); - assert_eq!(format!("{}", err.get_ref().unwrap()), format!("Pipe {fd} is not readable")); - } - - #[test] - fn convert_pipe_reader_to_pipe_writer_shall_fail() { - let (rx, _) = pipe().unwrap(); - let fd = rx.as_raw_fd(); - let err = PipeWriter::try_from(OwnedFd::from(rx)).unwrap_err(); - - assert_eq!(err.kind(), io::ErrorKind::InvalidInput); - assert_eq!(format!("{}", err.get_ref().unwrap()), format!("Pipe {fd} is not writable")); - } -} - -#[cfg(windows)] -mod windows_specific { - use super::*; - - use crate::{ - io, - os::windows::io::{AsHandle, AsRawHandle, OwnedHandle}, - }; - - #[test] - fn pipe_owned_handle_round_trip_conversion() { - let (rx, tx) = pipe().unwrap(); - let raw_handles = (rx.as_raw_handle(), tx.as_raw_handle()); - let (rx_owned_handle, tx_owned_handle) = (OwnedHandle::from(rx), OwnedHandle::from(tx)); - - let rx = PipeReader::try_from(rx_owned_handle).unwrap(); - let tx = PipeWriter::try_from(tx_owned_handle).unwrap(); - assert_eq!(raw_handles, (rx.as_raw_handle(), tx.as_raw_handle())); - } - - #[test] - fn convert_from_non_pipe_to_pipe_reader_shall_fail() { - let file = io::stdin().as_handle().try_clone_to_owned().unwrap(); - let err = PipeReader::try_from(OwnedHandle::from(file)).unwrap_err(); - - assert_eq!(err.kind(), io::ErrorKind::InvalidInput); - assert_eq!(format!("{}", err.get_ref().unwrap()), "Not a pipe"); - } - - #[test] - fn convert_from_non_pipe_to_pipe_writer_shall_fail() { - let file = io::stdout().as_handle().try_clone_to_owned().unwrap(); - let err = PipeWriter::try_from(OwnedHandle::from(file)).unwrap_err(); - - assert_eq!(err.kind(), io::ErrorKind::InvalidInput); - assert_eq!(format!("{}", err.get_ref().unwrap()), "Not a pipe"); - } -} diff --git a/library/std/src/sys/anonymous_pipe/unix.rs b/library/std/src/sys/anonymous_pipe/unix.rs index 8ab36dea4cdd2..18252f0183d3b 100644 --- a/library/std/src/sys/anonymous_pipe/unix.rs +++ b/library/std/src/sys/anonymous_pipe/unix.rs @@ -1,11 +1,7 @@ use super::*; use crate::{ - fs::File, - os::{ - fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}, - unix::fs::FileTypeExt, - }, + os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}, sys::{ fd::FileDesc, pipe::{anon_pipe, AnonPipe}, @@ -92,51 +88,20 @@ impl From for Stdio { } } -fn convert_to_pipe(owned_fd: OwnedFd) -> io::Result { - let file = File::from(owned_fd); - if file.metadata()?.file_type().is_fifo() { - Ok(AnonPipe::from_inner(FileDesc::from_inner(OwnedFd::from(file)))) - } else { - Err(io::Error::new(io::ErrorKind::InvalidInput, "Not a pipe")) - } +fn convert_to_pipe(owned_fd: OwnedFd) -> AnonPipe { + AnonPipe::from_inner(FileDesc::from_inner(OwnedFd::from(owned_fd))) } #[unstable(feature = "anonymous_pipe", issue = "127154")] -impl TryFrom for PipeReader { - type Error = io::Error; - - fn try_from(owned_fd: OwnedFd) -> Result { - convert_to_pipe(owned_fd) - .and_then(|pipe| { - if pipe.as_file_desc().get_access_mode()?.is_readable() { - Ok(pipe) - } else { - Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("Pipe {} is not readable", pipe.as_raw_fd()), - )) - } - }) - .map(Self) +impl From for PipeReader { + fn from(owned_fd: OwnedFd) -> Self { + Self(convert_to_pipe(owned_fd)) } } #[unstable(feature = "anonymous_pipe", issue = "127154")] -impl TryFrom for PipeWriter { - type Error = io::Error; - - fn try_from(owned_fd: OwnedFd) -> Result { - convert_to_pipe(owned_fd) - .and_then(|pipe| { - if pipe.as_file_desc().get_access_mode()?.is_writable() { - Ok(pipe) - } else { - Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!("Pipe {} is not writable", pipe.as_raw_fd()), - )) - } - }) - .map(Self) +impl From for PipeWriter { + fn from(owned_fd: OwnedFd) -> Self { + Self(convert_to_pipe(owned_fd)) } } diff --git a/library/std/src/sys/anonymous_pipe/windows.rs b/library/std/src/sys/anonymous_pipe/windows.rs index f107258ff8ddb..592e2f05a9ebf 100644 --- a/library/std/src/sys/anonymous_pipe/windows.rs +++ b/library/std/src/sys/anonymous_pipe/windows.rs @@ -5,7 +5,6 @@ use crate::{ AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, }, sys::{ - c::{GetFileType, FILE_TYPE_PIPE}, handle::Handle, pipe::{anon_pipe, AnonPipe, Pipes}, }, @@ -95,28 +94,20 @@ impl From for Stdio { } } -fn convert_to_pipe(owned_handle: OwnedHandle) -> io::Result { - if unsafe { GetFileType(owned_handle.as_raw_handle()) } == FILE_TYPE_PIPE { - Ok(AnonPipe::from_inner(Handle::from_inner(owned_handle))) - } else { - Err(io::Error::new(io::ErrorKind::InvalidInput, "Not a pipe")) - } +fn convert_to_pipe(owned_handle: OwnedHandle) -> AnonPipe { + AnonPipe::from_inner(Handle::from_inner(owned_handle)) } #[unstable(feature = "anonymous_pipe", issue = "127154")] -impl TryFrom for PipeReader { - type Error = io::Error; - - fn try_from(owned_handle: OwnedHandle) -> Result { - convert_to_pipe(owned_handle).map(Self) +impl From for PipeReader { + fn from(owned_handle: OwnedHandle) -> Self { + Self(convert_to_pipe(owned_handle)) } } #[unstable(feature = "anonymous_pipe", issue = "127154")] -impl TryFrom for PipeWriter { - type Error = io::Error; - - fn try_from(owned_handle: OwnedHandle) -> Result { - convert_to_pipe(owned_handle).map(Self) +impl From for PipeWriter { + fn from(owned_handle: OwnedHandle) -> Self { + Self(convert_to_pipe(owned_handle)) } } diff --git a/library/std/src/sys/pal/unix/fd.rs b/library/std/src/sys/pal/unix/fd.rs index 2536dd845548d..1701717db597c 100644 --- a/library/std/src/sys/pal/unix/fd.rs +++ b/library/std/src/sys/pal/unix/fd.rs @@ -26,24 +26,6 @@ use libc::off64_t; )))] use libc::off_t as off64_t; -#[derive(Copy, Clone)] -pub enum AccessMode { - ReadOnly, - WriteOnly, - ReadWrite, - None, -} - -impl AccessMode { - pub fn is_readable(self) -> bool { - matches!(self, AccessMode::ReadOnly | AccessMode::ReadWrite) - } - - pub fn is_writable(self) -> bool { - matches!(self, AccessMode::WriteOnly | AccessMode::ReadWrite) - } -} - #[derive(Debug)] pub struct FileDesc(OwnedFd); @@ -536,30 +518,20 @@ impl FileDesc { } } - fn get_flags(&self) -> io::Result { - unsafe { cvt(libc::fcntl(self.as_raw_fd(), libc::F_GETFL)) } - } - - pub fn get_access_mode(&self) -> io::Result { - let access_mode = self.get_flags()? & libc::O_ACCMODE; - Ok(match access_mode { - libc::O_RDWR => AccessMode::ReadWrite, - libc::O_RDONLY => AccessMode::ReadOnly, - libc::O_WRONLY => AccessMode::WriteOnly, - - _ => AccessMode::None, - }) - } - #[cfg(not(target_os = "linux"))] pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { - let previous = self.get_flags()?; - let new = - if nonblocking { previous | libc::O_NONBLOCK } else { previous & !libc::O_NONBLOCK }; - if new != previous { - unsafe { cvt(libc::fcntl(self.as_raw_fd(), libc::F_SETFL, new)) }?; + unsafe { + let previous = cvt(libc::fcntl(self.as_raw_fd(), libc::F_GETFL))?; + let new = if nonblocking { + previous | libc::O_NONBLOCK + } else { + previous & !libc::O_NONBLOCK + }; + if new != previous { + cvt(libc::fcntl(self.as_raw_fd(), libc::F_SETFL, new))?; + } + Ok(()) } - Ok(()) } #[inline]