From d0c589d5ced5006f72d766af2ccecf699ff76176 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Mon, 23 Feb 2015 23:50:32 +0100 Subject: [PATCH 1/2] Hide unnecessary error checking from the user This affects the `set_non_blocking` function which cannot fail for Unix or Windows, given correct parameters. Additionally, the short UDP write error case has been removed as there is no such thing as "short UDP writes", instead, the operating system will error out if the application tries to send a packet larger than the MTU of the network path. --- src/libstd/sys/common/net.rs | 21 ++++++++------------- src/libstd/sys/unix/mod.rs | 4 ++-- src/libstd/sys/unix/pipe.rs | 6 +++--- src/libstd/sys/unix/process.rs | 6 +++--- src/libstd/sys/unix/tcp.rs | 6 +++--- src/libstd/sys/windows/mod.rs | 6 +++--- src/libstd/sys/windows/net.rs | 13 ++++++++++++- src/libstd/sys/windows/tcp.rs | 2 +- 8 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/libstd/sys/common/net.rs b/src/libstd/sys/common/net.rs index b5cd42219e128..228362e3d62ae 100644 --- a/src/libstd/sys/common/net.rs +++ b/src/libstd/sys/common/net.rs @@ -503,7 +503,7 @@ pub fn connect_timeout(fd: sock_t, #[cfg(windows)] use libc::WSAEWOULDBLOCK as WOULDBLOCK; // Make sure the call to connect() doesn't block - try!(set_nonblocking(fd, true)); + set_nonblocking(fd, true); let ret = match unsafe { libc::connect(fd, addrp, len) } { // If the connection is in progress, then we need to wait for it to @@ -533,7 +533,7 @@ pub fn connect_timeout(fd: sock_t, }; // be sure to turn blocking I/O back on - try!(set_nonblocking(fd, false)); + set_nonblocking(fd, false); return ret; #[cfg(unix)] @@ -626,7 +626,7 @@ pub struct Guard<'a> { #[unsafe_destructor] impl<'a> Drop for Guard<'a> { fn drop(&mut self) { - assert!(set_nonblocking(self.fd, false).is_ok()); + set_nonblocking(self.fd, false); } } @@ -723,7 +723,7 @@ impl TcpStream { fd: self.fd(), guard: self.inner.lock.lock().unwrap(), }; - assert!(set_nonblocking(self.fd(), true).is_ok()); + set_nonblocking(self.fd(), true); ret } @@ -862,7 +862,7 @@ impl UdpSocket { fd: self.fd(), guard: self.inner.lock.lock().unwrap(), }; - assert!(set_nonblocking(self.fd(), true).is_ok()); + set_nonblocking(self.fd(), true); ret } @@ -887,9 +887,7 @@ impl UdpSocket { storagep, &mut addrlen) as libc::c_int })); - sockaddr_to_addr(&storage, addrlen as uint).and_then(|addr| { - Ok((n as uint, addr)) - }) + Ok((n as uint, sockaddr_to_addr(&storage, addrlen as uint).unwrap())) } pub fn send_to(&mut self, buf: &[u8], dst: SocketAddr) -> IoResult<()> { @@ -910,11 +908,8 @@ impl UdpSocket { }; let n = try!(write(fd, self.write_deadline, buf, false, dolock, dowrite)); - if n != buf.len() { - Err(short_write(n, "couldn't send entire packet at once")) - } else { - Ok(()) - } + assert!(n == buf.len(), "UDP packet not completely written."); + Ok(()) } pub fn join_multicast(&mut self, multi: IpAddr) -> IoResult<()> { diff --git a/src/libstd/sys/unix/mod.rs b/src/libstd/sys/unix/mod.rs index b79ad7031fa48..632270bc5ccbf 100644 --- a/src/libstd/sys/unix/mod.rs +++ b/src/libstd/sys/unix/mod.rs @@ -214,9 +214,9 @@ pub fn wouldblock() -> bool { err == libc::EWOULDBLOCK as i32 || err == libc::EAGAIN as i32 } -pub fn set_nonblocking(fd: sock_t, nb: bool) -> IoResult<()> { +pub fn set_nonblocking(fd: sock_t, nb: bool) { let set = nb as libc::c_int; - mkerr_libc(retry(|| unsafe { c::ioctl(fd, c::FIONBIO, &set) })) + mkerr_libc(retry(|| unsafe { c::ioctl(fd, c::FIONBIO, &set) })).unwrap(); } // nothing needed on unix platforms diff --git a/src/libstd/sys/unix/pipe.rs b/src/libstd/sys/unix/pipe.rs index 3c9cdc65975f6..1446600a77ed7 100644 --- a/src/libstd/sys/unix/pipe.rs +++ b/src/libstd/sys/unix/pipe.rs @@ -235,9 +235,9 @@ impl UnixListener { _ => { let (reader, writer) = try!(unsafe { sys::os::pipe() }); - try!(set_nonblocking(reader.fd(), true)); - try!(set_nonblocking(writer.fd(), true)); - try!(set_nonblocking(self.fd(), true)); + set_nonblocking(reader.fd(), true); + set_nonblocking(writer.fd(), true); + set_nonblocking(self.fd(), true); Ok(UnixAcceptor { inner: Arc::new(AcceptorInner { listener: self, diff --git a/src/libstd/sys/unix/process.rs b/src/libstd/sys/unix/process.rs index 2be841989e6b3..dc067e7e0c5ba 100644 --- a/src/libstd/sys/unix/process.rs +++ b/src/libstd/sys/unix/process.rs @@ -345,8 +345,8 @@ impl Process { unsafe { let mut pipes = [0; 2]; assert_eq!(libc::pipe(pipes.as_mut_ptr()), 0); - set_nonblocking(pipes[0], true).ok().unwrap(); - set_nonblocking(pipes[1], true).ok().unwrap(); + set_nonblocking(pipes[0], true); + set_nonblocking(pipes[1], true); WRITE_FD = pipes[1]; let mut old: c::sigaction = mem::zeroed(); @@ -362,7 +362,7 @@ impl Process { fn waitpid_helper(input: libc::c_int, messages: Receiver, (read_fd, old): (libc::c_int, c::sigaction)) { - set_nonblocking(input, true).ok().unwrap(); + set_nonblocking(input, true); let mut set: c::fd_set = unsafe { mem::zeroed() }; let mut tv: libc::timeval; let mut active = Vec::<(libc::pid_t, Sender, u64)>::new(); diff --git a/src/libstd/sys/unix/tcp.rs b/src/libstd/sys/unix/tcp.rs index c8f9d318482b0..b08f6ef9b9032 100644 --- a/src/libstd/sys/unix/tcp.rs +++ b/src/libstd/sys/unix/tcp.rs @@ -67,9 +67,9 @@ impl TcpListener { -1 => Err(last_net_error()), _ => { let (reader, writer) = try!(unsafe { sys::os::pipe() }); - try!(set_nonblocking(reader.fd(), true)); - try!(set_nonblocking(writer.fd(), true)); - try!(set_nonblocking(self.fd(), true)); + set_nonblocking(reader.fd(), true); + set_nonblocking(writer.fd(), true); + set_nonblocking(self.fd(), true); Ok(TcpAcceptor { inner: Arc::new(AcceptorInner { listener: self, diff --git a/src/libstd/sys/windows/mod.rs b/src/libstd/sys/windows/mod.rs index a756fb29f81ae..a13c03c7448cb 100644 --- a/src/libstd/sys/windows/mod.rs +++ b/src/libstd/sys/windows/mod.rs @@ -192,13 +192,13 @@ pub fn wouldblock() -> bool { err == libc::WSAEWOULDBLOCK as i32 } -pub fn set_nonblocking(fd: sock_t, nb: bool) -> IoResult<()> { +pub fn set_nonblocking(fd: sock_t, nb: bool) { let mut set = nb as libc::c_ulong; - if unsafe { c::ioctlsocket(fd, c::FIONBIO, &mut set) != 0 } { + (if unsafe { c::ioctlsocket(fd, c::FIONBIO, &mut set) } != 0 { Err(last_error()) } else { Ok(()) - } + }).unwrap(); } pub fn init_net() { diff --git a/src/libstd/sys/windows/net.rs b/src/libstd/sys/windows/net.rs index 3451232f40ab8..6caa4df5dfe62 100644 --- a/src/libstd/sys/windows/net.rs +++ b/src/libstd/sys/windows/net.rs @@ -25,6 +25,8 @@ pub type wrlen_t = i32; pub struct Socket(libc::SOCKET); +/// Checks whether the Windows socket interface has been started already, and +/// if not, starts it. pub fn init() { static START: Once = ONCE_INIT; @@ -38,10 +40,16 @@ pub fn init() { }); } +/// Returns the last error from the Windows socket interface. fn last_error() -> io::Error { io::Error::from_os_error(unsafe { c::WSAGetLastError() }) } +/// Checks if the signed integer is the Windows constant `SOCKET_ERROR` (-1) +/// and if so, returns the last error from the Windows socket interface. . This +/// function must be called before another call to the socket API is made. +/// +/// FIXME: generics needed? pub fn cvt(t: T) -> io::Result { let one: T = Int::one(); if t == -one { @@ -51,11 +59,14 @@ pub fn cvt(t: T) -> io::Result { } } +/// Provides the functionality of `cvt` for the return values of `getaddrinfo` +/// and similar, meaning that they return an error if the return value is 0. pub fn cvt_gai(err: c_int) -> io::Result<()> { if err == 0 { return Ok(()) } cvt(err).map(|_| ()) } +/// Provides the functionality of `cvt` for a closure. pub fn cvt_r(mut f: F) -> io::Result where F: FnMut() -> T { cvt(f()) } @@ -112,7 +123,7 @@ impl Socket { impl Drop for Socket { fn drop(&mut self) { - unsafe { let _ = libc::closesocket(self.0); } + unsafe { cvt(libc::closesocket(self.0)).unwrap(); } } } diff --git a/src/libstd/sys/windows/tcp.rs b/src/libstd/sys/windows/tcp.rs index 4804ca510cb0b..25b70918591d0 100644 --- a/src/libstd/sys/windows/tcp.rs +++ b/src/libstd/sys/windows/tcp.rs @@ -192,7 +192,7 @@ impl TcpAcceptor { c::WSAEventSelect(socket, events[1], 0) }; if ret != 0 { return Err(last_net_error()) } - try!(set_nonblocking(socket, false)); + set_nonblocking(socket, false); return Ok(stream) } } From 0fc1a7da93e5d431f5cd54a3f1263e2a5f9e5748 Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Tue, 24 Feb 2015 17:07:13 +0100 Subject: [PATCH 2/2] Improve readability of an error check in `set_non_blocking` --- src/libstd/sys/windows/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libstd/sys/windows/mod.rs b/src/libstd/sys/windows/mod.rs index a13c03c7448cb..3acb372f658c5 100644 --- a/src/libstd/sys/windows/mod.rs +++ b/src/libstd/sys/windows/mod.rs @@ -194,11 +194,11 @@ pub fn wouldblock() -> bool { pub fn set_nonblocking(fd: sock_t, nb: bool) { let mut set = nb as libc::c_ulong; - (if unsafe { c::ioctlsocket(fd, c::FIONBIO, &mut set) } != 0 { - Err(last_error()) - } else { - Ok(()) - }).unwrap(); + if unsafe { c::ioctlsocket(fd, c::FIONBIO, &mut set) } != 0 { + // The above function should not return an error unless we passed it + // invalid parameters. Panic on errors. + Err(last_error()).unwrap(); + } } pub fn init_net() {