diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml new file mode 100644 index 0000000..6b7ab5c --- /dev/null +++ b/.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 diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml new file mode 100644 index 0000000..8ce2ead --- /dev/null +++ b/.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: cargo-cranky@0.3.0 + 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 diff --git a/deny.toml b/deny.toml new file mode 100644 index 0000000..0ba9b55 --- /dev/null +++ b/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 diff --git a/examples/main.rs b/examples/main.rs index 1becaff..21d6152 100644 --- a/examples/main.rs +++ b/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(); diff --git a/examples/new.rs b/examples/new.rs index 51b3931..d78bc9e 100644 --- a/examples/new.rs +++ b/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(); diff --git a/rustfmt.toml b/rustfmt.toml deleted file mode 100644 index 5df0356..0000000 --- a/rustfmt.toml +++ /dev/null @@ -1,2 +0,0 @@ -tabs_spaces=4 -hard_tabs=false diff --git a/src/descriptor/mod.rs b/src/descriptor/mod.rs index df5d3f8..ce92d85 100644 --- a/src/descriptor/mod.rs +++ b/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; /// The constructor function `open` opens the path /// and returns the fd. - fn open(path: &CStr, - flag: libc::c_int, - mode: Option) - -> Result { + fn open( + path: &CStr, + flag: libc::c_int, + mode: Option, + ) -> Result { // Safety: we've just ensured that path is non-null and the // other params are valid by construction. unsafe { diff --git a/src/fork/err.rs b/src/fork/err.rs index 60c3aae..f245cd3 100644 --- a/src/fork/err.rs +++ b/src/fork/err.rs @@ -1,4 +1,4 @@ -use ::descriptor::DescriptorError; +use descriptor::DescriptorError; use std::error::Error; use std::fmt; diff --git a/src/fork/mod.rs b/src/fork/mod.rs index b441303..5fffd9b 100644 --- a/src/fork/mod.rs +++ b/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)> { 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 { 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) } } } diff --git a/src/fork/pty/master/err.rs b/src/fork/pty/master/err.rs index 19c331e..0f63fd6 100644 --- a/src/fork/pty/master/err.rs +++ b/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", - } } diff --git a/src/fork/pty/master/mod.rs b/src/fork/pty/master/mod.rs index 6927626..3332836 100644 --- a/src/fork/pty/master/mod.rs +++ b/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> { + // 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 { @@ -58,7 +66,7 @@ impl Master { /// Returns a pointer to a static buffer, which will be overwritten on /// subsequent calls. - pub fn ptsname_r(&self, buf: &mut Vec) -> 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 @@ -66,7 +74,7 @@ impl Master { 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 { 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 { 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), } diff --git a/src/fork/pty/slave/err.rs b/src/fork/pty/slave/err.rs index 4498acd..88cc272 100644 --- a/src/fork/pty/slave/err.rs +++ b/src/fork/pty/slave/err.rs @@ -1,4 +1,4 @@ -use ::descriptor::DescriptorError; +use descriptor::DescriptorError; use std::error::Error; use std::fmt; diff --git a/src/fork/pty/slave/mod.rs b/src/fork/pty/slave/mod.rs index 6f1ab01..7bf8179 100644 --- a/src/fork/pty/slave/mod.rs +++ b/src/fork/pty/slave/mod.rs @@ -1,10 +1,11 @@ mod err; -use ::descriptor::Descriptor; -use ::libc; +use descriptor::Descriptor; +use libc; -pub use self::err::{SlaveError, Result}; +pub use self::err::{Result, SlaveError}; use std::ffi::CStr; +use std::os::fd::BorrowedFd; use std::os::unix::io::RawFd; #[derive(Debug, Clone)] @@ -26,6 +27,13 @@ impl Slave { &self.pty } + /// Borrow the raw fd + pub fn borrow_fd(&self) -> Option> { + // Safety: we only ever close on drop, so this will be + // live the whole time. + self.pty.map(|fd| unsafe { BorrowedFd::borrow_raw(fd) }) + } + pub fn dup2(&self, std: libc::c_int) -> Result { if let Some(fd) = self.pty { unsafe { diff --git a/src/lib.rs b/src/lib.rs index 728d83d..04ea011 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -71,28 +71,12 @@ //! } //! ``` -#![crate_type = "lib"] - -#![cfg_attr(feature = "nightly", feature(plugin))] -#![cfg_attr(feature = "lints", plugin(clippy))] -#![cfg_attr(feature = "lints", deny(warnings))] -#![deny( - missing_debug_implementations, - missing_copy_implementations, - trivial_casts, - trivial_numeric_casts, - unstable_features, - unused_import_braces, - unused_features, - unused_qualifications, -)] - -extern crate libc; extern crate errno; +extern crate libc; extern crate log; mod descriptor; pub mod fork; pub mod prelude; -const DEFAULT_PTMX: &'static str = "/dev/ptmx"; +const DEFAULT_PTMX: &str = "/dev/ptmx"; diff --git a/tests/it_can_read_write.rs b/tests/it_can_read_write.rs index 90558dc..91da56f 100644 --- a/tests/it_can_read_write.rs +++ b/tests/it_can_read_write.rs @@ -1,17 +1,17 @@ -extern crate shpool_pty; extern crate libc; +extern crate shpool_pty; use self::shpool_pty::prelude::*; use std::io::prelude::*; -use std::string::String; use std::process::Command; +use std::string::String; -fn read_line(master:&mut Master) -> String { +fn read_line(master: &mut Master) -> String { let mut buf = [0]; let mut res = String::new(); while buf[0] as char != '\n' { - master.read(&mut buf).expect("cannot read 1 byte"); + master.read_exact(&mut buf).expect("cannot read 1 byte"); res.push(buf[0] as char) } res @@ -21,7 +21,7 @@ fn read_line(master:&mut Master) -> String { fn it_can_read_write() { let fork = Fork::from_ptmx().unwrap(); - if let Some(mut master) = fork.is_parent().ok() { + if let Ok(mut master) = fork.is_parent() { let _ = master.write("echo readme!\n".to_string().as_bytes()); read_line(&mut master); // this is the "echo readme!" we just sent diff --git a/tests/it_fork_with_new_pty.rs b/tests/it_fork_with_new_pty.rs index 65d0373..b8a5c14 100644 --- a/tests/it_fork_with_new_pty.rs +++ b/tests/it_fork_with_new_pty.rs @@ -1,6 +1,6 @@ -extern crate shpool_pty; -extern crate libc; extern crate errno; +extern crate libc; +extern crate shpool_pty; use self::shpool_pty::prelude::*; @@ -12,10 +12,12 @@ use std::string::String; fn it_fork_with_new_pty() { 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()) @@ -27,14 +29,14 @@ fn it_fork_with_new_pty() { let parent_tty = output_str.trim(); let child_tty = string.trim(); - assert!(child_tty != ""); + assert!(!child_tty.is_empty()); assert!(child_tty != parent_tty); // only compare if parent is tty // travis runs the tests without tty if parent_tty != "not a 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(); diff --git a/tests/lib.rs b/tests/lib.rs index 51b0338..9c047bb 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -1,2 +1,2 @@ -mod it_fork_with_new_pty; mod it_can_read_write; +mod it_fork_with_new_pty;