Skip to content

Commit

Permalink
Return ProcessResult from Subshell::start_and_wait
Browse files Browse the repository at this point in the history
This commit changes the return type of Subshell::start_and_wait from
(Result<(Pid, ProcessState), Errno>) to (Result<(Pid, ProcessResult),
Errno). The new type is more precise since the return value cannot be
ProcessState::Running. This change removes the falsely fallible
conversion from ProcessState to ExitStatus in the calling functions.
  • Loading branch information
magicant committed May 23, 2024
1 parent e280478 commit 317f825
Show file tree
Hide file tree
Showing 6 changed files with 58 additions and 59 deletions.
55 changes: 28 additions & 27 deletions yash-builtin/src/fg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ mod tests {
use std::cell::Cell;
use std::rc::Rc;
use yash_env::job::Job;
use yash_env::job::ProcessResult;
use yash_env::option::Option::Monitor;
use yash_env::option::State::On;
use yash_env::subshell::JobControl;
Expand Down Expand Up @@ -235,11 +236,11 @@ mod tests {
})
})
.job_control(JobControl::Foreground);
let (pid, subshell_state) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_state, ProcessState::stopped(Signal::SIGSTOP));
let (pid, subshell_result) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_result, ProcessResult::Stopped(Signal::SIGSTOP));
let mut job = Job::new(pid);
job.job_controlled = true;
job.state = subshell_state;
job.state = subshell_result.into();
let index = env.jobs.add(job);

resume_job_by_index(&mut env, index).await.unwrap();
Expand All @@ -255,11 +256,11 @@ mod tests {
env.options.set(Monitor, On);
let subshell =
Subshell::new(|env, _| Box::pin(suspend(env))).job_control(JobControl::Foreground);
let (pid, subshell_state) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_state, ProcessState::stopped(Signal::SIGSTOP));
let (pid, subshell_result) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_result, ProcessResult::Stopped(Signal::SIGSTOP));
let mut job = Job::new(pid);
job.job_controlled = true;
job.state = subshell_state;
job.state = subshell_result.into();
"my job name".clone_into(&mut job.name);
let index = env.jobs.add(job);

Expand All @@ -281,11 +282,11 @@ mod tests {
})
})
.job_control(JobControl::Foreground);
let (pid, subshell_state) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_state, ProcessState::stopped(Signal::SIGSTOP));
let (pid, subshell_result) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_result, ProcessResult::Stopped(Signal::SIGSTOP));
let mut job = Job::new(pid);
job.job_controlled = true;
job.state = subshell_state;
job.state = subshell_result.into();
let index = env.jobs.add(job);

let result = resume_job_by_index(&mut env, index).await.unwrap();
Expand All @@ -311,11 +312,11 @@ mod tests {
})
})
.job_control(JobControl::Foreground);
let (pid, subshell_state) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_state, ProcessState::stopped(Signal::SIGSTOP));
let (pid, subshell_result) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_result, ProcessResult::Stopped(Signal::SIGSTOP));
let mut job = Job::new(pid);
job.job_controlled = true;
job.state = subshell_state;
job.state = subshell_result.into();
let index = env.jobs.add(job);

let result = resume_job_by_index(&mut env, index).await.unwrap();
Expand All @@ -335,11 +336,11 @@ mod tests {
env.options.set(Monitor, On);
let subshell =
Subshell::new(|env, _| Box::pin(suspend(env))).job_control(JobControl::Foreground);
let (pid, subshell_state) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_state, ProcessState::stopped(Signal::SIGSTOP));
let (pid, subshell_result) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_result, ProcessResult::Stopped(Signal::SIGSTOP));
let mut job = Job::new(pid);
job.job_controlled = true;
job.state = subshell_state;
job.state = subshell_result.into();
let index = env.jobs.add(job);

