Merge pull request #1 from shell-pool/add-borrow-fd

add borrow_fd method to Slave and Master

.github/workflows/nightly.yml

@@ -0,0 +1,20 @@
+name: nightly
+on:
+  schedule:
+    - cron: '04 05 * * *'
+
+jobs:
+  deny:
+    name: cargo deny --all-features check
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
+      - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f
+        with:
+          channel: '1.74.0'
+          bins: cargo-deny
+      - run: sudo apt-get install libpam0g-dev
+      - run: cargo deny --all-features check
+
+  postsubmit:
+    uses: ./.github/workflows/presubmit.yml

.github/workflows/presubmit.yml

@@ -0,0 +1,51 @@
+name: presubmit
+on: [pull_request, workflow_call, workflow_dispatch]
+
+jobs:
+  test:
+    name: cargo test --all-features
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
+      - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f
+        with:
+          channel: '1.74.0'
+      - run: sudo apt-get install libpam0g-dev
+      - run: cargo test --all-features
+      - run: cargo test --no-default-features
+
+  rustfmt:
+    name: cargo +nightly fmt -- --check
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
+      - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f
+        with:
+          components: rustfmt
+          channel: nightly
+      - run: cargo +nightly fmt -- --check
+
+  cranky:
+    name: cargo +nightly cranky --all-targets -- -D warnings
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
+      - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f
+        with:
+          components: clippy
+          bins: [email protected]
+          channel: nightly
+      - run: sudo apt-get install libpam0g-dev
+      - run: cargo +nightly cranky --all-targets -- -D warnings
+
+  deny:
+    name: cargo deny --all-features check licenses
+    runs-on: ubuntu-22.04
+    steps:
+      - uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11
+      - uses: moonrepo/setup-rust@b8edcc56aab474d90c7cf0bb8beeaf8334c15e9f
+        with:
+          channel: '1.74.0'
+          bins: cargo-deny
+      - run: sudo apt-get install libpam0g-dev
+      - run: cargo deny --all-features check licenses

deny.toml

@@ -0,0 +1,18 @@
+[graph]
+all-features = true
+
+[advisories]
+version = 2
+db-path = "~/.cargo/advisory-db"
+db-urls = ["https://github.com/rustsec/advisory-db"]
+yanked = "deny"
+ignore = [
+]
+
+[licenses]
+allow = [
+    "Apache-2.0",
+    "MIT",
+    "Unicode-DFS-2016",
+]
+confidence-threshold = 1.0

examples/main.rs

@@ -1,15 +1,15 @@
-extern crate shpool_pty;
-extern crate libc;
 extern crate errno;
+extern crate libc;
+extern crate shpool_pty;
 
 use shpool_pty::fork::*;
 use std::io::Read;
