Diagnostic: extend from_env function

Cargo.toml

@@ -15,7 +15,7 @@ edition = "2018"
 libc = "0.2.50"
 
 [features]
-check_pipe = []
+do_not_check_pipe = []
 
 [dev-dependencies]
 futures = "0.1"

src/lib.rs

@@ -159,14 +159,36 @@ pub enum ErrFromEnv {
     /// Cannot connect following this process's environment.
     PlatformSpecific {
         /// Error.
-        err: imp::ErrFromEnv,
+        err: io::Error,
         /// Name of gotten env var.
         env: &'static str,
         /// Value of gotten env var.
         var: String,
     },
 }
 
+impl std::fmt::Display for ErrFromEnv {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ErrFromEnv::IsNotConfigured => {
+                write!(f, "couldn't find relevant environment variable")
+            }
+            ErrFromEnv::PlatformSpecific { err, env, var } => {
+                write!(f, "{err} ({env}={var}")
+            }
+        }
+    }
+}
+
+impl std::error::Error for ErrFromEnv {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match self {
+            ErrFromEnv::IsNotConfigured => None,
+            ErrFromEnv::PlatformSpecific { err, .. } => Some(err),
+        }
+    }
+}
+
 impl Client {
     /// Creates a new jobserver initialized with the given parallelism limit.
     ///

src/unix.rs

@@ -25,13 +25,6 @@ pub struct Acquired {
     byte: u8,
 }
 
-#[derive(Debug)]
-pub enum ErrFromEnv {
-    ParseEnvVar,
-    OpenFile(String),
-    InvalidDescriptor(i32, i32),
-}
-
 impl Client {
     pub fn new(mut limit: usize) -> io::Result<Client> {
         let client = unsafe { Client::mk()? };
@@ -88,51 +81,41 @@ impl Client {
         Ok(Client::from_fds(pipes[0], pipes[1]))
     }
 
-    pub unsafe fn open(s: &str) -> Result<Client, ErrFromEnv> {
-        match (Self::from_fifo(s), Self::from_pipe(s)) {
-            (Some(Ok(c)), _) | (_, Some(Ok(c))) => Ok(c),
-            (Some(Err(e)), _) | (_, Some(Err(e))) => Err(e),
-            (None, None) => Err(ErrFromEnv::ParseEnvVar),
-        }
+    pub unsafe fn open(s: &str) -> io::Result<Client> {
+        Ok(Self::from_fifo(s)?.unwrap_or(Self::from_pipe(s)?))
     }
 
     /// `--jobserver-auth=fifo:PATH`
-    fn from_fifo(s: &str) -> Option<Result<Client, ErrFromEnv>> {
+    fn from_fifo(s: &str) -> io::Result<Option<Client>> {
         let mut parts = s.splitn(2, ':');
         if parts.next().unwrap() != "fifo" {
-            return None;
+            return Ok(None);
         }
-        let path = match parts.next() {
-            Some(p) => Path::new(p),
-            None => return Some(Err(ErrFromEnv::ParseEnvVar)),
-        };
-        let file = match OpenOptions::new().read(true).write(true).open(path) {
-            Ok(f) => f,
-            Err(e) => return Some(Err(ErrFromEnv::OpenFile(e.to_string()))),
-        };
-        Some(Ok(Client::Fifo {
+        let path = Path::new(parts.next().ok_or(io::Error::new(
+            io::ErrorKind::InvalidInput,
+            "expected ':' after `fifo`",
+        ))?);
+        let file = OpenOptions::new().read(true).write(true).open(path)?;
+        Ok(Some(Client::Fifo {
             file,
             path: path.into(),
         }))
     }
 
     /// `--jobserver-auth=R,W`
-    unsafe fn from_pipe(s: &str) -> Option<Result<Client, ErrFromEnv>> {
+    unsafe fn from_pipe(s: &str) -> io::Result<Client> {
         let mut parts = s.splitn(2, ',');
         let read = parts.next().unwrap();
-        let write = match parts.next() {
-            Some(s) => s,
-            None => return Some(Err(ErrFromEnv::ParseEnvVar)),
-        };
-
-        let read = match read.parse() {
-            Ok(n) => n,
-            Err(_) => return Some(Err(ErrFromEnv::ParseEnvVar)),
-        };
-        let write = match write.parse() {
-            Ok(n) => n,
-            Err(_) => return Some(Err(ErrFromEnv::ParseEnvVar)),
-        };
+        let write = parts.next().ok_or(io::Error::new(
+            io::ErrorKind::InvalidInput,
+            "expected ',' in `auth=R,W`",
+        ))?;
+        let read = read
+            .parse()
+            .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
+        let write = write
+            .parse()
+            .map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
 
         // Ok so we've got two integers that look like file descriptors, but
         // for extra sanity checking let's see if they actually look like
@@ -142,13 +125,11 @@ impl Client {
         // If we're called from `make` *without* the leading + on our rule
         // then we'll have `MAKEFLAGS` env vars but won't actually have
         // access to the file descriptors.
-        if check_fd(read) && check_fd(write) {
-            drop(set_cloexec(read, true));
-            drop(set_cloexec(write, true));
-            Some(Ok(Client::from_fds(read, write)))
-        } else {
-            Some(Err(ErrFromEnv::InvalidDescriptor(read, write)))
-        }
+        check_fd(read)?;
+        check_fd(write)?;
+        drop(set_cloexec(read, true));
+        drop(set_cloexec(write, true));
+        Ok(Client::from_fds(read, write))
     }
 
     unsafe fn from_fds(read: c_int, write: c_int) -> Client {
@@ -397,25 +378,34 @@ impl Helper {
     }
 }
 
-fn check_fd(fd: c_int) -> bool {
-    #[cfg(feature = "check_pipe")]
+fn check_fd(fd: c_int) -> io::Result<()> {
+    #[cfg(not(feature = "do_not_check_pipe"))]
     unsafe {
         let mut stat = mem::zeroed();
-        if libc::fstat(fd, &mut stat) == 0 {
+        if libc::fstat(fd, &mut stat) == -1 {
+            Err(io::Error::last_os_error())
+        } else {
             // On android arm and i686 mode_t is u16 and st_mode is u32,
             // this generates a type mismatch when S_IFIFO (declared as mode_t)
             // is used in operations with st_mode, so we use this workaround
             // to get the value of S_IFIFO with the same type of st_mode.
             let mut s_ififo = stat.st_mode;
             s_ififo = libc::S_IFIFO as _;
-            stat.st_mode & s_ififo == s_ififo
-        } else {
-            false
+            if stat.st_mode & s_ififo == s_ififo {
+                return Ok(());
+            }
+            Err(io::Error::last_os_error()) //
         }
     }
-    #[cfg(not(feature = "check_pipe"))]
+    #[cfg(feature = "do_not_check_pipe")]
     unsafe {
-        libc::fcntl(fd, libc::F_GETFD) != -1
+        match libc::fcntl(fd, libc::F_GETFD) {
+            r if r == -1 => Err(io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("{fd} is not a pipe"),
+            )),
+            _ => Ok(()),
+        }
     }
 }
 

src/wasm.rs

@@ -17,11 +17,6 @@ struct Inner {
 #[derive(Debug)]
 pub struct Acquired(());
 
-#[derive(Debug)]
-pub enum ErrFromEnv {
-    UnavailableOnTarget,
-}
-
 impl Client {
     pub fn new(limit: usize) -> io::Result<Client> {
         Ok(Client {
@@ -32,8 +27,8 @@ impl Client {
         })
     }
 
-    pub unsafe fn open(_s: &str) -> Result<Client, ErrFromEnv> {
-        Err(ErrFromEnv::UnavailableOnTarget)
+    pub unsafe fn open(_s: &str) -> io::Result<Client> {
+        Err(io::ErrorKind::Unsupported.into())
     }
 
     pub fn acquire(&self) -> io::Result<Acquired> {

src/windows.rs

@@ -14,12 +14,6 @@ pub struct Client {
 #[derive(Debug)]
 pub struct Acquired;
 
-#[derive(Debug)]
-pub enum ErrFromEnv {
-    IsNotCString,
-    CannotAcquireSemaphore,
-}
-
 type BOOL = i32;
 type DWORD = u32;
 type HANDLE = *mut u8;
@@ -133,15 +127,12 @@ impl Client {
         ))
     }
 
-    pub unsafe fn open(s: &str) -> Result<Client, ErrFromEnv> {
-        let name = match CString::new(s) {
-            Ok(s) => s,
-            Err(_) => return Err(ErrFromEnv::IsNotCString),
-        };
+    pub unsafe fn open(s: &str) -> io::Result<Client> {
+        let name = CString::new(s).map_err(|e| io::Error::new(io::ErrorKind::Other, e))?;
 
         let sem = OpenSemaphoreA(SYNCHRONIZE | SEMAPHORE_MODIFY_STATE, FALSE, name.as_ptr());
         if sem.is_null() {
-            Err(ErrFromEnv::CannotAcquireSemaphore)
+            Err(io::Error::last_os_error())
         } else {
             Ok(Client {
                 sem: Handle(sem),