_ = resume_job_by_index(&mut env, index).await.unwrap();
Expand Down Expand Up @@ -419,20 +420,20 @@ mod tests {
})
})
.job_control(JobControl::Foreground);
let (pid1, subshell_state1) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_state1, ProcessState::stopped(Signal::SIGSTOP));
let (pid1, subshell_result_1) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_result_1, ProcessResult::Stopped(Signal::SIGSTOP));
let mut job = Job::new(pid1);
job.job_controlled = true;
job.state = subshell_state1;
job.state = subshell_result_1.into();
env.jobs.add(job);
// current job
let subshell =
Subshell::new(|env, _| Box::pin(suspend(env))).job_control(JobControl::Foreground);
let (pid2, subshell_state2) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_state2, ProcessState::stopped(Signal::SIGSTOP));
let (pid2, subshell_result_2) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_result_2, ProcessResult::Stopped(Signal::SIGSTOP));
let mut job = Job::new(pid2);
job.job_controlled = true;
job.state = subshell_state2;
job.state = subshell_result_2.into();
let index2 = env.jobs.add(job);
env.jobs.set_current_job(index2).unwrap();

Expand Down Expand Up @@ -468,11 +469,11 @@ mod tests {
// previous job
let subshell =
Subshell::new(|env, _| Box::pin(suspend(env))).job_control(JobControl::Foreground);
let (pid1, subshell_state1) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_state1, ProcessState::stopped(Signal::SIGSTOP));
let (pid1, subshell_result_1) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_result_1, ProcessResult::Stopped(Signal::SIGSTOP));
let mut job = Job::new(pid1);
job.job_controlled = true;
job.state = subshell_state1;
job.state = subshell_result_1.into();
job.name = "previous job".to_string();
let index1 = env.jobs.add(job);
// current job
Expand All @@ -483,11 +484,11 @@ mod tests {
})
})
.job_control(JobControl::Foreground);
let (pid2, subshell_state2) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_state2, ProcessState::stopped(Signal::SIGSTOP));
let (pid2, subshell_result_2) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(subshell_result_2, ProcessResult::Stopped(Signal::SIGSTOP));
let mut job = Job::new(pid2);
job.job_controlled = true;
job.state = subshell_state2;
job.state = subshell_result_2.into();
let index2 = env.jobs.add(job);
env.jobs.set_current_job(index2).unwrap();

Expand Down
30 changes: 14 additions & 16 deletions yash-env/src/subshell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
//! properly.

use crate::job::Pid;
use crate::job::ProcessResult;
use crate::job::ProcessState;
use crate::stack::Frame;
use crate::system::ChildProcessTask;
Expand Down Expand Up @@ -224,9 +225,8 @@ where
/// suspended.
///
/// If the subshell started successfully, the return value is the process ID
/// and the process state of the subshell, which is `Exited`, `Signaled`, or
/// `Stopped`. If there was an error starting the subshell, this function
/// returns the error.
/// and the process result of the subshell. If there was an error starting
/// the subshell, this function returns the error.
///
/// If you set [`job_control`](Self::job_control) to
/// `JobControl::Foreground` and job control is effective as per
Expand All @@ -235,16 +235,14 @@ where
///
/// When a job-controlled subshell suspends, this function does not add it
/// to `env.jobs`. You have to do it for yourself if necessary.
pub async fn start_and_wait(self, env: &mut Env) -> Result<(Pid, ProcessState), Errno> {
pub async fn start_and_wait(self, env: &mut Env) -> Result<(Pid, ProcessResult), Errno> {
let (pid, job_control) = self.start(env).await?;
let result = loop {
let (pid, state) = env.wait_for_subshell(pid).await?;
let is_done = match state {
ProcessState::Running => false,
ProcessState::Halted(result) => !result.is_stopped() || job_control.is_some(),
};
if is_done {
break Ok((pid, state));
let state = env.wait_for_subshell(pid).await?.1;
if let ProcessState::Halted(result) = state {
if !result.is_stopped() || job_control.is_some() {
break result;
}
}
};

Expand All @@ -254,7 +252,7 @@ where
}
}

result
Ok((pid, result))
}
}

