forkpty: Address review comments

nix-rust · Apr 14, 2019 · 3a95beb · 3a95beb
1 parent 67688aa
commit 3a95beb
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 50 deletions.
diff --git a/src/pty.rs b/src/pty.rs
@@ -31,8 +31,7 @@ pub struct OpenptyResult {
 ///
 /// This is returned by `forkpty`. Note that this type does *not* implement `Drop`, so the user
 /// must manually close the file descriptors.
-#[derive(Clone, Copy)]
-#[allow(missing_debug_implementations)]
+#[derive(Clone, Copy, Debug)]
 pub struct ForkptyResult {
     /// The master port in a virtual pty pair
     pub master: RawFd,
@@ -288,58 +287,31 @@ pub fn openpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>
 /// If `winsize` is not `None`, the window size of the slave will be set to
 /// the values in `winsize`. If `termios` is not `None`, the pseudoterminal's
 /// terminal settings of the slave will be set to the values in `termios`.
-#[inline]
-pub fn forkpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>>>(winsize: T, termios: U) -> Result<ForkptyResult> {
+pub fn forkpty<'a, 'b, T: Into<Option<&'a Winsize>>, U: Into<Option<&'b Termios>>>(
+    winsize: T,
+    termios: U,
+) -> Result<ForkptyResult> {
     use std::ptr;
     use unistd::Pid;
     use unistd::ForkResult::*;
 
     let mut master: libc::c_int = unsafe { mem::uninitialized() };
-    let res = {
-        match (termios.into(), winsize.into()) {
-            (Some(termios), Some(winsize)) => {
-                let inner_termios = termios.get_libc_termios();
-                unsafe {
-                    libc::forkpty(
-                        &mut master,
-                        ptr::null_mut(),
-                        &*inner_termios as *const libc::termios as *mut _,
-                        winsize as *const Winsize as *mut _,
-                    )
-                }
-            }
-            (None, Some(winsize)) => {
-                unsafe {
-                    libc::forkpty(
-                        &mut master,
-                        ptr::null_mut(),
-                        ptr::null_mut(),
-                        winsize as *const Winsize as *mut _,
-                    )
-                }
-            }
-            (Some(termios), None) => {
-                let inner_termios = termios.get_libc_termios();
-                unsafe {
-                    libc::forkpty(
-                        &mut master,
-                        ptr::null_mut(),
-                        &*inner_termios as *const libc::termios as *mut _,
-                        ptr::null_mut(),
-                    )
-                }
-            }
-            (None, None) => {
-                unsafe {
-                    libc::forkpty(
-                        &mut master,
-                        ptr::null_mut(),
-                        ptr::null_mut(),
-                        ptr::null_mut(),
-                    )
-                }
-            }
-        }
+
+    let term = match termios.into() {
+        Some(termios) => {
+            let inner_termios = termios.get_libc_termios();
+            &*inner_termios as *const libc::termios as *mut _
+        },
+        None => ptr::null_mut(),
+    };
+
+    let win = winsize
+        .into()
+        .map(|ws| ws as *const Winsize as *mut _)
+        .unwrap_or(ptr::null_mut());
+
+    let res = unsafe {
+        libc::forkpty(&mut master, ptr::null_mut(), term, win)
     };
 
     let fork_result = Errno::result(res).map(|res| match res {

diff --git a/test/test_pty.rs b/test/test_pty.rs
@@ -3,6 +3,7 @@ use std::path::Path;
 use std::os::unix::prelude::*;
 use tempfile::tempfile;
 
+use libc::{_exit, STDOUT_FILENO};
 use nix::fcntl::{OFlag, open};
 use nix::pty::*;
 use nix::sys::stat;
@@ -217,8 +218,9 @@ fn test_forkpty() {
     let pty = forkpty(None, None).unwrap();
     match pty.fork_result {
         Child => {
-            write(0, string.as_bytes()).unwrap();
+            write(STDOUT_FILENO, string.as_bytes()).unwrap();
             pause();  // we need the child to stay alive until the parent calls read
+            unsafe { _exit(0); }
         },
         Parent { child } => {
             let mut buf = [0u8; 10];