From c31d5b504144c4c542c3042b850cfc3b1066aa38 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Fri, 27 Jan 2017 19:45:18 +0100 Subject: [PATCH 1/2] Use less syscalls in `anon_pipe()` Save a `ENOSYS` failure from `pipe2` and don't try again. Use `cvt` instead of `cvt_r` for `pipe2` - `EINTR` is not an error `pipe2` can return. --- src/libstd/sys/unix/pipe.rs | 38 ++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/libstd/sys/unix/pipe.rs b/src/libstd/sys/unix/pipe.rs index a8ed415b7f47f..a5d60c257ed63 100644 --- a/src/libstd/sys/unix/pipe.rs +++ b/src/libstd/sys/unix/pipe.rs @@ -13,7 +13,8 @@ use io; use libc::{self, c_int}; use mem; use ptr; -use sys::cvt_r; +use sync::atomic::{AtomicBool, Ordering}; +use sys::{cvt, cvt_r}; use sys::fd::FileDesc; //////////////////////////////////////////////////////////////////////////////// @@ -29,34 +30,33 @@ pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> { // CLOEXEC flag is to use the `pipe2` syscall on Linux. This was added in // 2.6.27, however, and because we support 2.6.18 we must detect this // support dynamically. - if cfg!(target_os = "linux") { + static TRY_PIPE2: AtomicBool = AtomicBool::new(cfg!(target_os = "linux")); + if TRY_PIPE2.load(Ordering::Relaxed) { weak! { fn pipe2(*mut c_int, c_int) -> c_int } if let Some(pipe) = pipe2.get() { - match cvt_r(|| unsafe { pipe(fds.as_mut_ptr(), libc::O_CLOEXEC) }) { - Ok(_) => { + match cvt(unsafe { pipe(fds.as_mut_ptr(), libc::O_CLOEXEC) }) { + Err(ref e) if e.raw_os_error() == Some(libc::ENOSYS) => { + TRY_PIPE2.store(false, Ordering::Relaxed); + // Fall through + }, + res => { + res?; return Ok((AnonPipe(FileDesc::new(fds[0])), - AnonPipe(FileDesc::new(fds[1])))) + AnonPipe(FileDesc::new(fds[1])))); } - Err(ref e) if e.raw_os_error() == Some(libc::ENOSYS) => {} - Err(e) => return Err(e), } } } - if unsafe { libc::pipe(fds.as_mut_ptr()) == 0 } { - let fd0 = FileDesc::new(fds[0]); - let fd1 = FileDesc::new(fds[1]); - Ok((AnonPipe::from_fd(fd0)?, AnonPipe::from_fd(fd1)?)) - } else { - Err(io::Error::last_os_error()) - } + cvt(unsafe { libc::pipe(fds.as_mut_ptr()) })?; + + let fd0 = FileDesc::new(fds[0]); + let fd1 = FileDesc::new(fds[1]); + fd0.set_cloexec()?; + fd1.set_cloexec()?; + Ok((AnonPipe(fd0), AnonPipe(fd1))) } impl AnonPipe { - pub fn from_fd(fd: FileDesc) -> io::Result { - fd.set_cloexec()?; - Ok(AnonPipe(fd)) - } - pub fn read(&self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } From 4b46d2a3a222f090b07b019df0e9346b08c40ae1 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Mon, 30 Jan 2017 17:37:49 +0100 Subject: [PATCH 2/2] Don't handle ENOSYS in `anon_pipe()` We're not calling the raw syscall but a libc function, the libc will have a compatibility layer. --- src/libstd/sys/unix/pipe.rs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/libstd/sys/unix/pipe.rs b/src/libstd/sys/unix/pipe.rs index a5d60c257ed63..51e00fc1ab96a 100644 --- a/src/libstd/sys/unix/pipe.rs +++ b/src/libstd/sys/unix/pipe.rs @@ -13,7 +13,6 @@ use io; use libc::{self, c_int}; use mem; use ptr; -use sync::atomic::{AtomicBool, Ordering}; use sys::{cvt, cvt_r}; use sys::fd::FileDesc; @@ -30,21 +29,17 @@ pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> { // CLOEXEC flag is to use the `pipe2` syscall on Linux. This was added in // 2.6.27, however, and because we support 2.6.18 we must detect this // support dynamically. - static TRY_PIPE2: AtomicBool = AtomicBool::new(cfg!(target_os = "linux")); - if TRY_PIPE2.load(Ordering::Relaxed) { + if cfg!(any(target_os = "dragonfly", + target_os = "freebsd", + target_os = "linux", + target_os = "netbsd", + target_os = "openbsd")) + { weak! { fn pipe2(*mut c_int, c_int) -> c_int } if let Some(pipe) = pipe2.get() { - match cvt(unsafe { pipe(fds.as_mut_ptr(), libc::O_CLOEXEC) }) { - Err(ref e) if e.raw_os_error() == Some(libc::ENOSYS) => { - TRY_PIPE2.store(false, Ordering::Relaxed); - // Fall through - }, - res => { - res?; - return Ok((AnonPipe(FileDesc::new(fds[0])), - AnonPipe(FileDesc::new(fds[1])))); - } - } + cvt(unsafe { pipe(fds.as_mut_ptr(), libc::O_CLOEXEC) })?; + return Ok((AnonPipe(FileDesc::new(fds[0])), + AnonPipe(FileDesc::new(fds[1])))); } } cvt(unsafe { libc::pipe(fds.as_mut_ptr()) })?;