Expand Down Expand Up @@ -585,8 +583,8 @@ mod tests {
let subshell = Subshell::new(|env, _job_control| {
Box::pin(async { env.exit_status = ExitStatus(42) })
});
let (_pid, process_state) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(process_state, ProcessState::exited(42));
let (_pid, process_result) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(process_result, ProcessResult::exited(42));
});
}

Expand All @@ -600,8 +598,8 @@ mod tests {
Box::pin(async { env.exit_status = ExitStatus(123) })
})
.job_control(JobControl::Foreground);
let (_pid, process_state) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(process_state, ProcessState::exited(123));
let (_pid, process_result) = subshell.start_and_wait(&mut env).await.unwrap();
assert_eq!(process_result, ProcessResult::exited(123));
assert_eq!(state.borrow().foreground, Some(env.main_pgid));
});
}
Expand Down
8 changes: 4 additions & 4 deletions yash-semantics/src/command/compound_command/subshell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,16 @@ pub async fn execute(env: &mut Env, body: Rc<List>, location: &Location) -> Resu
let subshell = Subshell::new(|sub_env, _job_control| Box::pin(subshell_main(sub_env, body_2)));
let subshell = subshell.job_control(JobControl::Foreground);
match subshell.start_and_wait(env).await {
Ok((pid, state)) => {
if state.is_stopped() {
Ok((pid, result)) => {
if result.is_stopped() {
let mut job = Job::new(pid);
job.job_controlled = true;
job.state = state;
job.state = result.into();
job.name = body.to_string();
env.jobs.add(job);
}

env.exit_status = state.try_into().unwrap();
env.exit_status = result.into();
env.apply_errexit()
}
Err(errno) => {
Expand Down
8 changes: 4 additions & 4 deletions yash-semantics/src/command/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,16 +142,16 @@ async fn execute_job_controlled_pipeline(
.job_control(JobControl::Foreground);

match subshell.start_and_wait(env).await {
Ok((pid, state)) => {
if state.is_stopped() {
Ok((pid, result)) => {
if result.is_stopped() {
let mut job = Job::new(pid);
job.job_controlled = true;
job.state = state;
job.state = result.into();
job.name = to_job_name(commands);
env.jobs.add(job);
}

env.exit_status = state.try_into().unwrap();
env.exit_status = result.into();
Continue(())
}
Err(errno) => {
Expand Down
8 changes: 4 additions & 4 deletions yash-semantics/src/command/simple_command/absent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,19 +68,19 @@ pub async fn execute_absent_target(
.job_control(JobControl::Foreground);

match subshell.start_and_wait(env).await {
Ok((pid, state)) => {
if state.is_stopped() {
Ok((pid, result)) => {
if result.is_stopped() {
let mut job = Job::new(pid);
job.job_controlled = true;
job.state = state;
job.state = result.into();
job.name = redirs
.iter()
.format_with(" ", |redir, f| f(&format_args!("{redir}")))
.to_string();
env.jobs.add(job);
}

state.try_into().unwrap()
result.into()
}
Err(errno) => {
print_error(
Expand Down
8 changes: 4 additions & 4 deletions yash-semantics/src/command/simple_command/external.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,16 @@ pub async fn start_external_utility_in_subshell_and_wait(
.job_control(JobControl::Foreground);

match subshell.start_and_wait(env).await {
Ok((pid, state)) => {
if state.is_stopped() {
Ok((pid, result)) => {
if result.is_stopped() {
let mut job = Job::new(pid);
job.job_controlled = true;
job.state = state;
job.state = result.into();
job.name = job_name;
env.jobs.add(job);
}

state.try_into().unwrap()
result.into()
}
Err(errno) => {
print_error(
Expand Down

0 comments on commit 317f825

Please sign in to comment.