From 165c82e68ae16082b892a7d406d226ff8b6453f4 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 24 Mar 2019 12:29:25 -0700 Subject: [PATCH] Promote process_type_t to an enum class --- src/exec.cpp | 31 ++++++++++++++++--------------- src/parse_execution.cpp | 32 ++++++++++++++++---------------- src/proc.cpp | 2 +- src/proc.h | 34 +++++++++++++++++----------------- 4 files changed, 50 insertions(+), 49 deletions(-) diff --git a/src/exec.cpp b/src/exec.cpp index b93c78f02..d22e81334 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -488,7 +488,7 @@ static bool fork_child_for_process(const std::shared_ptr &job, process_t static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptr &j, process_t *p, const io_pipe_t *pipe_read, const io_chain_t &proc_io_chain, io_streams_t &streams) { - assert(p->type == INTERNAL_BUILTIN && "Process must be a builtin"); + assert(p->type == process_type_t::builtin && "Process must be a builtin"); int local_builtin_stdin = STDIN_FILENO; autoclose_fd_t locally_opened_stdin{}; @@ -590,7 +590,7 @@ static bool exec_internal_builtin_proc(parser_t &parser, const std::shared_ptr &j, process_t *p, io_chain_t *io_chain, const io_streams_t &builtin_io_streams) { - assert(p->type == INTERNAL_BUILTIN && "Process is not a builtin"); + assert(p->type == process_type_t::builtin && "Process is not a builtin"); const output_stream_t &stdout_stream = builtin_io_streams.out; const output_stream_t &stderr_stream = builtin_io_streams.err; @@ -677,7 +677,7 @@ static bool handle_builtin_output(const std::shared_ptr &j, process_t *p, /// \return true on success, false if there is an exec error. static bool exec_external_command(env_stack_t &vars, const std::shared_ptr &j, process_t *p, const io_chain_t &proc_io_chain) { - assert(p->type == EXTERNAL && "Process is not external"); + assert(p->type == process_type_t::external && "Process is not external"); // Get argv and envv before we fork. null_terminated_array_t argv_array; convert_wide_array_to_narrow(p->get_argv_array(), &argv_array); @@ -786,7 +786,7 @@ static bool exec_external_command(env_stack_t &vars, const std::shared_ptr j, process_t *p, const io_chain_t &user_ios, io_chain_t io_chain) { - assert((p->type == INTERNAL_FUNCTION || p->type == INTERNAL_BLOCK_NODE) && + assert((p->type == process_type_t::function || p->type == process_type_t::block_node) && "Unexpected process type"); // Create an output buffer if we're piping to another process. @@ -802,7 +802,7 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr io_chain.push_back(block_output_bufferfill); } - if (p->type == INTERNAL_FUNCTION) { + if (p->type == process_type_t::function) { const wcstring func_name = p->argv0(); auto props = function_get_properties(func_name); if (!props) { @@ -822,7 +822,7 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr parser.allow_function(); parser.pop_block(fb); } else { - assert(p->type == INTERNAL_BLOCK_NODE); + assert(p->type == process_type_t::block_node); assert(p->block_node_source && p->internal_block_node && "Process is missing node info"); internal_exec_helper(parser, p->block_node_source, p->internal_block_node, io_chain, j); } @@ -939,7 +939,7 @@ static bool exec_process_in_job(parser_t &parser, process_t *p, std::shared_ptr< // to generate it, since that result would not get written back to the parent. This call // could be safely removed, but it would result in slightly lower performance - at least on // uniprocessor systems. - if (p->type == EXTERNAL) { + if (p->type == process_type_t::external) { // Apply universal barrier so we have the most recent uvar changes if (!get_proc_had_barrier()) { set_proc_had_barrier(true); @@ -951,15 +951,15 @@ static bool exec_process_in_job(parser_t &parser, process_t *p, std::shared_ptr< // Execute the process. p->check_generations_before_launch(); switch (p->type) { - case INTERNAL_FUNCTION: - case INTERNAL_BLOCK_NODE: { + case process_type_t::function: + case process_type_t::block_node: { if (!exec_block_or_func_process(parser, j, p, all_ios, process_net_io_chain)) { return false; } break; } - case INTERNAL_BUILTIN: { + case process_type_t::builtin: { io_streams_t builtin_io_streams{stdout_read_limit}; if (!exec_internal_builtin_proc(parser, j, p, pipe_read.get(), process_net_io_chain, builtin_io_streams)) { @@ -971,16 +971,17 @@ static bool exec_process_in_job(parser_t &parser, process_t *p, std::shared_ptr< break; } - case EXTERNAL: { + case process_type_t::external: { if (!exec_external_command(parser.vars(), j, p, process_net_io_chain)) { return false; } break; } - case INTERNAL_EXEC: { + case process_type_t::exec: { // We should have handled exec up above. - DIE("INTERNAL_EXEC process found in pipeline, where it should never be. Aborting."); + DIE("process_type_t::exec process found in pipeline, where it should never be. " + "Aborting."); break; } } @@ -1002,7 +1003,7 @@ bool exec_job(parser_t &parser, shared_ptr j) { const std::shared_ptr parent_job = j->get_parent(); // Perhaps inherit our parent's pgid and job control flag. - if (parent_job && j->processes.front()->type == EXTERNAL) { + if (parent_job && j->processes.front()->type == process_type_t::external) { if (parent_job->pgid != INVALID_PID) { j->pgid = parent_job->pgid; j->set_flag(job_flag_t::JOB_CONTROL, true); @@ -1019,7 +1020,7 @@ bool exec_job(parser_t &parser, shared_ptr j) { } } - if (j->processes.front()->type == INTERNAL_EXEC) { + if (j->processes.front()->type == process_type_t::exec) { internal_exec(parser.vars(), j.get(), all_ios); // internal_exec only returns if it failed to set up redirections. // In case of an successful exec, this code is not reached. diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 85de38ab8..2034f0d56 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -143,9 +143,9 @@ tnode_t parse_execution_context_t::infinite_recursive_statem return infinite_recursive_statement; } -enum process_type_t parse_execution_context_t::process_type_for_command( +process_type_t parse_execution_context_t::process_type_for_command( tnode_t statement, const wcstring &cmd) const { - enum process_type_t process_type = EXTERNAL; + enum process_type_t process_type = process_type_t::external; // Determine the process type, which depends on the statement decoration (command, builtin, // etc). @@ -153,19 +153,19 @@ enum process_type_t parse_execution_context_t::process_type_for_command( if (decoration == parse_statement_decoration_exec) { // Always exec. - process_type = INTERNAL_EXEC; + process_type = process_type_t::exec; } else if (decoration == parse_statement_decoration_command) { // Always a command. - process_type = EXTERNAL; + process_type = process_type_t::external; } else if (decoration == parse_statement_decoration_builtin) { // What happens if this builtin is not valid? - process_type = INTERNAL_BUILTIN; + process_type = process_type_t::builtin; } else if (function_exists(cmd)) { - process_type = INTERNAL_FUNCTION; + process_type = process_type_t::function; } else if (builtin_exists(cmd)) { - process_type = INTERNAL_BUILTIN; + process_type = process_type_t::builtin; } else { - process_type = EXTERNAL; + process_type = process_type_t::external; } return process_type; } @@ -775,7 +775,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process( enum process_type_t process_type = process_type_for_command(statement, cmd); // Check for stack overflow. - if (process_type == INTERNAL_FUNCTION && + if (process_type == process_type_t::function && parser->forbidden_function.size() > FISH_MAX_STACK_DEPTH) { this->report_error(statement, CALL_STACK_LIMIT_EXCEEDED_ERR_MSG); return parse_execution_errored; @@ -783,7 +783,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process( // Protect against exec with background processes running static uint32_t last_exec_run_counter = -1; - if (process_type == INTERNAL_EXEC && shell_is_interactive()) { + if (process_type == process_type_t::exec && shell_is_interactive()) { job_iterator_t jobs; bool have_bg = false; const job_t *bg = nullptr; @@ -811,7 +811,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process( } wcstring path_to_external_command; - if (process_type == EXTERNAL || process_type == INTERNAL_EXEC) { + if (process_type == process_type_t::external || process_type == process_type_t::exec) { // Determine the actual command. This may be an implicit cd. bool has_command = path_get_path(cmd, &path_to_external_command, parser->vars()); @@ -847,7 +847,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process( path_to_external_command.clear(); // If we have defined a wrapper around cd, use it, otherwise use the cd builtin. - process_type = function_exists(L"cd") ? INTERNAL_FUNCTION : INTERNAL_BUILTIN; + process_type = function_exists(L"cd") ? process_type_t::function : process_type_t::builtin; } else { // Not implicit cd. const globspec_t glob_behavior = (cmd == L"set" || cmd == L"count") ? nullglob : failglob; @@ -1006,8 +1006,8 @@ template parse_execution_result_t parse_execution_context_t::populate_block_process( job_t *job, process_t *proc, tnode_t statement, tnode_t specific_statement) { - // We handle block statements by creating INTERNAL_BLOCK_NODE, that will bounce back to us when - // it's time to execute them. + // We handle block statements by creating process_type_t::block_node, that will bounce back to + // us when it's time to execute them. UNUSED(job); static_assert(Type::token == symbol_block_statement || Type::token == symbol_if_statement || Type::token == symbol_switch_statement, @@ -1022,7 +1022,7 @@ parse_execution_result_t parse_execution_context_t::populate_block_process( bool errored = !this->determine_io_chain(arguments, &process_io_chain); if (errored) return parse_execution_errored; - proc->type = INTERNAL_BLOCK_NODE; + proc->type = process_type_t::block_node; proc->block_node_source = pstree; proc->internal_block_node = statement; proc->set_io_chain(process_io_chain); @@ -1245,7 +1245,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(tnode_t jo // Check to see if this contained any external commands. bool job_contained_external_command = false; for (const auto &proc : job->processes) { - if (proc->type == EXTERNAL) { + if (proc->type == process_type_t::external) { job_contained_external_command = true; break; } diff --git a/src/proc.cpp b/src/proc.cpp index df60dd461..119bbfeee 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -917,7 +917,7 @@ void proc_sanity_check() { for (const process_ptr_t &p : j->processes) { // Internal block nodes do not have argv - see issue #1545. - bool null_ok = (p->type == INTERNAL_BLOCK_NODE); + bool null_ok = (p->type == process_type_t::block_node); validate_pointer(p->get_argv(), _(L"Process argument list"), null_ok); validate_pointer(p->argv0(), _(L"Process name"), null_ok); diff --git a/src/proc.h b/src/proc.h index b161589ab..716fd42f7 100644 --- a/src/proc.h +++ b/src/proc.h @@ -25,17 +25,17 @@ #include "topic_monitor.h" /// Types of processes. -enum process_type_t { +enum class process_type_t { /// A regular external command. - EXTERNAL, + external, /// A builtin command. - INTERNAL_BUILTIN, + builtin, /// A shellscript function. - INTERNAL_FUNCTION, + function, /// A block of commands, represented as a node. - INTERNAL_BLOCK_NODE, + block_node, /// The exec builtin. - INTERNAL_EXEC + exec, }; enum class job_control_t { @@ -137,21 +137,22 @@ class internal_proc_t { /// process, an internal builtin which may or may not spawn a fake IO process during execution, a /// shellscript function or a block of commands to be evaluated by calling eval. Lastly, this /// process can be the result of an exec command. The role of this process_t is determined by the -/// type field, which can be one of EXTERNAL, INTERNAL_BUILTIN, INTERNAL_FUNCTION, INTERNAL_EXEC. +/// type field, which can be one of process_type_t::external, process_type_t::builtin, +/// process_type_t::function, process_type_t::exec. /// /// The process_t contains information on how the process should be started, such as command name /// and arguments, as well as runtime information on the status of the actual physical process which /// represents it. Shellscript functions, builtins and blocks of code may all need to spawn an /// external process that handles the piping and redirecting of IO for them. /// -/// If the process is of type EXTERNAL or INTERNAL_EXEC, argv is the argument array and actual_cmd -/// is the absolute path of the command to execute. +/// If the process is of type process_type_t::external or process_type_t::exec, argv is the argument +/// array and actual_cmd is the absolute path of the command to execute. /// -/// If the process is of type INTERNAL_BUILTIN, argv is the argument vector, and argv[0] is the name -/// of the builtin command. +/// If the process is of type process_type_t::builtin, argv is the argument vector, and argv[0] is +/// the name of the builtin command. /// -/// If the process is of type INTERNAL_FUNCTION, argv is the argument vector, and argv[0] is the -/// name of the shellscript function. +/// If the process is of type process_type_t::function, argv is the argument vector, and argv[0] is +/// the name of the shellscript function. class process_t { private: null_terminated_array_t argv_array; @@ -169,9 +170,8 @@ class process_t { bool is_first_in_job{false}; bool is_last_in_job{false}; - /// Type of process. Can be one of \c EXTERNAL, \c INTERNAL_BUILTIN, \c INTERNAL_FUNCTION, \c - /// INTERNAL_EXEC. - enum process_type_t type { EXTERNAL }; + /// Type of process. + process_type_t type{process_type_t::external}; /// For internal block processes only, the node offset of the statement. /// This is always either block, ifs, or switchs, never boolean or decorated. @@ -208,7 +208,7 @@ class process_t { /// launch. This helps us avoid spurious waitpid calls. void check_generations_before_launch(); - /// Actual command to pass to exec in case of EXTERNAL or INTERNAL_EXEC. + /// Actual command to pass to exec in case of process_type_t::external or process_type_t::exec. wcstring actual_cmd; /// Generation counts for reaping.