diff --git a/examples/dup2_to_replace_stdio.rs b/examples/dup2_to_replace_stdio.rs index 2f0f4bbed..cc8565915 100644 --- a/examples/dup2_to_replace_stdio.rs +++ b/examples/dup2_to_replace_stdio.rs @@ -16,15 +16,16 @@ fn main() { // that stdin and stdout will be open or safe to use. It's ok here, because // we're directly inside `main`, so we know that stdin and stdout haven't // been closed and aren't being used for other purposes. - let (stdin, stdout) = unsafe { (rustix::io::take_stdin(), rustix::io::take_stdout()) }; + let (mut stdin, mut stdout) = unsafe { (rustix::io::take_stdin(), rustix::io::take_stdout()) }; // Use `dup2` to copy our new file descriptors over the stdio file descriptors. // - // These take their second argument as an `&OwnedFd` rather than the usual - // `impl AsFd` because they conceptually do a `close` on the original file - // descriptor, which one shouldn't be able to do with just a `BorrowedFd`. - dup2(&reader, &stdin).unwrap(); - dup2(&writer, &stdout).unwrap(); + // These take their second argument as an `&mut OwnedFd` rather than the + // usual `impl AsFd` because they conceptually do a `close` on the original + // file descriptor, which one shouldn't be able to do with just a + // `BorrowedFd`. + dup2(&reader, &mut stdin).unwrap(); + dup2(&writer, &mut stdout).unwrap(); // Then, forget the stdio `OwnedFd`s, because actually dropping them would // close them. Here, we want stdin and stdout to remain open for the rest diff --git a/src/imp/libc/io/syscalls.rs b/src/imp/libc/io/syscalls.rs index 0cbedf52a..c7754178b 100644 --- a/src/imp/libc/io/syscalls.rs +++ b/src/imp/libc/io/syscalls.rs @@ -375,7 +375,7 @@ pub(crate) fn dup(fd: BorrowedFd<'_>) -> io::Result { } #[cfg(not(target_os = "wasi"))] -pub(crate) fn dup2(fd: BorrowedFd<'_>, new: &OwnedFd) -> io::Result<()> { +pub(crate) fn dup2(fd: BorrowedFd<'_>, new: &mut OwnedFd) -> io::Result<()> { unsafe { ret_discarded_fd(c::dup2(borrowed_fd(fd), borrowed_fd(new.as_fd()))) } } @@ -387,7 +387,7 @@ pub(crate) fn dup2(fd: BorrowedFd<'_>, new: &OwnedFd) -> io::Result<()> { target_os = "redox", target_os = "wasi" )))] -pub(crate) fn dup3(fd: BorrowedFd<'_>, new: &OwnedFd, flags: DupFlags) -> io::Result<()> { +pub(crate) fn dup3(fd: BorrowedFd<'_>, new: &mut OwnedFd, flags: DupFlags) -> io::Result<()> { unsafe { ret_discarded_fd(c::dup3( borrowed_fd(fd), @@ -404,14 +404,11 @@ pub(crate) fn dup3(fd: BorrowedFd<'_>, new: &OwnedFd, flags: DupFlags) -> io::Re target_os = "ios", target_os = "redox" ))] -pub(crate) fn dup3(fd: BorrowedFd<'_>, new: &OwnedFd, _flags: DupFlags) -> io::Result<()> { +pub(crate) fn dup3(fd: BorrowedFd<'_>, new: &mut OwnedFd, _flags: DupFlags) -> io::Result<()> { // Android 5.0 has `dup3`, but libc doesn't have bindings. Emulate it - // using `dup2`, including the difference of failing with `EINVAL` - // when the file descriptors are equal. - use std::os::unix::io::AsRawFd; - if fd.as_raw_fd() == new.as_raw_fd() { - return Err(io::Errno::INVAL); - } + // using `dup2`. We don't need to worry about the difference between + // `dup2` and `dup3` when the file descriptors are equal because we + // have an `&mut OwnedFd` which means `fd` doesn't alias it. dup2(fd, new) } diff --git a/src/imp/linux_raw/io/syscalls.rs b/src/imp/linux_raw/io/syscalls.rs index 6af2bb5f8..543c562b1 100644 --- a/src/imp/linux_raw/io/syscalls.rs +++ b/src/imp/linux_raw/io/syscalls.rs @@ -394,15 +394,12 @@ pub(crate) fn dup(fd: BorrowedFd<'_>) -> io::Result { } #[inline] -pub(crate) fn dup2(fd: BorrowedFd<'_>, new: &OwnedFd) -> io::Result<()> { +pub(crate) fn dup2(fd: BorrowedFd<'_>, new: &mut OwnedFd) -> io::Result<()> { #[cfg(any(target_arch = "aarch64", target_arch = "riscv64"))] { - // `dup3` fails if the old and new file descriptors have the same - // value, so emulate the `dup2` behavior. - use crate::fd::AsRawFd; - if fd.as_raw_fd() == new.as_raw_fd() { - return Ok(()); - } + // We don't need to worry about the difference between `dup2` and + // `dup3` when the file descriptors are equal because we have an + // `&mut OwnedFd` which means `fd` doesn't alias it. dup3(fd, new, DupFlags::empty()) } @@ -413,7 +410,7 @@ pub(crate) fn dup2(fd: BorrowedFd<'_>, new: &OwnedFd) -> io::Result<()> { } #[inline] -pub(crate) fn dup3(fd: BorrowedFd<'_>, new: &OwnedFd, flags: DupFlags) -> io::Result<()> { +pub(crate) fn dup3(fd: BorrowedFd<'_>, new: &mut OwnedFd, flags: DupFlags) -> io::Result<()> { unsafe { ret_discarded_fd(syscall_readonly!(__NR_dup3, fd, new.as_fd(), flags)) } } diff --git a/src/io/dup.rs b/src/io/dup.rs index 9e6801311..aa6f8ee51 100644 --- a/src/io/dup.rs +++ b/src/io/dup.rs @@ -31,13 +31,16 @@ pub fn dup(fd: Fd) -> io::Result { imp::io::syscalls::dup(fd.as_fd()) } -/// `dup2(fd, new)`—Creates a new `OwnedFd` instance that shares the -/// same underlying [file description] as the existing `OwnedFd` instance, -/// closing `new` and reusing its file descriptor. +/// `dup2(fd, new)`—Changes the [file description] of a file descriptor. +/// +/// `dup2` conceptually closes `new` and then sets the file description for +/// `new` to be the same as the one for `fd`. This is a very unusual operation, +/// and should only be used on file descriptors where you know how `new` will +/// be subsequently used. /// /// This function does not set the `O_CLOEXEC` flag. To do a `dup2` that does /// set `O_CLOEXEC`, use [`dup3`] with [`DupFlags::CLOEXEC`] on platforms which -/// support it. +/// support it, or [`fcntl_dupfd_cloexec`] /// /// # References /// - [POSIX] @@ -46,15 +49,15 @@ pub fn dup(fd: Fd) -> io::Result { /// [file description]: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_258 /// [POSIX]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/dup2.html /// [Linux]: https://man7.org/linux/man-pages/man2/dup2.2.html +/// [`fcntl_dupfd_cloexec`]: crate::fs::fcntl_dupfd_cloexec #[cfg(not(target_os = "wasi"))] #[inline] -pub fn dup2(fd: Fd, new: &OwnedFd) -> io::Result<()> { +pub fn dup2(fd: Fd, new: &mut OwnedFd) -> io::Result<()> { imp::io::syscalls::dup2(fd.as_fd(), new) } -/// `dup3(fd, new, flags)`—Creates a new `OwnedFd` instance that shares the -/// same underlying [file description] as the existing `OwnedFd` instance, -/// closing `new` and reusing its file descriptor, with flags. +/// `dup3(fd, new, flags)`—Changes the [file description] of a file +/// descriptor, with flags. /// /// `dup3` is the same as [`dup2`] but adds an additional flags operand, /// and it fails in the case that `fd` and `new` have the same file descriptor @@ -70,6 +73,6 @@ pub fn dup2(fd: Fd, new: &OwnedFd) -> io::Result<()> { /// [Linux]: https://man7.org/linux/man-pages/man2/dup2.2.html #[cfg(not(target_os = "wasi"))] #[inline] -pub fn dup3(fd: Fd, new: &OwnedFd, flags: DupFlags) -> io::Result<()> { +pub fn dup3(fd: Fd, new: &mut OwnedFd, flags: DupFlags) -> io::Result<()> { imp::io::syscalls::dup3(fd.as_fd(), new, flags) } diff --git a/tests/io/dup3.rs b/tests/io/dup3.rs deleted file mode 100644 index 51672f3fc..000000000 --- a/tests/io/dup3.rs +++ /dev/null @@ -1,41 +0,0 @@ -use rustix::io::DupFlags; - -/// `dup2` uses POSIX `dup2` which silently does nothing if the file -/// descriptors are equal. -#[test] -fn test_dup2() { - let (a, b) = rustix::io::pipe().unwrap(); - rustix::io::dup2(&a, &a).unwrap(); - rustix::io::dup2(&b, &b).unwrap(); -} - -/// `dup3` uses Linux `dup3` which fails with `INVAL` if the -/// file descriptors are equal. -#[test] -fn test_dup3() { - let (a, b) = rustix::io::pipe().unwrap(); - assert_eq!( - rustix::io::dup3(&a, &a, DupFlags::empty()), - Err(rustix::io::Errno::INVAL) - ); - assert_eq!( - rustix::io::dup3(&b, &b, DupFlags::empty()), - Err(rustix::io::Errno::INVAL) - ); - #[cfg(not(any( - target_os = "android", - target_os = "ios", - target_os = "macos", - target_os = "redox" - )))] // Android 5.0 has dup3, but libc doesn't have bindings - { - assert_eq!( - rustix::io::dup3(&a, &a, DupFlags::CLOEXEC), - Err(rustix::io::Errno::INVAL) - ); - assert_eq!( - rustix::io::dup3(&b, &b, DupFlags::CLOEXEC), - Err(rustix::io::Errno::INVAL) - ); - } -} diff --git a/tests/io/main.rs b/tests/io/main.rs index f4ce1b90a..6e8b0e461 100644 --- a/tests/io/main.rs +++ b/tests/io/main.rs @@ -7,9 +7,6 @@ #[cfg(not(windows))] #[cfg(not(target_os = "wasi"))] mod dup2_to_replace_stdio; -#[cfg(not(windows))] -#[cfg(not(target_os = "wasi"))] -mod dup3; #[cfg(not(feature = "rustc-dep-of-std"))] // TODO #[cfg(not(windows))] #[cfg(feature = "net")]