RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 13:27:35 +00:00
Code Issues Activity Actions

Shell: Don't reset 'last_return_code' before running commands

Browse source 
Some variables depend on its value to function correctly.
Fixes the following issue:
    $ false; echo $?
    1
    $ false
    $ echo $?
    128
This commit is contained in:
Ali Mohammad Pur 2022-01-05 08:00:06 +03:30 • committed by Andreas Kling
parent 689fe7ddff
commit 310a18da1e
4 changed files with 20 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Userland/Shell/AST.cpp
View file

@ -3564,7 +3564,7 @@ Vector<String> SpecialVariableValue::resolve_as_list(RefPtr<Shell> shell)
switch (m_name) { switch (m_name) {
case '?': case '?':
return { resolve_slices(shell, String::number(shell->last_return_code), m_slices) }; return { resolve_slices(shell, String::number(shell->last_return_code.value_or(0)), m_slices) };
case '$': case '$':
return { resolve_slices(shell, String::number(getpid()), m_slices) }; return { resolve_slices(shell, String::number(getpid()), m_slices) };
case '*': case '*':

2
Userland/Shell/Builtin.cpp
View file

@ -1059,7 +1059,7 @@ int Shell::builtin_not(int argc, const char** argv)
} }
// In case it was a function. // In case it was a function.
if (!found_a_job) if (!found_a_job)
exit_code = last_return_code; exit_code = last_return_code.value_or(0);
return exit_code == 0 ? 1 : 0; return exit_code == 0 ? 1 : 0;
} }

32
Userland/Shell/Shell.cpp
View file

@ -469,7 +469,7 @@ bool Shell::invoke_function(const AST::Command& command, int& retval)
(void)function.body->run(*this); (void)function.body->run(*this);
retval = last_return_code; retval = last_return_code.value_or(0);
return true; return true;
} }
@ -536,8 +536,8 @@ int Shell::run_command(StringView cmd, Optional<SourcePosition> source_position_
take_error(); take_error();
// If we end up executing nothing, let's return a failing exit code. if (!last_return_code.has_value())
last_return_code = 128; last_return_code = 0;
ScopedValueRollback source_position_rollback { m_source_position }; ScopedValueRollback source_position_rollback { m_source_position };
if (source_position_override.has_value()) if (source_position_override.has_value())
@ -581,7 +581,7 @@ int Shell::run_command(StringView cmd, Optional<SourcePosition> source_position_
return 1; return 1;
} }
return last_return_code; return last_return_code.value_or(0);
} }
ErrorOr<RefPtr<Job>> Shell::run_command(const AST::Command& command) ErrorOr<RefPtr<Job>> Shell::run_command(const AST::Command& command)
@ -595,7 +595,7 @@ ErrorOr<RefPtr<Job>> Shell::run_command(const AST::Command& command)
if (command.argv.is_empty() && !command.should_immediately_execute_next) { if (command.argv.is_empty() && !command.should_immediately_execute_next) {
m_global_redirections.extend(command.redirections); m_global_redirections.extend(command.redirections);
for (auto& next_in_chain : command.next_chain) for (auto& next_in_chain : command.next_chain)
run_tail(command, next_in_chain, last_return_code); run_tail(command, next_in_chain, last_return_code.value_or(0));
return nullptr; return nullptr;
} }
@ -668,9 +668,10 @@ ErrorOr<RefPtr<Job>> Shell::run_command(const AST::Command& command)
for (auto& redirection : command.redirections) for (auto& redirection : command.redirections)
TRY(resolve_redirection(redirection)); TRY(resolve_redirection(redirection));
if (command.should_wait && run_builtin(command, rewirings, last_return_code)) { if (int local_return_code = 0; command.should_wait && run_builtin(command, rewirings, local_return_code)) {
last_return_code = local_return_code;
for (auto& next_in_chain : command.next_chain) for (auto& next_in_chain : command.next_chain)
run_tail(command, next_in_chain, last_return_code); run_tail(command, next_in_chain, *last_return_code);
return nullptr; return nullptr;
} }
@ -681,9 +682,10 @@ ErrorOr<RefPtr<Job>> Shell::run_command(const AST::Command& command)
for (auto& rewiring : rewirings) for (auto& rewiring : rewirings)
TRY(Core::System::dup2(rewiring.old_fd, rewiring.new_fd)); TRY(Core::System::dup2(rewiring.old_fd, rewiring.new_fd));
if (invoke_function(command, last_return_code)) { if (int local_return_code = 0; invoke_function(command, local_return_code)) {
last_return_code = local_return_code;
for (auto& next_in_chain : command.next_chain) for (auto& next_in_chain : command.next_chain)
run_tail(command, next_in_chain, last_return_code); run_tail(command, next_in_chain, *last_return_code);
return nullptr; return nullptr;
} }
} }
@ -695,7 +697,7 @@ ErrorOr<RefPtr<Job>> Shell::run_command(const AST::Command& command)
&& command.next_chain.first().node->should_override_execution_in_current_process()) { && command.next_chain.first().node->should_override_execution_in_current_process()) {
for (auto& next_in_chain : command.next_chain) for (auto& next_in_chain : command.next_chain)
run_tail(command, next_in_chain, last_return_code); run_tail(command, next_in_chain, last_return_code.value_or(0));
return nullptr; return nullptr;
} }
@ -752,14 +754,14 @@ ErrorOr<RefPtr<Job>> Shell::run_command(const AST::Command& command)
for (auto& next_in_chain : command.next_chain) for (auto& next_in_chain : command.next_chain)
run_tail(command, next_in_chain, 0); run_tail(command, next_in_chain, 0);
_exit(last_return_code); _exit(last_return_code.value_or(0));
} }
if (run_builtin(command, {}, last_return_code)) if (int local_return_code = 0; run_builtin(command, {}, local_return_code))
_exit(last_return_code); _exit(local_return_code);
if (invoke_function(command, last_return_code)) if (int local_return_code = 0; invoke_function(command, local_return_code))
_exit(last_return_code); _exit(local_return_code);
// We no longer need the jobs here. // We no longer need the jobs here.
jobs.clear(); jobs.clear();

2
Userland/Shell/Shell.h
View file

@ -212,7 +212,7 @@ public:
char hostname[HostNameSize]; char hostname[HostNameSize];
uid_t uid; uid_t uid;
int last_return_code { 0 }; Optional<int> last_return_code;
Vector<String> directory_stack; Vector<String> directory_stack;
CircularQueue<String, 8> cd_history; // FIXME: have a configurable cd history length CircularQueue<String, 8> cd_history; // FIXME: have a configurable cd history length
HashMap<u64, NonnullRefPtr<Job>> jobs; HashMap<u64, NonnullRefPtr<Job>> jobs;