From 6c16479eb2e40238239ca67b0e8eca1848e840d6 Mon Sep 17 00:00:00 2001 From: Erik Karlsson Date: Fri, 7 Oct 2022 19:20:38 +0200 Subject: [PATCH] Remove unnecessary immediately invoked closures Signed-off-by: Erik Karlsson --- src/process.rs | 104 +++++++++++++++------------------ src/reader.rs | 40 ++++++------- src/session.rs | 154 +++++++++++++++++++++---------------------------- 3 files changed, 132 insertions(+), 166 deletions(-) diff --git a/src/process.rs b/src/process.rs index aa095fb6..1fae19bd 100644 --- a/src/process.rs +++ b/src/process.rs @@ -88,47 +88,44 @@ fn ptsname_r(fd: &PtyMaster) -> nix::Result { impl PtyProcess { /// Start a process in a forked pty pub fn new(mut command: Command) -> Result { - || -> nix::Result { - // Open a new PTY master - let master_fd = posix_openpt(OFlag::O_RDWR)?; + // Open a new PTY master + let master_fd = posix_openpt(OFlag::O_RDWR)?; - // Allow a slave to be generated for it - grantpt(&master_fd)?; - unlockpt(&master_fd)?; + // Allow a slave to be generated for it + grantpt(&master_fd)?; + unlockpt(&master_fd)?; - // on Linux this is the libc function, on OSX this is our implementation of ptsname_r - let slave_name = ptsname_r(&master_fd)?; + // on Linux this is the libc function, on OSX this is our implementation of ptsname_r + let slave_name = ptsname_r(&master_fd)?; - match fork()? { - ForkResult::Child => { - setsid()?; // create new session with child as session leader - let slave_fd = open( - std::path::Path::new(&slave_name), - OFlag::O_RDWR, - stat::Mode::empty(), - )?; + match fork()? { + ForkResult::Child => { + setsid()?; // create new session with child as session leader + let slave_fd = open( + std::path::Path::new(&slave_name), + OFlag::O_RDWR, + stat::Mode::empty(), + )?; - // assign stdin, stdout, stderr to the tty, just like a terminal does - dup2(slave_fd, STDIN_FILENO)?; - dup2(slave_fd, STDOUT_FILENO)?; - dup2(slave_fd, STDERR_FILENO)?; + // assign stdin, stdout, stderr to the tty, just like a terminal does + dup2(slave_fd, STDIN_FILENO)?; + dup2(slave_fd, STDOUT_FILENO)?; + dup2(slave_fd, STDERR_FILENO)?; - // set echo off - let mut flags = termios::tcgetattr(STDIN_FILENO)?; - flags.local_flags &= !termios::LocalFlags::ECHO; - termios::tcsetattr(STDIN_FILENO, termios::SetArg::TCSANOW, &flags)?; + // set echo off + let mut flags = termios::tcgetattr(STDIN_FILENO)?; + flags.local_flags &= !termios::LocalFlags::ECHO; + termios::tcsetattr(STDIN_FILENO, termios::SetArg::TCSANOW, &flags)?; - command.exec(); - Err(nix::Error::last()) - } - ForkResult::Parent { child: child_pid } => Ok(PtyProcess { - pty: master_fd, - child_pid, - kill_timeout: None, - }), + command.exec(); + Err(Error::Nix(nix::Error::last())) } - }() - .map_err(Error::from) + ForkResult::Parent { child: child_pid } => Ok(PtyProcess { + pty: master_fd, + child_pid, + kill_timeout: None, + }), + } } /// Get handle to pty fork for reading/writing @@ -242,28 +239,23 @@ mod tests { #[test] /// Open cat, write string, read back string twice, send Ctrl^C and check that cat exited - fn test_cat() { - // wrapping into closure so I can use ? - || -> std::io::Result<()> { - let process = PtyProcess::new(Command::new("cat")).expect("could not execute cat"); - let f = process.get_file_handle(); - let mut writer = LineWriter::new(&f); - let mut reader = BufReader::new(&f); - let _ = writer.write(b"hello cat\n")?; - let mut buf = String::new(); - reader.read_line(&mut buf)?; - assert_eq!(buf, "hello cat\r\n"); + fn test_cat() -> std::io::Result<()> { + let process = PtyProcess::new(Command::new("cat")).expect("could not execute cat"); + let f = process.get_file_handle(); + let mut writer = LineWriter::new(&f); + let mut reader = BufReader::new(&f); + let _ = writer.write(b"hello cat\n")?; + let mut buf = String::new(); + reader.read_line(&mut buf)?; + assert_eq!(buf, "hello cat\r\n"); - // this sleep solves an edge case of some cases when cat is somehow not "ready" - // to take the ^C (occasional test hangs) - thread::sleep(time::Duration::from_millis(100)); - writer.write_all(&[3])?; // send ^C - writer.flush()?; - let should = - wait::WaitStatus::Signaled(process.child_pid, signal::Signal::SIGINT, false); - assert_eq!(should, wait::waitpid(process.child_pid, None).unwrap()); - Ok(()) - }() - .unwrap_or_else(|e| panic!("test_cat failed: {}", e)); + // this sleep solves an edge case of some cases when cat is somehow not "ready" + // to take the ^C (occasional test hangs) + thread::sleep(time::Duration::from_millis(100)); + writer.write_all(&[3])?; // send ^C + writer.flush()?; + let should = wait::WaitStatus::Signaled(process.child_pid, signal::Signal::SIGINT, false); + assert_eq!(should, wait::waitpid(process.child_pid, None).unwrap()); + Ok(()) } } diff --git a/src/reader.rs b/src/reader.rs index c2f11c0e..e6a8a103 100644 --- a/src/reader.rs +++ b/src/reader.rs @@ -123,29 +123,27 @@ impl NBReader { let (tx, rx) = channel(); // spawn a thread which reads one char and sends it to tx - thread::spawn(move || { - let _ = || -> Result<(), Error> { - let mut reader = BufReader::new(f); - let mut byte = [0u8]; - loop { - match reader.read(&mut byte) { - Ok(0) => { - tx.send(Ok(PipedChar::EOF)) - .map_err(|_| Error::MpscSendError)?; - break; - } - Ok(_) => { - tx.send(Ok(PipedChar::Char(byte[0]))) - .map_err(|_| Error::MpscSendError)?; - } - Err(error) => { - tx.send(Err(PipeError::IO(error))) - .map_err(|_| Error::MpscSendError)?; - } + thread::spawn(move || -> Result<(), Error> { + let mut reader = BufReader::new(f); + let mut byte = [0u8]; + loop { + match reader.read(&mut byte) { + Ok(0) => { + tx.send(Ok(PipedChar::EOF)) + .map_err(|_| Error::MpscSendError)?; + break; + } + Ok(_) => { + tx.send(Ok(PipedChar::Char(byte[0]))) + .map_err(|_| Error::MpscSendError)?; + } + Err(error) => { + tx.send(Err(PipeError::IO(error))) + .map_err(|_| Error::MpscSendError)?; } } - Ok(()) - }(); + } + Ok(()) // don't do error handling as on an error it was most probably // the main thread which exited (remote hangup) }); diff --git a/src/session.rs b/src/session.rs index 5e06c3c6..4de5a23e 100644 --- a/src/session.rs +++ b/src/session.rs @@ -431,34 +431,28 @@ mod tests { use super::*; #[test] - fn test_read_line() { - || -> Result<(), Error> { - let mut s = spawn("cat", Some(1000))?; - s.send_line("hans")?; - assert_eq!("hans", s.read_line()?); - let should = crate::process::wait::WaitStatus::Signaled( - s.process.child_pid, - crate::process::signal::Signal::SIGTERM, - false, - ); - assert_eq!(should, s.process.exit()?); - Ok(()) - }() - .unwrap_or_else(|e| panic!("test_read_line failed: {}", e)); + fn test_read_line() -> Result<(), Error> { + let mut s = spawn("cat", Some(1000))?; + s.send_line("hans")?; + assert_eq!("hans", s.read_line()?); + let should = crate::process::wait::WaitStatus::Signaled( + s.process.child_pid, + crate::process::signal::Signal::SIGTERM, + false, + ); + assert_eq!(should, s.process.exit()?); + Ok(()) } #[test] - fn test_expect_eof_timeout() { - || -> Result<(), Error> { - let mut p = spawn("sleep 3", Some(1000)).expect("cannot run sleep 3"); - match p.exp_eof() { - Ok(_) => panic!("should raise Timeout"), - Err(Error::Timeout { .. }) => {} - Err(_) => panic!("should raise TimeOut"), - } - Ok(()) - }() - .unwrap_or_else(|e| panic!("test_timeout failed: {}", e)); + fn test_expect_eof_timeout() -> Result<(), Error> { + let mut p = spawn("sleep 3", Some(1000)).expect("cannot run sleep 3"); + match p.exp_eof() { + Ok(_) => panic!("should raise Timeout"), + Err(Error::Timeout { .. }) => {} + Err(_) => panic!("should raise TimeOut"), + } + Ok(()) } #[test] @@ -468,44 +462,35 @@ mod tests { } #[test] - fn test_expect_string() { - || -> Result<(), Error> { - let mut p = spawn("cat", Some(1000)).expect("cannot run cat"); - p.send_line("hello world!")?; - p.exp_string("hello world!")?; - p.send_line("hello heaven!")?; - p.exp_string("hello heaven!")?; - Ok(()) - }() - .unwrap_or_else(|e| panic!("test_expect_string failed: {}", e)); + fn test_expect_string() -> Result<(), Error> { + let mut p = spawn("cat", Some(1000)).expect("cannot run cat"); + p.send_line("hello world!")?; + p.exp_string("hello world!")?; + p.send_line("hello heaven!")?; + p.exp_string("hello heaven!")?; + Ok(()) } #[test] - fn test_read_string_before() { - || -> Result<(), Error> { - let mut p = spawn("cat", Some(1000)).expect("cannot run cat"); - p.send_line("lorem ipsum dolor sit amet")?; - assert_eq!("lorem ipsum dolor sit ", p.exp_string("amet")?); - Ok(()) - }() - .unwrap_or_else(|e| panic!("test_read_string_before failed: {}", e)); + fn test_read_string_before() -> Result<(), Error> { + let mut p = spawn("cat", Some(1000)).expect("cannot run cat"); + p.send_line("lorem ipsum dolor sit amet")?; + assert_eq!("lorem ipsum dolor sit ", p.exp_string("amet")?); + Ok(()) } #[test] - fn test_expect_any() { - || -> Result<(), Error> { - let mut p = spawn("cat", Some(1000)).expect("cannot run cat"); - p.send_line("Hi")?; - match p.exp_any(vec![ - ReadUntil::NBytes(3), - ReadUntil::String("Hi".to_string()), - ]) { - Ok(s) => assert_eq!(("".to_string(), "Hi".to_string()), s), - Err(e) => panic!("got error: {}", e), - } - Ok(()) - }() - .unwrap_or_else(|e| panic!("test_expect_any failed: {}", e)); + fn test_expect_any() -> Result<(), Error> { + let mut p = spawn("cat", Some(1000)).expect("cannot run cat"); + p.send_line("Hi")?; + match p.exp_any(vec![ + ReadUntil::NBytes(3), + ReadUntil::String("Hi".to_string()), + ]) { + Ok(s) => assert_eq!(("".to_string(), "Hi".to_string()), s), + Err(e) => panic!("got error: {}", e), + } + Ok(()) } #[test] @@ -519,46 +504,37 @@ mod tests { } #[test] - fn test_kill_timeout() { - || -> Result<(), Error> { - let mut p = spawn_bash(Some(1000))?; - p.execute("cat <(echo ready) -", "ready")?; - Ok(()) - }() - .unwrap_or_else(|e| panic!("test_kill_timeout failed: {}", e)); + fn test_kill_timeout() -> Result<(), Error> { + let mut p = spawn_bash(Some(1000))?; + p.execute("cat <(echo ready) -", "ready")?; + Ok(()) // p is dropped here and kill is sent immediately to bash // Since that is not enough to make bash exit, a kill -9 is sent within 1s (timeout) } #[test] - fn test_bash() { - || -> Result<(), Error> { - let mut p = spawn_bash(Some(1000))?; - p.send_line("cd /tmp/")?; - p.wait_for_prompt()?; - p.send_line("pwd")?; - assert_eq!("/tmp\r\n", p.wait_for_prompt()?); - Ok(()) - }() - .unwrap_or_else(|e| panic!("test_bash failed: {}", e)); + fn test_bash() -> Result<(), Error> { + let mut p = spawn_bash(Some(1000))?; + p.send_line("cd /tmp/")?; + p.wait_for_prompt()?; + p.send_line("pwd")?; + assert_eq!("/tmp\r\n", p.wait_for_prompt()?); + Ok(()) } #[test] - fn test_bash_control_chars() { - || -> Result<(), Error> { - let mut p = spawn_bash(Some(1000))?; - p.execute("cat <(echo ready) -", "ready")?; - p.send_control('c')?; // abort: SIGINT - p.wait_for_prompt()?; - p.execute("cat <(echo ready) -", "ready")?; - p.send_control('z')?; // suspend:SIGTSTPcon - p.exp_regex(r"(Stopped|suspended)\s+cat .*")?; - p.send_line("fg")?; - p.execute("cat <(echo ready) -", "ready")?; - p.send_control('c')?; - Ok(()) - }() - .unwrap_or_else(|e| panic!("test_bash_control_chars failed: {}", e)); + fn test_bash_control_chars() -> Result<(), Error> { + let mut p = spawn_bash(Some(1000))?; + p.execute("cat <(echo ready) -", "ready")?; + p.send_control('c')?; // abort: SIGINT + p.wait_for_prompt()?; + p.execute("cat <(echo ready) -", "ready")?; + p.send_control('z')?; // suspend:SIGTSTPcon + p.exp_regex(r"(Stopped|suspended)\s+cat .*")?; + p.send_line("fg")?; + p.execute("cat <(echo ready) -", "ready")?; + p.send_control('c')?; + Ok(()) } #[test]