-use std::process::{Command};
+use std::process::Command;
 
 fn main() {
     let fork = Fork::from_ptmx().unwrap();
 
-    if let Some(mut master) = fork.is_parent().ok() {
+    if let Ok(mut master) = fork.is_parent() {
         // Read output via PTY master
         let mut output = String::new();
 

examples/new.rs

@@ -1,5 +1,5 @@
-extern crate shpool_pty;
 extern crate libc;
+extern crate shpool_pty;
 
 use self::shpool_pty::prelude::*;
 
@@ -9,10 +9,12 @@ use std::process::{Command, Stdio};
 fn main() {
     let fork = Fork::from_ptmx().unwrap();
 
-    if let Some(mut master) = fork.is_parent().ok() {
+    if let Ok(mut master) = fork.is_parent() {
         let mut string = String::new();
 
-        master.read_to_string(&mut string).unwrap_or_else(|e| panic!("{}", e));
+        master
+            .read_to_string(&mut string)
+            .unwrap_or_else(|e| panic!("{}", e));
 
         let output = Command::new("tty")
             .stdin(Stdio::inherit())
@@ -24,12 +26,17 @@ fn main() {
         let parent_tty = output_str.trim();
         let child_tty = string.trim();
 
-        println!("child_tty(\"{}\")[{}] != \"{}\" => {}", child_tty, child_tty.len(), "", child_tty != "");
-        assert!(child_tty != "");
+        println!(
+            "child_tty(\"{}\")[{}] != \"\" => {}",
+            child_tty,
+            child_tty.len(),
+            !child_tty.is_empty()
+        );
+        assert!(!child_tty.is_empty());
         assert!(child_tty != parent_tty);
 
-        let mut parent_tty_dir: Vec<&str> = parent_tty.split("/").collect();
-        let mut child_tty_dir: Vec<&str> = child_tty.split("/").collect();
+        let mut parent_tty_dir: Vec<&str> = parent_tty.split('/').collect();
+        let mut child_tty_dir: Vec<&str> = child_tty.split('/').collect();
 
         parent_tty_dir.pop();
         child_tty_dir.pop();

src/descriptor/mod.rs

@@ -1,21 +1,27 @@
 mod err;
 
-use ::libc;
+use libc;
 
 pub use self::err::DescriptorError;
 use std::ffi::CStr;
 use std::os::unix::io::RawFd;
 
+/// # Safety
+///
+/// Implmentations MUST only return valid fds from
+/// these methods. Callers MUST NOT use the fds once the
+/// returning object has fallen out of scope.
 pub unsafe trait Descriptor {
     /// If the descriptor has a valid fd, return it
     fn take_raw_fd(&mut self) -> Option<RawFd>;
 
     /// The constructor function `open` opens the path
     /// and returns the fd.
-    fn open(path: &CStr,
-            flag: libc::c_int,
-            mode: Option<libc::c_int>)
-            -> Result<RawFd, DescriptorError> {
+    fn open(
+        path: &CStr,
+        flag: libc::c_int,
+        mode: Option<libc::c_int>,
+    ) -> Result<RawFd, DescriptorError> {
         // Safety: we've just ensured that path is non-null and the
         // other params are valid by construction.
         unsafe {

src/fork/err.rs

@@ -1,4 +1,4 @@
-use ::descriptor::DescriptorError;
+use descriptor::DescriptorError;
 use std::error::Error;
 use std::fmt;
 

src/fork/mod.rs

@@ -1,12 +1,12 @@
-mod pty;
 mod err;
+mod pty;
 
-use ::descriptor::Descriptor;
+use descriptor::Descriptor;
 
 pub use self::err::{ForkError, Result};
 pub use self::pty::{Master, MasterError};
 pub use self::pty::{Slave, SlaveError};
-use ::libc;
+use libc;
 use std::ffi::CStr;
 use std::ffi::CString;
 
@@ -47,13 +47,14 @@ impl Fork {
                             // Safety: ptsname_r returns a valid c string when it returns a 0
                             // (success) code, and we make double extra sure there is a null
                             // terminator by adding one ourselves.
-                            let name: &CStr = unsafe { CStr::from_ptr(name_ptr as *const libc::c_char) };
+                            let name: &CStr =
+                                unsafe { CStr::from_ptr(name_ptr as *const libc::c_char) };
                             Fork::from_pts(name)
                         }
                         pid => Ok(Fork::Parent(pid, master)),
                     }
                 }
-            },
+            }
         }
     }
 
@@ -68,11 +69,11 @@ impl Fork {
                 match Slave::new(ptsname) {
                     Err(cause) => Err(ForkError::BadSlave(cause)),
                     Ok(slave) => {
-                        if let Some(cause) = slave.dup2(libc::STDIN_FILENO)
+                        if let Some(cause) = slave.dup2(libc::STDIN_FILENO).err().or(slave
+                            .dup2(libc::STDOUT_FILENO)
                             .err()
-                            .or(slave.dup2(libc::STDOUT_FILENO)
-                                .err()
-                                .or(slave.dup2(libc::STDERR_FILENO).err())) {
+                            .or(slave.dup2(libc::STDERR_FILENO).err()))
+                        {
                             Err(ForkError::BadSlave(cause))
                         } else {
                             Ok(Fork::Child(slave))
@@ -98,24 +99,22 @@ impl Fork {
     pub fn wait_for_exit(&self) -> Result<(libc::pid_t, Option<i32>)> {
         match *self {
             Fork::Child(_) => Err(ForkError::IsChild),
-            Fork::Parent(pid, _) => {
-                loop {
-                    unsafe {
-                        let mut status = 0;
-                        match libc::waitpid(pid, &mut status, 0) {
-                            0 => continue,
-                            -1 => return Err(ForkError::WaitpidFail),
-                            _ => {
-                                if libc::WIFEXITED(status) {
-                                    return Ok((pid, Some(libc::WEXITSTATUS(status))));
-                                } else {
-                                    return Ok((pid, None));
-                                }
+            Fork::Parent(pid, _) => loop {
+                unsafe {
+                    let mut status = 0;
+                    match libc::waitpid(pid, &mut status, 0) {
+                        0 => continue,
+                        -1 => return Err(ForkError::WaitpidFail),
+                        _ => {
+                            if libc::WIFEXITED(status) {
+                                return Ok((pid, Some(libc::WEXITSTATUS(status))));
+                            } else {
+                                return Ok((pid, None));
                             }
                         }
                     }
                 }
-            }
+            },
         }
     }
 
@@ -134,7 +133,7 @@ impl Fork {
     pub fn is_parent(&self) -> Result<Master> {
         match *self {
             Fork::Child(_) => Err(ForkError::IsChild),
-            Fork::Parent(_, ref master) => Ok(master.clone()),
+            Fork::Parent(_, ref master) => Ok(*master),
         }
     }
 
@@ -150,9 +149,8 @@ impl Fork {
 
 impl Drop for Fork {
     fn drop(&mut self) {
-        match self {
-            Fork::Parent(_, master) => Descriptor::drop(master),
-            _ => {}
+        if let Fork::Parent(_, master) = self {
+            Descriptor::drop(master)
         }
     }
 }

src/fork/pty/master/err.rs

@@ -1,4 +1,4 @@
-use ::descriptor::DescriptorError;
+use descriptor::DescriptorError;
 use std::error::Error;
 use std::fmt;
 
@@ -34,7 +34,6 @@ impl Error for MasterError {
             MasterError::UnlockptError => "the `grantpt` has a error, errnois set appropriately.",
             MasterError::PtsnameError => "the `ptsname` has a error",
             MasterError::NoFdError => "already closed, no fd",
-
         }
     }
 

src/fork/pty/master/mod.rs

@@ -2,11 +2,12 @@ mod err;
 
 use libc;
 
-use ::descriptor::Descriptor;
+use descriptor::Descriptor;
 
 pub use self::err::{MasterError, Result};
 use std::ffi::CStr;
 use std::io;
+use std::os::fd::BorrowedFd;
 use std::os::unix::io::RawFd;
 
 #[derive(Debug, Copy, Clone)]
@@ -27,6 +28,13 @@ impl Master {
         &self.pty
     }
 
+    /// Borrow the raw fd
+    pub fn borrow_fd(&self) -> Option<BorrowedFd<'_>> {
+        // Safety: we only ever close on drop, so this will be
+        //         live the whole time.
+        self.pty.map(|fd| unsafe { BorrowedFd::borrow_raw(fd) })
+    }
+
     /// Change UID and GID of slave pty associated with master pty whose
     /// fd is provided, to the real UID and real GID of the calling thread.
     pub fn grantpt(&self) -> Result<libc::c_int> {
@@ -58,15 +66,15 @@ impl Master {
 
     /// Returns a pointer to a static buffer, which will be overwritten on
     /// subsequent calls.
-    pub fn ptsname_r(&self, buf: &mut Vec<u8>) -> Result<()> {
+    pub fn ptsname_r(&self, buf: &mut [u8]) -> Result<()> {
         if let Some(fd) = self.pty {
             // Safety: the vector's memory is valid for the duration
             // of the call
             unsafe {
                 let data: *mut u8 = &mut buf[0];
                 match libc::ptsname_r(fd, data as *mut libc::c_char, buf.len()) {
                     0 => Ok(()),
-                    _ => Err(MasterError::PtsnameError),  // should probably capture errno
+                    _ => Err(MasterError::PtsnameError), // should probably capture errno
                 }
             }
         } else {
@@ -85,9 +93,7 @@ impl io::Read for Master {
     fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
         if let Some(fd) = self.pty {
             unsafe {
-                match libc::read(fd,
-                                 buf.as_mut_ptr() as *mut libc::c_void,
-                                 buf.len()) {
+                match libc::read(fd, buf.as_mut_ptr() as *mut libc::c_void, buf.len()) {
                     -1 => Ok(0),
                     len => Ok(len as usize),
                 }
@@ -102,9 +108,7 @@ impl io::Write for Master {
     fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
         if let Some(fd) = self.pty {
             unsafe {
-                match libc::write(fd,
-                                  buf.as_ptr() as *const libc::c_void,
-                                  buf.len()) {
+                match libc::write(fd, buf.as_ptr() as *const libc::c_void, buf.len()) {
                     -1 => Err(io::Error::last_os_error()),
                     ret => Ok(ret as usize),
                 }

src/fork/pty/slave/err.rs

@@ -1,4 +1,4 @@
-use ::descriptor::DescriptorError;
+use descriptor::DescriptorError;
 use std::error::Error;
 use std::fmt;