From 1df57ecd0d10b69469ca04557bdf5f576943f8d6 Mon Sep 17 00:00:00 2001
From: Alan Somers <asomers@gmail.com>
Date: Sun, 6 Jun 2021 10:43:02 -0600
Subject: [PATCH] WIP Overhauling Nix's error types

For many of Nix's consumers it be convenient to easily convert a Nix
error into a std::io::Error.  That's currently not possible because of
the InvalidPath, InvalidUtf8, and UnsupportedOperation types that have
no equivalent in std::io::Error.

However, very few of Nix's public APIs actually return those unusual
errors.  So a more useful API would be for Nix's standard error type to
implement Into<std::io::Error>, and the few functions that must return
unusual errors like InvalidUtf8 should use bespoke error types.

This commit prototypes that approach by modifying just one function, for
now, to use the new error type.
---
 src/errno.rs  | 10 ++++++++
 src/lib.rs    | 67 +++++++++++++++++++++++++++++++++++++++++++++++++--
 src/unistd.rs |  6 ++---
 3 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/src/errno.rs b/src/errno.rs
index 9275febed1..ac3e61ea5c 100644
--- a/src/errno.rs
+++ b/src/errno.rs
@@ -64,6 +64,16 @@ impl Errno {
         clear()
     }
 
+    pub(crate) fn result2<S: ErrnoSentinel + PartialEq<S>>(value: S)
+        -> std::result::Result<S, Self>
+    {
+        if value == S::sentinel() {
+            Err(Self::last())
+        } else {
+            Ok(value)
+        }
+    }
+
     /// Returns `Ok(value)` if it does not contain the sentinel value. This
     /// should not be used when `-1` is not the errno sentinel value.
     pub fn result<S: ErrnoSentinel + PartialEq<S>>(value: S) -> Result<S> {
diff --git a/src/lib.rs b/src/lib.rs
index 899d3f8bd2..5144818ac8 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -78,14 +78,16 @@ pub mod unistd;
 
 use libc::{c_char, PATH_MAX};
 
-use std::{error, fmt, ptr, result};
+use std::convert::TryFrom;
+use std::{error, fmt, io, ptr, result};
 use std::ffi::{CStr, OsStr};
 use std::os::unix::ffi::OsStrExt;
 use std::path::{Path, PathBuf};
 
-use errno::Errno;
+use errno::{Errno, ErrnoSentinel};
 
 /// Nix Result Type
+// TODO: change Error to SysError
 pub type Result<T> = result::Result<T, Error>;
 
 /// Nix Error Type
@@ -107,6 +109,67 @@ pub enum Error {
     UnsupportedOperation,
 }
 
+/// Nix's main error type.
+///
+/// It's a wrapper around Errno.  As such, it's very interoperable with
+/// [`std::io::Error`], but it has the advantages of:
+/// * `Clone`
+/// * `Copy`
+/// * `Eq`
+/// * Small size
+/// * Represents all of the system's errnos, instead of just the most common
+/// ones.
+// TODO: rename to "Error"
+#[derive(Clone, Copy, Debug, Eq, PartialEq)]
+pub struct SysError(Errno);
+
+impl SysError {
+    /// Returns `Ok(value)` if it does not contain the sentinel value. This
+    /// should not be used when `-1` is not the errno sentinel value.
+    pub(crate) fn result<S: ErrnoSentinel + PartialEq<S>>(value: S)
+        -> std::result::Result<S, SysError>
+    {
+        Errno::result2(value).map_err(Self::from)
+    }
+}
+
+impl fmt::Display for SysError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{:?}: {}", self.0, self.0.desc())
+    }
+}
+
+impl error::Error for SysError {}
+
+impl From<Errno> for SysError {
+    fn from(errno: Errno) -> Self {
+        Self(errno)
+    }
+}
+
+impl From<SysError> for Errno {
+    fn from(error: SysError) -> Self {
+        error.0
+    }
+}
+
+impl TryFrom<io::Error> for SysError {
+    type Error = io::Error;
+
+    fn try_from(ioerror: io::Error) -> std::result::Result<Self, io::Error> {
+        ioerror.raw_os_error()
+            .map(Errno::from_i32)
+            .map(SysError::from)
+            .ok_or(ioerror)
+    }
+}
+
+impl From<SysError> for io::Error {
+    fn from(error: SysError) -> Self {
+        Self::from_raw_os_error(error.0 as i32)
+    }
+}
+
 impl Error {
     /// Convert this `Error` to an [`Errno`](enum.Errno.html).
     ///
diff --git a/src/unistd.rs b/src/unistd.rs
index d406efe87c..972e981934 100644
--- a/src/unistd.rs
+++ b/src/unistd.rs
@@ -3,7 +3,7 @@
 #[cfg(not(target_os = "redox"))]
 use cfg_if::cfg_if;
 use crate::errno::{self, Errno};
-use crate::{Error, Result, NixPath};
+use crate::{Error, Result, NixPath, SysError};
 #[cfg(not(target_os = "redox"))]
 use crate::fcntl::{AtFlags, at_rawfd};
 use crate::fcntl::{FdFlag, OFlag, fcntl};
@@ -1056,13 +1056,13 @@ pub fn lseek64(fd: RawFd, offset: libc::off64_t, whence: Whence) -> Result<libc:
 /// Create an interprocess channel.
 ///
 /// See also [pipe(2)](https://pubs.opengroup.org/onlinepubs/9699919799/functions/pipe.html)
-pub fn pipe() -> Result<(RawFd, RawFd)> {
+pub fn pipe() -> std::result::Result<(RawFd, RawFd), SysError> {
     unsafe {
         let mut fds = mem::MaybeUninit::<[c_int; 2]>::uninit();
 
         let res = libc::pipe(fds.as_mut_ptr() as *mut c_int);
 
-        Errno::result(res)?;
+        SysError::result(res)?;
 
         Ok((fds.assume_init()[0], fds.assume_init()[1]))
     }