From 0fff73b64a9658c9a3aa228488964d09e63115b4 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 4 Feb 2016 13:22:51 -0800 Subject: [PATCH] std: When duplicating fds, skip extra set_cloexec Similar to the previous commit, if `F_DUPFD_CLOEXEC` succeeds then there's no need for us to then call `set_cloexec` on platforms other than Linux. The bug mentioned of kernels not actually setting the `CLOEXEC` flag has only been repored on Linux, not elsewhere. --- src/libstd/sys/unix/fd.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/libstd/sys/unix/fd.rs b/src/libstd/sys/unix/fd.rs index d5f03764be2..0eadee54e26 100644 --- a/src/libstd/sys/unix/fd.rs +++ b/src/libstd/sys/unix/fd.rs @@ -77,9 +77,7 @@ impl FileDesc { // follow a strategy similar to musl [1] where if passing // F_DUPFD_CLOEXEC causes `fcntl` to return EINVAL it means it's not // supported (the third parameter, 0, is always valid), so we stop - // trying that. We also *still* call the `set_cloexec` method as - // apparently some kernel at some point stopped setting CLOEXEC even - // though it reported doing so on F_DUPFD_CLOEXEC. + // trying that. // // Also note that Android doesn't have F_DUPFD_CLOEXEC, but get it to // resolve so we at least compile this. @@ -95,14 +93,25 @@ impl FileDesc { fd.set_cloexec(); fd }; - static TRY_CLOEXEC: AtomicBool = AtomicBool::new(true); + static TRY_CLOEXEC: AtomicBool = + AtomicBool::new(!cfg!(target_os = "android")); let fd = self.raw(); - if !cfg!(target_os = "android") && TRY_CLOEXEC.load(Ordering::Relaxed) { + if TRY_CLOEXEC.load(Ordering::Relaxed) { match cvt(unsafe { libc::fcntl(fd, F_DUPFD_CLOEXEC, 0) }) { + // We *still* call the `set_cloexec` method as apparently some + // linux kernel at some point stopped setting CLOEXEC even + // though it reported doing so on F_DUPFD_CLOEXEC. + Ok(fd) => { + return Ok(if cfg!(target_os = "linux") { + make_filedesc(fd) + } else { + FileDesc::new(fd) + }) + } Err(ref e) if e.raw_os_error() == Some(libc::EINVAL) => { TRY_CLOEXEC.store(false, Ordering::Relaxed); } - res => return res.map(make_filedesc), + Err(e) => return Err(e), } } cvt(unsafe { libc::fcntl(fd, libc::F_DUPFD, 0) }).map(make_filedesc)