Refactor Socket::take_error

Author: Thomasdezeeuw

src/socket.rs

@@ -245,7 +245,7 @@ impl Socket {
     /// the field in the process. This can be useful for checking errors between
     /// calls.
     pub fn take_error(&self) -> io::Result<Option<io::Error>> {
-        self.inner().take_error()
+        sys::take_error(self.inner)
     }
 
     /// Moves this TCP stream into or out of nonblocking mode.

src/sys/unix.rs

@@ -9,7 +9,7 @@
 #[cfg(not(target_os = "redox"))]
 use std::io::{IoSlice, IoSliceMut};
 use std::io::{Read, Write};
-use std::mem::{self, size_of_val};
+use std::mem::{self, size_of_val, MaybeUninit};
 use std::net::Shutdown;
 use std::net::{self, Ipv4Addr, Ipv6Addr};
 #[cfg(feature = "all")]
@@ -349,6 +349,14 @@ pub(crate) fn try_clone(fd: SysSocket) -> io::Result<SysSocket> {
     syscall!(fcntl(fd, libc::F_DUPFD_CLOEXEC, 0))
 }
 
+pub(crate) fn take_error(fd: SysSocket) -> io::Result<Option<io::Error>> {
+    match unsafe { getsockopt::<c_int>(fd, libc::SOL_SOCKET, libc::SO_ERROR) } {
+        Ok(0) => Ok(None),
+        Ok(errno) => Ok(Some(io::Error::from_raw_os_error(errno))),
+        Err(err) => Err(err),
+    }
+}
+
 /// Unix only API.
 impl crate::Socket {
     /// Accept a new incoming connection from this listener.
@@ -423,6 +431,25 @@ fn fcntl_add(fd: SysSocket, flag: c_int) -> io::Result<()> {
     }
 }
 
+/// Caller must ensure `T` is the correct type for `opt` and `val`.
+unsafe fn getsockopt<T>(fd: SysSocket, opt: c_int, val: c_int) -> io::Result<T> {
+    let mut payload: MaybeUninit<T> = MaybeUninit::uninit();
+    let mut len = mem::size_of::<T>() as libc::socklen_t;
+    syscall!(getsockopt(
+        fd,
+        opt,
+        val,
+        payload.as_mut_ptr().cast(),
+        &mut len,
+    ))
+    .map(|_| {
+        debug_assert_eq!(len as usize, mem::size_of::<T>());
+        // Safety: `getsockopt` initialised `payload` for us.
+        payload.assume_init()
+    })
+}
+
+/// Caller must ensure `T` is the correct type for `opt` and `val`.
 #[cfg(all(feature = "all", target_vendor = "apple"))]
 unsafe fn setsockopt<T>(fd: SysSocket, opt: c_int, val: c_int, payload: T) -> io::Result<()>
 where
@@ -435,8 +462,8 @@ where
         val,
         payload,
         mem::size_of::<T>() as libc::socklen_t,
-    ))?;
-    Ok(())
+    ))
+    .map(|_| ())
 }
 
 #[repr(transparent)] // Required during rewriting.
@@ -445,17 +472,6 @@ pub struct Socket {
 }
 
 impl Socket {
-    pub fn take_error(&self) -> io::Result<Option<io::Error>> {
-        unsafe {
-            let raw: c_int = self.getsockopt(libc::SOL_SOCKET, libc::SO_ERROR)?;
-            if raw == 0 {
-                Ok(None)
-            } else {
-                Ok(Some(io::Error::from_raw_os_error(raw as i32)))
-            }
-        }
-    }
-
     pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> {
         let previous = syscall!(fcntl(self.fd, libc::F_GETFL))?;
         let new = if nonblocking {

src/sys/windows.rs

@@ -72,6 +72,7 @@ pub(crate) use winapi::um::ws2tcpip::socklen_t;
 /// Helper macro to execute a system call that returns an `io::Result`.
 macro_rules! syscall {
     ($fn: ident ( $($arg: expr),* $(,)* ), $err_test: path, $err_value: expr) => {{
+        #[allow(unused_unsafe)]
         let res = unsafe { sock::$fn($($arg, )*) };
         if $err_test(&res, &$err_value) {
             Err(io::Error::last_os_error())
@@ -252,6 +253,29 @@ pub(crate) fn try_clone(socket: SysSocket) -> io::Result<SysSocket> {
     )
 }
 
+pub(crate) fn take_error(socket: SysSocket) -> io::Result<Option<io::Error>> {
+    match unsafe { getsockopt::<c_int>(socket, SOL_SOCKET, SO_ERROR) } {
+        Ok(0) => Ok(None),
+        Ok(errno) => Ok(Some(io::Error::from_raw_os_error(errno))),
+        Err(err) => Err(err),
+    }
+}
+
+unsafe fn getsockopt<T>(socket: SysSocket, opt: c_int, val: c_int) -> io::Result<T> {
+    let mut payload: MaybeUninit<T> = MaybeUninit::uninit();
+    let mut len = mem::size_of::<T>() as c_int;
+    syscall!(
+        getsockopt(socket, opt, val, payload.as_mut_ptr().cast(), &mut len,),
+        PartialEq::eq,
+        sock::SOCKET_ERROR
+    )
+    .map(|_| {
+        debug_assert_eq!(len as usize, mem::size_of::<T>());
+        // Safety: `getsockopt` initialised `payload` for us.
+        payload.assume_init()
+    })
+}
+
 /// Windows only API.
 impl crate::Socket {
     /// Sets `HANDLE_FLAG_INHERIT` to zero using `SetHandleInformation`.
@@ -276,17 +300,6 @@ pub struct Socket {
 }
 
 impl Socket {
-    pub fn take_error(&self) -> io::Result<Option<io::Error>> {
-        unsafe {
-            let raw: c_int = self.getsockopt(SOL_SOCKET, SO_ERROR)?;
-            if raw == 0 {
-                Ok(None)
-            } else {
-                Ok(Some(io::Error::from_raw_os_error(raw as i32)))
-            }
-        }
-    }
-
     pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> {
         unsafe {
             let mut nonblocking = nonblocking as c_ulong;