Skip to content

Commit 6c16479

Browse files
ekarlsnekarlsn
committed
Remove unnecessary immediately invoked closures
Signed-off-by: Erik Karlsson <triggger@gmail.com>
1 parent 11f81df commit 6c16479

File tree

3 files changed

+132
-166
lines changed

3 files changed

+132
-166
lines changed

‎src/process.rs‎

Lines changed: 48 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -88,47 +88,44 @@ fn ptsname_r(fd: &PtyMaster) -> nix::Result<String> {
8888
impl PtyProcess {
8989
/// Start a process in a forked pty
9090
pub fn new(mut command: Command) -> Result<Self, Error> {
91-
|| -> nix::Result<Self> {
92-
// Open a new PTY master
93-
let master_fd = posix_openpt(OFlag::O_RDWR)?;
91+
// Open a new PTY master
92+
let master_fd = posix_openpt(OFlag::O_RDWR)?;
9493

95-
// Allow a slave to be generated for it
96-
grantpt(&master_fd)?;
97-
unlockpt(&master_fd)?;
94+
// Allow a slave to be generated for it
95+
grantpt(&master_fd)?;
96+
unlockpt(&master_fd)?;
9897

99-
// on Linux this is the libc function, on OSX this is our implementation of ptsname_r
100-
let slave_name = ptsname_r(&master_fd)?;
98+
// on Linux this is the libc function, on OSX this is our implementation of ptsname_r
99+
let slave_name = ptsname_r(&master_fd)?;
101100

102-
match fork()? {
103-
ForkResult::Child => {
104-
setsid()?; // create new session with child as session leader
105-
let slave_fd = open(
106-
std::path::Path::new(&slave_name),
107-
OFlag::O_RDWR,
108-
stat::Mode::empty(),
109-
)?;
101+
match fork()? {
102+
ForkResult::Child => {
103+
setsid()?; // create new session with child as session leader
104+
let slave_fd = open(
105+
std::path::Path::new(&slave_name),
106+
OFlag::O_RDWR,
107+
stat::Mode::empty(),
108+
)?;
110109

111-
// assign stdin, stdout, stderr to the tty, just like a terminal does
112-
dup2(slave_fd, STDIN_FILENO)?;
113-
dup2(slave_fd, STDOUT_FILENO)?;
114-
dup2(slave_fd, STDERR_FILENO)?;
110+
// assign stdin, stdout, stderr to the tty, just like a terminal does
111+
dup2(slave_fd, STDIN_FILENO)?;
112+
dup2(slave_fd, STDOUT_FILENO)?;
113+
dup2(slave_fd, STDERR_FILENO)?;
115114

116-
// set echo off
117-
let mut flags = termios::tcgetattr(STDIN_FILENO)?;
118-
flags.local_flags &= !termios::LocalFlags::ECHO;
119-
termios::tcsetattr(STDIN_FILENO, termios::SetArg::TCSANOW, &flags)?;
115+
// set echo off
116+
let mut flags = termios::tcgetattr(STDIN_FILENO)?;
117+
flags.local_flags &= !termios::LocalFlags::ECHO;
118+
termios::tcsetattr(STDIN_FILENO, termios::SetArg::TCSANOW, &flags)?;
120119

121-
command.exec();
122-
Err(nix::Error::last())
123-
}
124-
ForkResult::Parent { child: child_pid } => Ok(PtyProcess {
125-
pty: master_fd,
126-
child_pid,
127-
kill_timeout: None,
128-
}),
120+
command.exec();
121+
Err(Error::Nix(nix::Error::last()))
129122
}
130-
}()
131-
.map_err(Error::from)
123+
ForkResult::Parent { child: child_pid } => Ok(PtyProcess {
124+
pty: master_fd,
125+
child_pid,
126+
kill_timeout: None,
127+
}),
128+
}
132129
}
133130

134131
/// Get handle to pty fork for reading/writing
@@ -242,28 +239,23 @@ mod tests {
242239

243240
#[test]
244241
/// Open cat, write string, read back string twice, send Ctrl^C and check that cat exited
245-
fn test_cat() {
246-
// wrapping into closure so I can use ?
247-
|| -> std::io::Result<()> {
248-
let process = PtyProcess::new(Command::new("cat")).expect("could not execute cat");
249-
let f = process.get_file_handle();
250-
let mut writer = LineWriter::new(&f);
251-
let mut reader = BufReader::new(&f);
252-
let _ = writer.write(b"hello cat\n")?;
253-
let mut buf = String::new();
254-
reader.read_line(&mut buf)?;
255-
assert_eq!(buf, "hello cat\r\n");
242+
fn test_cat() -> std::io::Result<()> {
243+
let process = PtyProcess::new(Command::new("cat")).expect("could not execute cat");
244+
let f = process.get_file_handle();
245+
let mut writer = LineWriter::new(&f);
246+
let mut reader = BufReader::new(&f);
247+
let _ = writer.write(b"hello cat\n")?;
248+
let mut buf = String::new();
249+
reader.read_line(&mut buf)?;
250+
assert_eq!(buf, "hello cat\r\n");
256251

257-
// this sleep solves an edge case of some cases when cat is somehow not "ready"
258-
// to take the ^C (occasional test hangs)
259-
thread::sleep(time::Duration::from_millis(100));
260-
writer.write_all(&[3])?; // send ^C
261-
writer.flush()?;
262-
let should =
263-
wait::WaitStatus::Signaled(process.child_pid, signal::Signal::SIGINT, false);
264-
assert_eq!(should, wait::waitpid(process.child_pid, None).unwrap());
265-
Ok(())
266-
}()
267-
.unwrap_or_else(|e| panic!("test_cat failed: {}", e));
252+
// this sleep solves an edge case of some cases when cat is somehow not "ready"
253+
// to take the ^C (occasional test hangs)
254+
thread::sleep(time::Duration::from_millis(100));
255+
writer.write_all(&[3])?; // send ^C
256+
writer.flush()?;
257+
let should = wait::WaitStatus::Signaled(process.child_pid, signal::Signal::SIGINT, false);
258+
assert_eq!(should, wait::waitpid(process.child_pid, None).unwrap());
259+
Ok(())
268260
}
269261
}

‎src/reader.rs‎

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -123,29 +123,27 @@ impl NBReader {
123123
let (tx, rx) = channel();
124124

125125
// spawn a thread which reads one char and sends it to tx
126-
thread::spawn(move || {
127-
let _ = || -> Result<(), Error> {
128-
let mut reader = BufReader::new(f);
129-
let mut byte = [0u8];
130-
loop {
131-
match reader.read(&mut byte) {
132-
Ok(0) => {
133-
tx.send(Ok(PipedChar::EOF))
134-
.map_err(|_| Error::MpscSendError)?;
135-
break;
136-
}
137-
Ok(_) => {
138-
tx.send(Ok(PipedChar::Char(byte[0])))
139-
.map_err(|_| Error::MpscSendError)?;
140-
}
141-
Err(error) => {
142-
tx.send(Err(PipeError::IO(error)))
143-
.map_err(|_| Error::MpscSendError)?;
144-
}
126+
thread::spawn(move || -> Result<(), Error> {
127+
let mut reader = BufReader::new(f);
128+
let mut byte = [0u8];
129+
loop {
130+
match reader.read(&mut byte) {
131+
Ok(0) => {
132+
tx.send(Ok(PipedChar::EOF))
133+
.map_err(|_| Error::MpscSendError)?;
134+
break;
135+
}
136+
Ok(_) => {
137+
tx.send(Ok(PipedChar::Char(byte[0])))
138+
.map_err(|_| Error::MpscSendError)?;
139+
}
140+
Err(error) => {
141+
tx.send(Err(PipeError::IO(error)))
142+
.map_err(|_| Error::MpscSendError)?;
145143
}
146144
}
147-
Ok(())
148-
}();
145+
}
146+
Ok(())
149147
// don't do error handling as on an error it was most probably
150148
// the main thread which exited (remote hangup)
151149
});

‎src/session.rs‎

Lines changed: 65 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -431,34 +431,28 @@ mod tests {
431431
use super::*;
432432

433433
#[test]
434-
fn test_read_line() {
435-
|| -> Result<(), Error> {
436-
let mut s = spawn("cat", Some(1000))?;
437-
s.send_line("hans")?;
438-
assert_eq!("hans", s.read_line()?);
439-
let should = crate::process::wait::WaitStatus::Signaled(
440-
s.process.child_pid,
441-
crate::process::signal::Signal::SIGTERM,
442-
false,
443-
);
444-
assert_eq!(should, s.process.exit()?);
445-
Ok(())
446-
}()
447-
.unwrap_or_else(|e| panic!("test_read_line failed: {}", e));
434+
fn test_read_line() -> Result<(), Error> {
435+
let mut s = spawn("cat", Some(1000))?;
436+
s.send_line("hans")?;
437+
assert_eq!("hans", s.read_line()?);
438+
let should = crate::process::wait::WaitStatus::Signaled(
439+
s.process.child_pid,
440+
crate::process::signal::Signal::SIGTERM,
441+
false,
442+
);
443+
assert_eq!(should, s.process.exit()?);
444+
Ok(())
448445
}
449446

450447
#[test]
451-
fn test_expect_eof_timeout() {
452-
|| -> Result<(), Error> {
453-
let mut p = spawn("sleep 3", Some(1000)).expect("cannot run sleep 3");
454-
match p.exp_eof() {
455-
Ok(_) => panic!("should raise Timeout"),
456-
Err(Error::Timeout { .. }) => {}
457-
Err(_) => panic!("should raise TimeOut"),
458-
}
459-
Ok(())
460-
}()
461-
.unwrap_or_else(|e| panic!("test_timeout failed: {}", e));
448+
fn test_expect_eof_timeout() -> Result<(), Error> {
449+
let mut p = spawn("sleep 3", Some(1000)).expect("cannot run sleep 3");
450+
match p.exp_eof() {
451+
Ok(_) => panic!("should raise Timeout"),
452+
Err(Error::Timeout { .. }) => {}
453+
Err(_) => panic!("should raise TimeOut"),
454+
}
455+
Ok(())
462456
}
463457

464458
#[test]
@@ -468,44 +462,35 @@ mod tests {
468462
}
469463

470464
#[test]
471-
fn test_expect_string() {
472-
|| -> Result<(), Error> {
473-
let mut p = spawn("cat", Some(1000)).expect("cannot run cat");
474-
p.send_line("hello world!")?;
475-
p.exp_string("hello world!")?;
476-
p.send_line("hello heaven!")?;
477-
p.exp_string("hello heaven!")?;
478-
Ok(())
479-
}()
480-
.unwrap_or_else(|e| panic!("test_expect_string failed: {}", e));
465+
fn test_expect_string() -> Result<(), Error> {
466+
let mut p = spawn("cat", Some(1000)).expect("cannot run cat");
467+
p.send_line("hello world!")?;
468+
p.exp_string("hello world!")?;
469+
p.send_line("hello heaven!")?;
470+
p.exp_string("hello heaven!")?;
471+
Ok(())
481472
}
482473

483474
#[test]
484-
fn test_read_string_before() {
485-
|| -> Result<(), Error> {
486-
let mut p = spawn("cat", Some(1000)).expect("cannot run cat");
487-
p.send_line("lorem ipsum dolor sit amet")?;
488-
assert_eq!("lorem ipsum dolor sit ", p.exp_string("amet")?);
489-
Ok(())
490-
}()
491-
.unwrap_or_else(|e| panic!("test_read_string_before failed: {}", e));
475+
fn test_read_string_before() -> Result<(), Error> {
476+
let mut p = spawn("cat", Some(1000)).expect("cannot run cat");
477+
p.send_line("lorem ipsum dolor sit amet")?;
478+
assert_eq!("lorem ipsum dolor sit ", p.exp_string("amet")?);
479+
Ok(())
492480
}
493481

494482
#[test]
495-
fn test_expect_any() {
496-
|| -> Result<(), Error> {
497-
let mut p = spawn("cat", Some(1000)).expect("cannot run cat");
498-
p.send_line("Hi")?;
499-
match p.exp_any(vec![
500-
ReadUntil::NBytes(3),
501-
ReadUntil::String("Hi".to_string()),
502-
]) {
503-
Ok(s) => assert_eq!(("".to_string(), "Hi".to_string()), s),
504-
Err(e) => panic!("got error: {}", e),
505-
}
506-
Ok(())
507-
}()
508-
.unwrap_or_else(|e| panic!("test_expect_any failed: {}", e));
483+
fn test_expect_any() -> Result<(), Error> {
484+
let mut p = spawn("cat", Some(1000)).expect("cannot run cat");
485+
p.send_line("Hi")?;
486+
match p.exp_any(vec![
487+
ReadUntil::NBytes(3),
488+
ReadUntil::String("Hi".to_string()),
489+
]) {
490+
Ok(s) => assert_eq!(("".to_string(), "Hi".to_string()), s),
491+
Err(e) => panic!("got error: {}", e),
492+
}
493+
Ok(())
509494
}
510495

511496
#[test]
@@ -519,46 +504,37 @@ mod tests {
519504
}
520505

521506
#[test]
522-
fn test_kill_timeout() {
523-
|| -> Result<(), Error> {
524-
let mut p = spawn_bash(Some(1000))?;
525-
p.execute("cat <(echo ready) -", "ready")?;
526-
Ok(())
527-
}()
528-
.unwrap_or_else(|e| panic!("test_kill_timeout failed: {}", e));
507+
fn test_kill_timeout() -> Result<(), Error> {
508+
let mut p = spawn_bash(Some(1000))?;
509+
p.execute("cat <(echo ready) -", "ready")?;
510+
Ok(())
529511
// p is dropped here and kill is sent immediately to bash
530512
// Since that is not enough to make bash exit, a kill -9 is sent within 1s (timeout)
531513
}
532514

533515
#[test]
534-
fn test_bash() {
535-
|| -> Result<(), Error> {
536-
let mut p = spawn_bash(Some(1000))?;
537-
p.send_line("cd /tmp/")?;
538-
p.wait_for_prompt()?;
539-
p.send_line("pwd")?;
540-
assert_eq!("/tmp\r\n", p.wait_for_prompt()?);
541-
Ok(())
542-
}()
543-
.unwrap_or_else(|e| panic!("test_bash failed: {}", e));
516+
fn test_bash() -> Result<(), Error> {
517+
let mut p = spawn_bash(Some(1000))?;
518+
p.send_line("cd /tmp/")?;
519+
p.wait_for_prompt()?;
520+
p.send_line("pwd")?;
521+
assert_eq!("/tmp\r\n", p.wait_for_prompt()?);
522+
Ok(())
544523
}
545524

546525
#[test]
547-
fn test_bash_control_chars() {
548-
|| -> Result<(), Error> {
549-
let mut p = spawn_bash(Some(1000))?;
550-
p.execute("cat <(echo ready) -", "ready")?;
551-
p.send_control('c')?; // abort: SIGINT
552-
p.wait_for_prompt()?;
553-
p.execute("cat <(echo ready) -", "ready")?;
554-
p.send_control('z')?; // suspend:SIGTSTPcon
555-
p.exp_regex(r"(Stopped|suspended)\s+cat .*")?;
556-
p.send_line("fg")?;
557-
p.execute("cat <(echo ready) -", "ready")?;
558-
p.send_control('c')?;
559-
Ok(())
560-
}()
561-
.unwrap_or_else(|e| panic!("test_bash_control_chars failed: {}", e));
526+
fn test_bash_control_chars() -> Result<(), Error> {
527+
let mut p = spawn_bash(Some(1000))?;
528+
p.execute("cat <(echo ready) -", "ready")?;
529+
p.send_control('c')?; // abort: SIGINT
530+
p.wait_for_prompt()?;
531+
p.execute("cat <(echo ready) -", "ready")?;
532+
p.send_control('z')?; // suspend:SIGTSTPcon
533+
p.exp_regex(r"(Stopped|suspended)\s+cat .*")?;
534+
p.send_line("fg")?;
535+
p.execute("cat <(echo ready) -", "ready")?;
536+
p.send_control('c')?;
537+
Ok(())
562538
}
563539

564540
#[test]

0 commit comments

Comments
 (0)