From 3cec8dab5338abb90637d0f59fca85cb6fe6ef05 Mon Sep 17 00:00:00 2001
From: Kevin Kuehler <keur@ocf.berkeley.edu>
Date: Thu, 11 Apr 2019 00:11:29 -0700
Subject: [PATCH] forkpty: Address review comments

---
 src/pty.rs       | 69 +++++++++++++++---------------------------------
 test/test_pty.rs |  4 ++-
 2 files changed, 24 insertions(+), 49 deletions(-)

diff --git a/src/pty.rs b/src/pty.rs
index 0001d12551..1eccd8d547 100644
--- a/src/pty.rs
+++ b/src/pty.rs
@@ -31,7 +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)]
+#[derive(Clone, Copy, Debug)]
 #[allow(missing_debug_implementations)]
 pub struct ForkptyResult {
     /// The master port in a virtual pty pair
@@ -288,58 +288,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 = match winsize.into() {
+        Some(winsize) => winsize as *const Winsize as *mut _,
+        None => 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
index 0fbdd79cc8..476b15c101 100644
--- 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];