diff --git a/src/builtin_source.cpp b/src/builtin_source.cpp index 7dadc8528..6abb467f0 100644 --- a/src/builtin_source.cpp +++ b/src/builtin_source.cpp @@ -78,7 +78,9 @@ int builtin_source(parser_t &parser, io_streams_t &streams, wchar_t **argv) { // This is slightly subtle. If this is a bare `source` with no args then `argv + optind` already // points to the end of argv. Otherwise we want to skip the file name to get to the args if any. - parser.vars().set_argv(argv + optind + (argc == optind ? 0 : 1)); + wcstring_list_t argv_list = + null_terminated_array_t::to_list(argv + optind + (argc == optind ? 0 : 1)); + parser.vars().set_argv(std::move(argv_list)); retval = reader_read(fd, streams.io_chain ? *streams.io_chain : io_chain_t()); diff --git a/src/common.h b/src/common.h index 4ca30c9db..4a94a4aef 100644 --- a/src/common.h +++ b/src/common.h @@ -584,6 +584,18 @@ class null_terminated_array_t { this->array = make_null_terminated_array(argv); } + /// Convert from a null terminated list to a vector of strings. + static string_list_t to_list(const CharType_t *const *arr) { + string_list_t result; + for (const auto *cursor = arr; cursor && *cursor; cursor++) { + result.push_back(*cursor); + } + return result; + } + + /// Instance method. + string_list_t to_list() const { return to_list(array); } + const CharType_t *const *get() const { return array; } CharType_t **get() { return array; } diff --git a/src/env.cpp b/src/env.cpp index 9deefb77b..bc36d4921 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -1257,17 +1257,7 @@ std::shared_ptr> env_stack_t::export_arr() { std::shared_ptr env_stack_t::snapshot() const { return acquire_impl()->snapshot(); } -void env_stack_t::set_argv(const wchar_t *const *argv) { - if (argv && *argv) { - wcstring_list_t list; - for (auto arg = argv; *arg; arg++) { - list.emplace_back(*arg); - } - set(L"argv", ENV_LOCAL, std::move(list)); - } else { - set_empty(L"argv", ENV_LOCAL); - } -} +void env_stack_t::set_argv(wcstring_list_t argv) { set(L"argv", ENV_LOCAL, std::move(argv)); } void env_stack_t::push(bool new_scope) { auto impl = acquire_impl(); diff --git a/src/env.h b/src/env.h index e310d856f..e51cee02b 100644 --- a/src/env.h +++ b/src/env.h @@ -284,8 +284,8 @@ class env_stack_t final : public environment_t { /// Update the termsize variable. void set_termsize(); - /// Sets up argv as the given null terminated array of strings. - void set_argv(const wchar_t *const *argv); + /// Sets up argv as the given list of strings. + void set_argv(wcstring_list_t argv); /// Mark that exported variables have changed. void mark_changed_exported(); diff --git a/src/exec.cpp b/src/exec.cpp index 067f9eb03..f5937e546 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -817,9 +817,14 @@ static bool exec_block_or_func_process(parser_t &parser, std::shared_ptr const std::map inherit_vars = function_get_inherit_vars(func_name); + // TODO: we want to store the args in both the function block and the environment. + // Find a way to share memory here? + wcstring_list_t argv = p->get_argv_array().to_list(); + // Remove the function name from argv. + if (!argv.empty()) argv.erase(argv.begin()); function_block_t *fb = - parser.push_block(p, func_name, props->shadow_scope); - function_prepare_environment(parser.vars(), func_name, p->get_argv() + 1, inherit_vars); + parser.push_block(func_name, argv, props->shadow_scope); + function_prepare_environment(parser.vars(), func_name, std::move(argv), inherit_vars); parser.forbid_function(func_name); internal_exec_helper(parser, props->parsed_source, props->body_node, io_chain, j); diff --git a/src/function.cpp b/src/function.cpp index 85c24ac0a..cc2f1f0f9 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -371,17 +371,16 @@ void function_invalidate_path() { // 1. argv // 2. named arguments // 3. inherited variables -void function_prepare_environment(env_stack_t &vars, const wcstring &name, - const wchar_t *const *argv, +void function_prepare_environment(env_stack_t &vars, const wcstring &name, wcstring_list_t argv, const std::map &inherited_vars) { vars.set_argv(argv); auto props = function_get_properties(name); if (props && !props->named_arguments.empty()) { - const wchar_t *const *arg = argv; + auto argv_iter = argv.cbegin(); for (const wcstring &named_arg : props->named_arguments) { - if (*arg) { - vars.set_one(named_arg, ENV_LOCAL | ENV_USER, *arg); - arg++; + if (argv_iter != argv.cend()) { + vars.set_one(named_arg, ENV_LOCAL | ENV_USER, std::move(*argv_iter)); + ++argv_iter; } else { vars.set_empty(named_arg, ENV_LOCAL | ENV_USER); } diff --git a/src/function.h b/src/function.h index ad9aee778..dcfb14cb8 100644 --- a/src/function.h +++ b/src/function.h @@ -109,8 +109,7 @@ std::map function_get_inherit_vars(const wcstring &name); bool function_copy(const wcstring &name, const wcstring &new_name); /// Prepares the environment for executing a function. -void function_prepare_environment(env_stack_t &vars, const wcstring &name, - const wchar_t *const *argv, +void function_prepare_environment(env_stack_t &vars, const wcstring &name, wcstring_list_t argv, const std::map &inherited_vars); /// Observes that fish_function_path has changed. diff --git a/src/parser.cpp b/src/parser.cpp index d38c8f301..84e58a8b2 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -368,8 +368,6 @@ void parser_t::stack_trace_internal(size_t block_idx, wcstring *buff) const { if (b->type() == FUNCTION_CALL || b->type() == FUNCTION_CALL_NO_SHADOW || b->type() == SOURCE || b->type() == SUBST) { // These types of blocks should be printed. - int i; - switch (b->type()) { case SOURCE: { const source_block_t *sb = static_cast(b); @@ -382,27 +380,23 @@ void parser_t::stack_trace_internal(size_t block_idx, wcstring *buff) const { case FUNCTION_CALL_NO_SHADOW: { const function_block_t *fb = static_cast(b); append_format(*buff, _(L"in function '%ls'"), fb->name.c_str()); - const process_t *const process = fb->process; // Print arguments on the same line. - if (process->argv(1)) { - wcstring tmp; - - for (i = 1; process->argv(i); i++) { - if (i > 1) tmp.push_back(L' '); - // We can't quote the arguments because we print this in quotes. - // As a special-case, add the empty argument as "". - if (process->argv(i)[0]) { - tmp.append( - escape_string(process->argv(i), ESCAPE_ALL | ESCAPE_NO_QUOTED)); - } else { - tmp.append(L"\"\""); - } + wcstring args_str; + for (const wcstring &arg : fb->args) { + if (!args_str.empty()) args_str.push_back(L' '); + // We can't quote the arguments because we print this in quotes. + // As a special-case, add the empty argument as "". + if (!arg.empty()) { + args_str.append(escape_string(arg, ESCAPE_ALL | ESCAPE_NO_QUOTED)); + } else { + args_str.append(L"\"\""); } - // TODO: Escape these. - append_format(*buff, _(L" with arguments '%ls'\n"), tmp.c_str()); - } else { - buff->append(L"\n"); } + if (!args_str.empty()) { + // TODO: Escape these. + append_format(*buff, _(L" with arguments '%ls'"), args_str.c_str()); + } + buff->push_back('\n'); break; } case SUBST: { @@ -846,8 +840,10 @@ if_block_t::if_block_t() : block_t(IF) {} event_block_t::event_block_t(const event_t &evt) : block_t(EVENT), event(evt) {} -function_block_t::function_block_t(const process_t *p, wcstring n, bool shadows) - : block_t(shadows ? FUNCTION_CALL : FUNCTION_CALL_NO_SHADOW), process(p), name(std::move(n)) {} +function_block_t::function_block_t(wcstring name, wcstring_list_t args, bool shadows) + : block_t(shadows ? FUNCTION_CALL : FUNCTION_CALL_NO_SHADOW), + name(std::move(name)), + args(std::move(args)) {} source_block_t::source_block_t(const wchar_t *src) : block_t(SOURCE), source_file(src) {} diff --git a/src/parser.h b/src/parser.h index e1f1af04e..6e5bad8f3 100644 --- a/src/parser.h +++ b/src/parser.h @@ -96,9 +96,9 @@ struct event_block_t : public block_t { }; struct function_block_t : public block_t { - const process_t *process; wcstring name; - function_block_t(const process_t *p, wcstring n, bool shadows); + wcstring_list_t args; + function_block_t(wcstring name, wcstring_list_t args, bool shadows); }; struct source_block_t : public block_t {