From a32244b3d9a52e88e75b1540558370b532bff985 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Thu, 23 Jun 2016 13:57:55 +0200 Subject: [PATCH 1/2] Don't ignore errors of syscalls in std::sys::unix::fd If any of these syscalls fail, it indicates a programmer error that should not be silently ignored. --- src/libstd/sys/unix/fd.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstd/sys/unix/fd.rs b/src/libstd/sys/unix/fd.rs index 94c48be02ffc4..bbc9531d1dac1 100644 --- a/src/libstd/sys/unix/fd.rs +++ b/src/libstd/sys/unix/fd.rs @@ -65,7 +65,7 @@ impl FileDesc { pub fn set_cloexec(&self) { unsafe { let ret = libc::ioctl(self.fd, libc::FIOCLEX); - debug_assert_eq!(ret, 0); + assert_eq!(ret, 0); } } #[cfg(any(target_env = "newlib", target_os = "solaris", target_os = "emscripten"))] @@ -73,21 +73,21 @@ impl FileDesc { unsafe { let previous = libc::fcntl(self.fd, libc::F_GETFD); let ret = libc::fcntl(self.fd, libc::F_SETFD, previous | libc::FD_CLOEXEC); - debug_assert_eq!(ret, 0); + assert_eq!(ret, 0); } } pub fn set_nonblocking(&self, nonblocking: bool) { unsafe { let previous = libc::fcntl(self.fd, libc::F_GETFL); - debug_assert!(previous != -1); + assert!(previous != -1); let new = if nonblocking { previous | libc::O_NONBLOCK } else { previous & !libc::O_NONBLOCK }; let ret = libc::fcntl(self.fd, libc::F_SETFL, new); - debug_assert!(ret != -1); + assert!(ret != -1); } } From 9347ffcf5c065b3d1aaac8c06c5c34d98bef96b9 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Fri, 24 Jun 2016 11:31:58 +0200 Subject: [PATCH 2/2] Bubble up the errors in `set_nonblocking` and `set_cloexec` --- src/libstd/sys/unix/fd.rs | 31 +++++++++++++++---------------- src/libstd/sys/unix/fs.rs | 2 +- src/libstd/sys/unix/net.rs | 8 ++++---- src/libstd/sys/unix/pipe.rs | 19 ++++++++++--------- 4 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/libstd/sys/unix/fd.rs b/src/libstd/sys/unix/fd.rs index bbc9531d1dac1..b99f4a2eacde5 100644 --- a/src/libstd/sys/unix/fd.rs +++ b/src/libstd/sys/unix/fd.rs @@ -62,32 +62,31 @@ impl FileDesc { } #[cfg(not(any(target_env = "newlib", target_os = "solaris", target_os = "emscripten")))] - pub fn set_cloexec(&self) { + pub fn set_cloexec(&self) -> io::Result<()> { unsafe { - let ret = libc::ioctl(self.fd, libc::FIOCLEX); - assert_eq!(ret, 0); + cvt(libc::ioctl(self.fd, libc::FIOCLEX))?; + Ok(()) } } #[cfg(any(target_env = "newlib", target_os = "solaris", target_os = "emscripten"))] - pub fn set_cloexec(&self) { + pub fn set_cloexec(&self) -> io::Result<()> { unsafe { - let previous = libc::fcntl(self.fd, libc::F_GETFD); - let ret = libc::fcntl(self.fd, libc::F_SETFD, previous | libc::FD_CLOEXEC); - assert_eq!(ret, 0); + let previous = cvt(libc::fcntl(self.fd, libc::F_GETFD))?; + cvt(libc::fcntl(self.fd, libc::F_SETFD, previous | libc::FD_CLOEXEC))?; + Ok(()) } } - pub fn set_nonblocking(&self, nonblocking: bool) { + pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { unsafe { - let previous = libc::fcntl(self.fd, libc::F_GETFL); - assert!(previous != -1); + let previous = cvt(libc::fcntl(self.fd, libc::F_GETFL))?; let new = if nonblocking { previous | libc::O_NONBLOCK } else { previous & !libc::O_NONBLOCK }; - let ret = libc::fcntl(self.fd, libc::F_SETFL, new); - assert!(ret != -1); + cvt(libc::fcntl(self.fd, libc::F_SETFL, new))?; + Ok(()) } } @@ -114,8 +113,8 @@ impl FileDesc { let make_filedesc = |fd| { let fd = FileDesc::new(fd); - fd.set_cloexec(); - fd + fd.set_cloexec()?; + Ok(fd) }; static TRY_CLOEXEC: AtomicBool = AtomicBool::new(!cfg!(target_os = "android")); @@ -127,7 +126,7 @@ impl FileDesc { // though it reported doing so on F_DUPFD_CLOEXEC. Ok(fd) => { return Ok(if cfg!(target_os = "linux") { - make_filedesc(fd) + make_filedesc(fd)? } else { FileDesc::new(fd) }) @@ -138,7 +137,7 @@ impl FileDesc { Err(e) => return Err(e), } } - cvt(unsafe { libc::fcntl(fd, libc::F_DUPFD, 0) }).map(make_filedesc) + cvt(unsafe { libc::fcntl(fd, libc::F_DUPFD, 0) }).and_then(make_filedesc) } } diff --git a/src/libstd/sys/unix/fs.rs b/src/libstd/sys/unix/fs.rs index 7f23ae53fcd1a..0524851df91ab 100644 --- a/src/libstd/sys/unix/fs.rs +++ b/src/libstd/sys/unix/fs.rs @@ -418,7 +418,7 @@ impl File { // The CLOEXEC flag, however, is supported on versions of OSX/BSD/etc // that we support, so we only do this on Linux currently. if cfg!(target_os = "linux") { - fd.set_cloexec(); + fd.set_cloexec()?; } Ok(File(fd)) diff --git a/src/libstd/sys/unix/net.rs b/src/libstd/sys/unix/net.rs index 830957a7e59c7..a784741c88cc7 100644 --- a/src/libstd/sys/unix/net.rs +++ b/src/libstd/sys/unix/net.rs @@ -77,7 +77,7 @@ impl Socket { let fd = cvt(libc::socket(fam, ty, 0))?; let fd = FileDesc::new(fd); - fd.set_cloexec(); + fd.set_cloexec()?; Ok(Socket(fd)) } } @@ -99,9 +99,9 @@ impl Socket { cvt(libc::socketpair(fam, ty, 0, fds.as_mut_ptr()))?; let a = FileDesc::new(fds[0]); - a.set_cloexec(); let b = FileDesc::new(fds[1]); - b.set_cloexec(); + a.set_cloexec()?; + b.set_cloexec()?; Ok((Socket(a), Socket(b))) } } @@ -132,7 +132,7 @@ impl Socket { libc::accept(self.0.raw(), storage, len) })?; let fd = FileDesc::new(fd); - fd.set_cloexec(); + fd.set_cloexec()?; Ok(Socket(fd)) } diff --git a/src/libstd/sys/unix/pipe.rs b/src/libstd/sys/unix/pipe.rs index beca2d467536d..2dde9c0e615f2 100644 --- a/src/libstd/sys/unix/pipe.rs +++ b/src/libstd/sys/unix/pipe.rs @@ -44,17 +44,18 @@ pub fn anon_pipe() -> io::Result<(AnonPipe, AnonPipe)> { } } if unsafe { libc::pipe(fds.as_mut_ptr()) == 0 } { - Ok((AnonPipe::from_fd(fds[0]), AnonPipe::from_fd(fds[1]))) + 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()) } } impl AnonPipe { - pub fn from_fd(fd: libc::c_int) -> AnonPipe { - let fd = FileDesc::new(fd); - fd.set_cloexec(); - AnonPipe(fd) + pub fn from_fd(fd: FileDesc) -> io::Result { + fd.set_cloexec()?; + Ok(AnonPipe(fd)) } pub fn read(&self, buf: &mut [u8]) -> io::Result { @@ -81,8 +82,8 @@ pub fn read2(p1: AnonPipe, // in the `select` loop below, and we wouldn't want one to block the other! let p1 = p1.into_fd(); let p2 = p2.into_fd(); - p1.set_nonblocking(true); - p2.set_nonblocking(true); + p1.set_nonblocking(true)?; + p2.set_nonblocking(true)?; let max = cmp::max(p1.raw(), p2.raw()); loop { @@ -114,11 +115,11 @@ pub fn read2(p1: AnonPipe, } }; if read(&p1, v1)? { - p2.set_nonblocking(false); + p2.set_nonblocking(false)?; return p2.read_to_end(v2).map(|_| ()); } if read(&p2, v2)? { - p1.set_nonblocking(false); + p1.set_nonblocking(false)?; return p1.read_to_end(v1).map(|_| ()); } }