From f872f25f5bf690ded63ba2a11b6c17aacefb1c61 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sat, 5 Aug 2017 15:08:39 -0700 Subject: [PATCH] change env_var_t to a vector of strings Internally fish should store vars as a vector of elements. The current flat string representation is a holdover from when the code was written in C. Fixes #4200 --- Makefile.in | 2 +- src/builtin_argparse.cpp | 17 +- src/builtin_cd.cpp | 12 +- src/builtin_fg.cpp | 2 +- src/builtin_read.cpp | 96 +++++---- src/builtin_set.cpp | 49 +++-- src/common.cpp | 8 +- src/complete.cpp | 5 +- src/env.cpp | 366 ++++++++++++++--------------------- src/env.h | 153 +++++++++------ src/env_universal_common.cpp | 18 +- src/env_universal_common.h | 5 +- src/exec.cpp | 6 +- src/expand.cpp | 21 +- src/expand.h | 3 +- src/fish.cpp | 3 +- src/fish_tests.cpp | 166 ++++++++-------- src/function.cpp | 35 ++-- src/highlight.cpp | 5 +- src/history.cpp | 2 +- src/input.cpp | 2 +- src/input_common.cpp | 4 +- src/output.cpp | 2 +- src/output.h | 3 +- src/parse_execution.cpp | 4 +- src/path.cpp | 34 ++-- src/reader.cpp | 12 +- src/wutil.cpp | 4 +- tests/read.in | 4 - tests/test3.in | 3 +- 30 files changed, 503 insertions(+), 543 deletions(-) diff --git a/Makefile.in b/Makefile.in index 513eaf33f..898729616 100644 --- a/Makefile.in +++ b/Makefile.in @@ -360,7 +360,7 @@ test_goals := test_low_level test_fishscript test_interactive test_invocation # are added that depend, directly or indirectly, on tests, they need to be recorded here. # test_test_deps = test_low_level $(test_high_level_test_deps) -test_high_level_test_deps = test_invocation test_fishscript test_interactive +test_high_level_test_deps = test_fishscript test_interactive test_invocation active_test_goals = $(filter $(test_goals),$(foreach a,$(or $(MAKECMDGOALS),$(.DEFAULT_GOAL)),$(a) $($(a)_test_deps))) filter_up_to = $(eval b:=1)$(foreach a,$(2),$(and $(b),$(if $(subst $(1),,$(a)),$(a),$(eval b:=)))) diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp index f1d934b65..dea5e1825 100644 --- a/src/builtin_argparse.cpp +++ b/src/builtin_argparse.cpp @@ -458,13 +458,14 @@ static int validate_arg(argparse_cmd_opts_t &opts, option_spec_t *opt_spec, bool wcstring_list_t cmd_output; env_push(true); - env_set(L"_argparse_cmd", ENV_LOCAL, opts.name.c_str()); + env_set_one(L"_argparse_cmd", ENV_LOCAL, opts.name); if (is_long_flag) { - env_set(var_name_prefix + L"name", ENV_LOCAL, opt_spec->long_flag.c_str()); + env_set_one(var_name_prefix + L"name", ENV_LOCAL, opt_spec->long_flag); } else { - env_set(var_name_prefix + L"name", ENV_LOCAL, wcstring(1, opt_spec->short_flag).c_str()); + env_set_one(var_name_prefix + L"name", ENV_LOCAL, + wcstring(1, opt_spec->short_flag).c_str()); } - env_set(var_name_prefix + L"value", ENV_LOCAL, woptarg); + env_set_one(var_name_prefix + L"value", ENV_LOCAL, woptarg); int retval = exec_subshell(opt_spec->validation_command, cmd_output, false); for (auto it : cmd_output) { @@ -643,9 +644,8 @@ static void set_argparse_result_vars(argparse_cmd_opts_t &opts) { option_spec_t *opt_spec = it.second; if (!opt_spec->num_seen) continue; - auto val = list_to_array_val(opt_spec->vals); if (opt_spec->short_flag_valid) { - env_set(var_name_prefix + opt_spec->short_flag, ENV_LOCAL, val->c_str()); + env_set(var_name_prefix + opt_spec->short_flag, ENV_LOCAL, opt_spec->vals); } if (!opt_spec->long_flag.empty()) { // We do a simple replacement of all non alphanum chars rather than calling @@ -654,12 +654,11 @@ static void set_argparse_result_vars(argparse_cmd_opts_t &opts) { for (size_t pos = 0; pos < long_flag.size(); pos++) { if (!iswalnum(long_flag[pos])) long_flag[pos] = L'_'; } - env_set(var_name_prefix + long_flag, ENV_LOCAL, val->c_str()); + env_set(var_name_prefix + long_flag, ENV_LOCAL, opt_spec->vals); } } - auto val = list_to_array_val(opts.argv); - env_set(L"argv", ENV_LOCAL, val->c_str()); + env_set(L"argv", ENV_LOCAL, opts.argv); } /// The argparse builtin. This is explicitly not compatible with the BSD or GNU version of this diff --git a/src/builtin_cd.cpp b/src/builtin_cd.cpp index 8513f4a50..29328e22c 100644 --- a/src/builtin_cd.cpp +++ b/src/builtin_cd.cpp @@ -37,7 +37,7 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { wcstring dir; if (argv[optind]) { - dir_in = env_var_t(argv[optind]); + dir_in = env_var_t(L"", argv[optind]); // unamed var } else { dir_in = env_get(L"HOME"); if (dir_in.missing_or_empty()) { @@ -53,15 +53,17 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (!got_cd_path) { if (errno == ENOTDIR) { - streams.err.append_format(_(L"%ls: '%ls' is not a directory\n"), cmd, dir_in.c_str()); + streams.err.append_format(_(L"%ls: '%ls' is not a directory\n"), cmd, + dir_in.as_string().c_str()); } else if (errno == ENOENT) { streams.err.append_format(_(L"%ls: The directory '%ls' does not exist\n"), cmd, - dir_in.c_str()); + dir_in.as_string().c_str()); } else if (errno == EROTTEN) { - streams.err.append_format(_(L"%ls: '%ls' is a rotten symlink\n"), cmd, dir_in.c_str()); + streams.err.append_format(_(L"%ls: '%ls' is a rotten symlink\n"), cmd, + dir_in.as_string().c_str()); } else { streams.err.append_format(_(L"%ls: Unknown error trying to locate directory '%ls'\n"), - cmd, dir_in.c_str()); + cmd, dir_in.as_string().c_str()); } if (!shell_is_interactive()) streams.err.append(parser.current_line()); diff --git a/src/builtin_fg.cpp b/src/builtin_fg.cpp index 33255cf71..7733b34c1 100644 --- a/src/builtin_fg.cpp +++ b/src/builtin_fg.cpp @@ -102,7 +102,7 @@ int builtin_fg(parser_t &parser, io_streams_t &streams, wchar_t **argv) { } const wcstring ft = tok_first(j->command()); - if (!ft.empty()) env_set(L"_", ENV_EXPORT, ft.c_str()); + if (!ft.empty()) env_set_one(L"_", ENV_EXPORT, ft); reader_write_title(j->command()); job_promote(j); diff --git a/src/builtin_read.cpp b/src/builtin_read.cpp index 6e50e3a7f..13c14482b 100644 --- a/src/builtin_read.cpp +++ b/src/builtin_read.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include "builtin.h" #include "builtin_read.h" @@ -422,81 +423,104 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (exit_res != STATUS_CMD_OK) { // Define the var(s) without any data. We do this because when this happens we want the user // to be able to use the var but have it expand to nothing. - for (int i = 0; i < argc; i++) env_set(argv[i], opts.place, NULL); + for (int i = 0; i < argc; i++) env_set_empty(argv[i], opts.place); return exit_res; } if (!opts.have_delimiter) { env_var_t ifs = env_get(L"IFS"); - if (!ifs.missing_or_empty()) { - opts.delimiter = ifs.as_string(); - } + if (!ifs.missing_or_empty()) opts.delimiter = ifs.as_string(); } + if (opts.delimiter.empty()) { - // Every character is a separate token. - size_t bufflen = buff.size(); - if (opts.array) { - if (bufflen > 0) { - wcstring chars(bufflen + (bufflen - 1), ARRAY_SEP); - wcstring::iterator out = chars.begin(); - for (wcstring::const_iterator it = buff.begin(), end = buff.end(); it != end; - ++it) { - *out = *it; - out += 2; - } - env_set(argv[0], opts.place, chars.c_str()); + // Every character is a separate token with one wrinkle involving non-array mode where the + // final var gets the remaining characters as a single string. + size_t x = std::max(static_cast(1), buff.size()); + size_t n_splits = (opts.array || static_cast(argc) > x) ? x : argc; + wcstring_list_t chars; + chars.reserve(n_splits); + x = x - n_splits + 1; + int i = 0; + for (auto it = buff.begin(), end = buff.end(); it != end; ++i, ++it) { + if (opts.array || i < argc) { + chars.emplace_back(wcstring(1, *it)); } else { - env_set(argv[0], opts.place, NULL); + if (x) { + chars.back().reserve(x); + x = 0; + } + chars.back().push_back(*it); } - } else { // not array mode + } + + if (opts.array) { + // Array mode: assign each char as a separate element of the sole var. + env_set(argv[0], opts.place, chars); + } else { + // Not array mode: assign each char to a separate var with the remainder being assigned + // to the last var. int i = 0; size_t j = 0; for (; i + 1 < argc; ++i) { - if (j < bufflen) { - wchar_t buffer[2] = {buff[j++], 0}; - env_set(argv[i], opts.place, buffer); + if (j < chars.size()) { + env_set_one(argv[i], opts.place, chars[j]); + j++; } else { - env_set(argv[i], opts.place, L""); + env_set_one(argv[i], opts.place, L""); } } - if (i < argc) env_set(argv[i], opts.place, &buff[j]); + + if (i < argc) { + wcstring val = chars.size() == static_cast(argc) ? chars[i] : L""; + env_set_one(argv[i], opts.place, val); + } else { + env_set_empty(argv[i], opts.place); + } } } else if (opts.array) { + // The user has requested the input be split into a sequence of tokens and all the tokens + // assigned to a single var. How we do the tokenizing depends on whether the user specified + // the delimiter string or we're using IFS. if (!opts.have_delimiter) { - // We're using IFS, so well split on every character in that, - // not on the entire thing at once. - wcstring tokens; - tokens.reserve(buff.size()); + // We're using IFS, so tokenize the buffer using each IFS char. This is for backward + // compatibility with old versions of fish. + wcstring_list_t tokens; for (wcstring_range loc = wcstring_tok(buff, opts.delimiter); loc.first != wcstring::npos; loc = wcstring_tok(buff, opts.delimiter, loc)) { - if (!tokens.empty()) tokens.push_back(ARRAY_SEP); - tokens.append(buff, loc.first, loc.second); + tokens.emplace_back(wcstring(buff, loc.first, loc.second)); } - env_set(argv[0], opts.place, tokens.empty() ? NULL : tokens.c_str()); + env_set(argv[0], opts.place, tokens); } else { + // We're using a delimiter provided by the user so use the `string split` behavior. wcstring_list_t splits; split_about(buff.begin(), buff.end(), opts.delimiter.begin(), opts.delimiter.end(), &splits, LONG_MAX); - auto val = list_to_array_val(splits); - env_set(argv[0], opts.place, val->c_str()); + env_set(argv[0], opts.place, splits); } - } else { // not array + } else { + // Not array mode. Split the input into tokens and assign each to the vars in sequence. if (!opts.have_delimiter) { + // We're using IFS, so tokenize the buffer using each IFS char. This is for backward + // compatibility with old versions of fish. wcstring_range loc = wcstring_range(0, 0); for (int i = 0; i < argc; i++) { + wcstring substr; loc = wcstring_tok(buff, (i + 1 < argc) ? opts.delimiter : wcstring(), loc); - env_set(argv[i], opts.place, - loc.first == wcstring::npos ? L"" : &buff.c_str()[loc.first]); + if (loc.first != wcstring::npos) { + substr = wcstring(buff, loc.first, loc.second); + } + env_set_one(argv[i], opts.place, substr); } } else { + // We're using a delimiter provided by the user so use the `string split` behavior. wcstring_list_t splits; // We're making at most argc - 1 splits so the last variable // is set to the remaining string. split_about(buff.begin(), buff.end(), opts.delimiter.begin(), opts.delimiter.end(), &splits, argc - 1); for (size_t i = 0; i < (size_t)argc && i < splits.size(); i++) { - env_set(argv[i], opts.place, splits[i].c_str()); + env_set_one(argv[i], opts.place, splits[i]); } } } diff --git a/src/builtin_set.cpp b/src/builtin_set.cpp index 9d45f9e80..b74df2f76 100644 --- a/src/builtin_set.cpp +++ b/src/builtin_set.cpp @@ -293,8 +293,7 @@ static int my_env_set(const wchar_t *cmd, const wchar_t *key, int scope, if (retval != STATUS_CMD_OK) return retval; } - auto val = list_to_array_val(list); - retval = env_set(key, scope | ENV_USER, val->c_str()); + retval = env_set(key, scope | ENV_USER, list); switch (retval) { case ENV_OK: { retval = STATUS_CMD_OK; @@ -502,8 +501,9 @@ static int builtin_set_query(const wchar_t *cmd, set_cmd_opts_t &opts, int argc, for (auto idx : indexes) { if (idx < 1 || (size_t)idx > result.size()) retval++; } - } else if (!env_exist(arg, scope)) { - retval++; + } else { + env_var_t var = env_get(arg, scope); + if (var.missing()) retval++; } free(dest); @@ -533,29 +533,26 @@ static void show_scope(const wchar_t *var_name, int scope, io_streams_t &streams } } - if (env_exist(var_name, scope)) { - const env_var_t evar = env_get(var_name, scope | ENV_EXPORT | ENV_USER); - const wchar_t *exportv = evar.missing() ? _(L"unexported") : _(L"exported"); - - const env_var_t var = env_get(var_name, scope | ENV_USER); - wcstring_list_t result; - var.to_list(result); - - streams.out.append_format(_(L"$%ls: set in %ls scope, %ls, with %d elements\n"), var_name, - scope_name, exportv, result.size()); - for (size_t i = 0; i < result.size(); i++) { - if (result.size() > 100) { - if (i == 50) streams.out.append(L"...\n"); - if (i >= 50 && i < result.size() - 50) continue; - } - const wcstring value = result[i]; - const wcstring escaped_val = - escape_string(value.c_str(), ESCAPE_NO_QUOTED, STRING_STYLE_SCRIPT); - streams.out.append_format(_(L"$%ls[%d]: length=%d value=|%ls|\n"), var_name, i + 1, - value.size(), escaped_val.c_str()); - } - } else { + const env_var_t var = env_get(var_name, scope); + if (var.missing()) { streams.out.append_format(_(L"$%ls: not set in %ls scope\n"), var_name, scope_name); + return; + } + + const wchar_t *exportv = var.exportv ? _(L"exported") : _(L"unexported"); + wcstring_list_t vals = var.as_const_list(); + streams.out.append_format(_(L"$%ls: set in %ls scope, %ls, with %d elements\n"), var_name, + scope_name, exportv, vals.size()); + for (size_t i = 0; i < vals.size(); i++) { + if (vals.size() > 100) { + if (i == 50) streams.out.append(L"...\n"); + if (i >= 50 && i < vals.size() - 50) continue; + } + const wcstring value = vals[i]; + const wcstring escaped_val = + escape_string(value.c_str(), ESCAPE_NO_QUOTED, STRING_STYLE_SCRIPT); + streams.out.append_format(_(L"$%ls[%d]: length=%d value=|%ls|\n"), var_name, i + 1, + value.size(), escaped_val.c_str()); } } diff --git a/src/common.cpp b/src/common.cpp index c3a3379d7..4a36d0a4b 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1557,9 +1557,9 @@ static void validate_new_termsize(struct winsize *new_termsize) { env_var_t row_var = env_get(L"LINES"); if (!col_var.missing_or_empty() && !row_var.missing_or_empty()) { // Both vars have to have valid values. - int col = fish_wcstoi(col_var.c_str()); + int col = fish_wcstoi(col_var.as_string().c_str()); bool col_ok = errno == 0 && col > 0 && col <= USHRT_MAX; - int row = fish_wcstoi(row_var.c_str()); + int row = fish_wcstoi(row_var.as_string().c_str()); bool row_ok = errno == 0 && row > 0 && row <= USHRT_MAX; if (col_ok && row_ok) { new_termsize->ws_col = col; @@ -1584,11 +1584,11 @@ static void export_new_termsize(struct winsize *new_termsize) { env_var_t cols = env_get(L"COLUMNS", ENV_EXPORT); swprintf(buf, 64, L"%d", (int)new_termsize->ws_col); - env_set(L"COLUMNS", ENV_GLOBAL | (cols.missing_or_empty() ? 0 : ENV_EXPORT), buf); + env_set_one(L"COLUMNS", ENV_GLOBAL | (cols.missing_or_empty() ? ENV_DEFAULT : ENV_EXPORT), buf); env_var_t lines = env_get(L"LINES", ENV_EXPORT); swprintf(buf, 64, L"%d", (int)new_termsize->ws_row); - env_set(L"LINES", ENV_GLOBAL | (lines.missing_or_empty() ? 0 : ENV_EXPORT), buf); + env_set_one(L"LINES", ENV_GLOBAL | (lines.missing_or_empty() ? ENV_DEFAULT : ENV_EXPORT), buf); #ifdef HAVE_WINSIZE ioctl(STDOUT_FILENO, TIOCSWINSZ, new_termsize); diff --git a/src/complete.cpp b/src/complete.cpp index 53b4f0954..322aadb4b 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -12,7 +12,9 @@ #include #include +#include #include +#include #include #include #include @@ -1108,7 +1110,7 @@ bool completer_t::complete_variable(const wcstring &str, size_t start_offset) { env_var_t var = env_get(env_name); if (var.missing()) continue; - wcstring value = expand_escape_variable(var.as_string()); + wcstring value = expand_escape_variable(var); if (this->type() != COMPLETE_AUTOSUGGEST) { desc = format_string(COMPLETE_VAR_DESC_VAL, value.c_str()); } @@ -1445,7 +1447,6 @@ void complete(const wcstring &cmd_with_subcmds, std::vector *out_c if (i == 0) { assert(wrap_chain.at(i) == current_command_unescape); } else if (!(flags & COMPLETION_REQUEST_AUTOSUGGESTION)) { - assert(cmd_node != NULL); wcstring faux_cmdline = cmd; faux_cmdline.replace(cmd_node->source_start, cmd_node->source_length, wrap_chain.at(i)); diff --git a/src/env.cpp b/src/env.cpp index 5a4b1b0d9..4ab201074 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -105,7 +105,6 @@ static const wcstring_list_t colon_delimited_variable({L"PATH", L"MANPATH", L"CD // Some forward declarations to make it easy to logically group the code. static void init_locale(); static void init_curses(); -static void tokenize_variable_array(const wcstring &val, wcstring_list_t &out); /// Construct a missing var object env_var_t create_missing_var() { @@ -116,6 +115,33 @@ env_var_t create_missing_var() { env_var_t missing_var = create_missing_var(); +/// This is used to convert a serialized env_var_t back into a list. It is used when reading legacy +/// (fish 2.x) encoded vars stored in the universal variable file and the environment. +static void tokenize_variable_array(const wcstring &val, wcstring_list_t &out) { + out.clear(); // ensure the output var is empty -- this will normally be a no-op + + // Zero element arrays are externally encoded as this placeholder string. + if (val == ENV_NULL) return; + + size_t pos = 0, end = val.size(); + while (pos <= end) { + size_t next_pos = val.find(ARRAY_SEP, pos); + if (next_pos == wcstring::npos) { + next_pos = end; + } + out.resize(out.size() + 1); + out.back().assign(val, pos, next_pos - pos); + pos = next_pos + 1; // skip the separator, or skip past the end + } +} + +/// This is used to convert a serialized env_var_t back into a list. +wcstring_list_t decode_serialized(const wcstring &s) { + wcstring_list_t values; + tokenize_variable_array(s, values); + return values; +} + // Struct representing one level in the function variable stack. // Only our variable stack should create and destroy these class env_node_t { @@ -139,10 +165,6 @@ class env_node_t { const env_var_t find_entry(const wcstring &key); }; -class variable_entry_t { - wcstring value; /**< Value of the variable */ -}; - static pthread_mutex_t env_lock = PTHREAD_MUTEX_INITIALIZER; static bool local_scope_exports(const env_node_t *n); @@ -331,10 +353,11 @@ static bool var_is_timezone(const wcstring &key) { return key == L"TZ"; } /// Properly sets all timezone information. static void handle_timezone(const wchar_t *env_var_name) { - debug(2, L"handle_timezone() called in response to '%ls' changing", env_var_name); - const env_var_t var = env_get(env_var_name, ENV_EXPORT); + // const env_var_t var = env_get(env_var_name, ENV_EXPORT); + const env_var_t var = env_get(env_var_name, ENV_DEFAULT); + debug(2, L"handle_timezone() current timezone var: |%ls| => |%ls|", env_var_name, + var.missing() ? L"MISSING" : var.as_string().c_str()); const std::string &name = wcs2string(env_var_name); - debug(2, L"timezone var %s='%s'", name.c_str(), var.c_str()); if (var.missing_or_empty()) { unsetenv(name.c_str()); } else { @@ -370,17 +393,9 @@ static void fix_colon_delimited_var(const wcstring &var_name) { return; } - wcstring new_val; - for (size_t i = 0; i < new_pathsv.size(); i++) { - new_val.append(new_pathsv[i]); - if (i < new_pathsv.size() - 1) { - new_val.append(ARRAY_SEP_STR); - } - } - - int scope = env_set(var_name, ENV_USER, new_val.c_str()); - if (scope != ENV_OK) { - debug(0, L"fix_colon_delimited_var failed unexpectedly with retval %d", scope); + int retval = env_set(var_name, ENV_DEFAULT | ENV_USER, new_pathsv); + if (retval != ENV_OK) { + debug(0, L"fix_colon_delimited_var failed unexpectedly with retval %d", retval); } } @@ -460,7 +475,7 @@ static bool does_term_support_setting_title() { const env_var_t term_var = env_get(L"TERM"); if (term_var.missing_or_empty()) return false; - const wchar_t *term = term_var.c_str(); + const wchar_t *term = term_var.as_string().c_str(); bool recognized = contains(title_terms, term_var.as_string()); if (!recognized) recognized = !wcsncmp(term, L"xterm-", wcslen(L"xterm-")); if (!recognized) recognized = !wcsncmp(term, L"screen-", wcslen(L"screen-")); @@ -585,7 +600,7 @@ static void init_curses() { if (term.missing_or_empty()) { debug(1, _(L"TERM environment variable not set.")); } else { - debug(1, _(L"TERM environment variable set to '%ls'."), term.c_str()); + debug(1, _(L"TERM environment variable set to '%ls'."), term.as_string().c_str()); debug(1, _(L"Check that this terminal type is supported on this system.")); } } @@ -675,7 +690,7 @@ static void universal_callback(fish_message_type_t type, const wchar_t *name) { static void setup_path() { const env_var_t path = env_get(L"PATH"); if (path.missing_or_empty()) { - const wchar_t *value = L"/usr/bin" ARRAY_SEP_STR L"/bin"; + wcstring_list_t value({L"/usr/bin", L"/bin"}); env_set(L"PATH", ENV_GLOBAL | ENV_EXPORT, value); } } @@ -685,20 +700,20 @@ static void setup_path() { /// adjusted. static void env_set_termsize() { env_var_t cols = env_get(L"COLUMNS"); - if (cols.missing_or_empty()) env_set(L"COLUMNS", ENV_GLOBAL, DFLT_TERM_COL_STR); + if (cols.missing_or_empty()) env_set_one(L"COLUMNS", ENV_GLOBAL, DFLT_TERM_COL_STR); env_var_t rows = env_get(L"LINES"); - if (rows.missing_or_empty()) env_set(L"LINES", ENV_GLOBAL, DFLT_TERM_ROW_STR); + if (rows.missing_or_empty()) env_set_one(L"LINES", ENV_GLOBAL, DFLT_TERM_ROW_STR); } bool env_set_pwd() { - wcstring res = wgetcwd(); - if (res.empty()) { + wcstring cwd = wgetcwd(); + if (cwd.empty()) { debug(0, _(L"Could not determine current working directory. Is your locale set correctly?")); return false; } - env_set(L"PWD", ENV_EXPORT | ENV_GLOBAL, res.c_str()); + env_set_one(L"PWD", ENV_EXPORT | ENV_GLOBAL, cwd); return true; } @@ -707,7 +722,7 @@ bool env_set_pwd() { void env_set_read_limit() { env_var_t read_byte_limit_var = env_get(L"FISH_READ_BYTE_LIMIT"); if (!read_byte_limit_var.missing_or_empty()) { - size_t limit = fish_wcstoull(read_byte_limit_var.c_str()); + size_t limit = fish_wcstoull(read_byte_limit_var.as_string().c_str()); if (errno) { debug(1, "Ignoring FISH_READ_BYTE_LIMIT since it is not valid"); } else { @@ -730,14 +745,14 @@ wcstring env_get_pwd_slash(void) { /// Set up the USER variable. static void setup_user(bool force) { - if (env_get(L"USER").missing_or_empty() || force) { + if (force || env_get(L"USER").missing_or_empty()) { struct passwd userinfo; struct passwd *result; char buf[8192]; int retval = getpwuid_r(getuid(), &userinfo, buf, sizeof(buf), &result); if (!retval && result) { const wcstring uname = str2wcstring(userinfo.pw_name); - env_set(L"USER", ENV_GLOBAL | ENV_EXPORT, uname.c_str()); + env_set_one(L"USER", ENV_GLOBAL | ENV_EXPORT, uname); } } } @@ -820,34 +835,35 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { wcstring key, val; const char *const *envp = environ; size_t i = 0; - while (envp && envp[i]) { - i++; - } + while (envp && envp[i]) i++; while (i--) { const wcstring key_and_val = str2wcstring(envp[i]); // like foo=bar size_t eql = key_and_val.find(L'='); if (eql == wcstring::npos) { - // No equals found. + // No equal-sign found so treat it as a defined var that has no value(s). if (is_read_only(key_and_val) || is_electric(key_and_val)) continue; - env_set(key_and_val, ENV_EXPORT | ENV_GLOBAL, L""); + env_set_empty(key_and_val, ENV_EXPORT | ENV_GLOBAL); } else { key.assign(key_and_val, 0, eql); if (is_read_only(key) || is_electric(key)) continue; val.assign(key_and_val, eql + 1, wcstring::npos); if (variable_is_colon_delimited_var(key)) { std::replace(val.begin(), val.end(), L':', ARRAY_SEP); + wcstring_list_t values = decode_serialized(val); + env_set(key, ENV_EXPORT | ENV_GLOBAL, values); + } else { + wcstring_list_t values = decode_serialized(val); + env_set(key, ENV_EXPORT | ENV_GLOBAL, values); } - - env_set(key, ENV_EXPORT | ENV_GLOBAL, val.c_str()); } } // Set the given paths in the environment, if we have any. if (paths != NULL) { - env_set(FISH_DATADIR_VAR, ENV_GLOBAL, paths->data.c_str()); - env_set(FISH_SYSCONFDIR_VAR, ENV_GLOBAL, paths->sysconf.c_str()); - env_set(FISH_HELPDIR_VAR, ENV_GLOBAL, paths->doc.c_str()); - env_set(FISH_BIN_DIR, ENV_GLOBAL, paths->bin.c_str()); + env_set_one(FISH_DATADIR_VAR, ENV_GLOBAL, paths->data); + env_set_one(FISH_SYSCONFDIR_VAR, ENV_GLOBAL, paths->sysconf); + env_set_one(FISH_HELPDIR_VAR, ENV_GLOBAL, paths->doc); + env_set_one(FISH_BIN_DIR, ENV_GLOBAL, paths->bin); } init_locale(); @@ -868,7 +884,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { // Set up the version variable. wcstring version = str2wcstring(get_fish_version()); - env_set(L"FISH_VERSION", ENV_GLOBAL, version.c_str()); + env_set_one(L"FISH_VERSION", ENV_GLOBAL, version); // Set up SHLVL variable. const env_var_t shlvl_var = env_get(L"SHLVL"); @@ -876,12 +892,12 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { if (!shlvl_var.missing_or_empty()) { const wchar_t *end; // TODO: Figure out how to handle invalid numbers better. Shouldn't we issue a diagnostic? - long shlvl_i = fish_wcstol(shlvl_var.c_str(), &end); + long shlvl_i = fish_wcstol(shlvl_var.as_string().c_str(), &end); if (!errno && shlvl_i >= 0) { nshlvl_str = to_string(shlvl_i + 1); } } - env_set(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, nshlvl_str.c_str()); + env_set_one(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, nshlvl_str); env_read_only.insert(L"SHLVL"); // Set up the HOME variable. @@ -892,7 +908,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { if (env_get(L"HOME").missing_or_empty()) { env_var_t user_var = env_get(L"USER"); if (!user_var.missing_or_empty()) { - char *unam_narrow = wcs2str(user_var.c_str()); + char *unam_narrow = wcs2str(user_var.as_string()); struct passwd userinfo; struct passwd *result; char buf[8192]; @@ -902,24 +918,22 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { setup_user(true); user_var = env_get(L"USER"); if (!user_var.missing_or_empty()) { - unam_narrow = wcs2str(user_var.c_str()); + unam_narrow = wcs2str(user_var.as_string()); retval = getpwnam_r(unam_narrow, &userinfo, buf, sizeof(buf), &result); } } if (!retval && result && userinfo.pw_dir) { const wcstring dir = str2wcstring(userinfo.pw_dir); - env_set(L"HOME", ENV_GLOBAL | ENV_EXPORT, dir.c_str()); + env_set_one(L"HOME", ENV_GLOBAL | ENV_EXPORT, dir); } else { - // We cannot get $HOME, set it to the empty list. - // This triggers warnings for history and config.fish already, + // We cannot get $HOME. This triggers warnings for history and config.fish already, // so it isn't necessary to warn here as well. - env_set(L"HOME", ENV_GLOBAL | ENV_EXPORT, ENV_NULL); + env_set_empty(L"HOME", ENV_GLOBAL | ENV_EXPORT); } free(unam_narrow); } else { - // If $USER is empty as well (which we tried to set above), - // we can't get $HOME. - env_set(L"HOME", ENV_GLOBAL | ENV_EXPORT, ENV_NULL); + // If $USER is empty as well (which we tried to set above), we can't get $HOME. + env_set_empty(L"HOME", ENV_GLOBAL | ENV_EXPORT); } } @@ -933,7 +947,7 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { use_posix_spawn.missing_or_empty() ? true : from_string(use_posix_spawn.as_string()); // Set fish_bind_mode to "default". - env_set(FISH_BIND_MODE_VAR, ENV_GLOBAL, DEFAULT_BIND_MODE); + env_set_one(FISH_BIND_MODE_VAR, ENV_GLOBAL, DEFAULT_BIND_MODE); // This is somewhat subtle. At this point we consider our environment to be sufficiently // initialized that we can react to changes to variables. Prior to doing this we expect that the @@ -966,6 +980,19 @@ static env_node_t *env_get_node(const wcstring &key) { return env; } +/// Sets the variable with the specified name to a single value. +int env_set_one(const wcstring &key, env_mode_flags_t mode, const wcstring &val) { + wcstring_list_t vals({ + val, + }); + return env_set(key, mode, vals); +} + +/// Sets the variable with the specified name to no (i.e., zero) values. +int env_set_empty(const wcstring &key, env_mode_flags_t mode) { + return env_set(key, mode, wcstring_list_t()); +} + /// Set the value of the environment variable whose name matches key to val. /// /// Memory policy: All keys and values are copied, the parameters can and should be freed by the @@ -974,8 +1001,8 @@ static env_node_t *env_get_node(const wcstring &key) { /// \param key The key /// \param val The value /// \param var_mode The type of the variable. Can be any combination of ENV_GLOBAL, ENV_LOCAL, -/// ENV_EXPORT and ENV_USER. If mode is zero, the current variable space is searched and the current -/// mode is used. If no current variable with the same name is found, ENV_LOCAL is assumed. +/// ENV_EXPORT and ENV_USER. If mode is ENV_DEFAULT, the current variable space is searched and the +/// current mode is used. If no current variable with the same name is found, ENV_LOCAL is assumed. /// /// Returns: /// @@ -985,21 +1012,23 @@ static env_node_t *env_get_node(const wcstring &key) { /// * ENV_SCOPE, the variable cannot be set in the given scope. This applies to readonly/electric /// variables set from the local or universal scopes, or set as exported. /// * ENV_INVALID, the variable value was invalid. This applies only to special variables. -int env_set(const wcstring &key, env_mode_flags_t var_mode, const wchar_t *val) { +int env_set(const wcstring &key, env_mode_flags_t var_mode, const wcstring_list_t &vals) { ASSERT_IS_MAIN_THREAD(); bool has_changed_old = vars_stack().has_changed_exported; int done = 0; - if (val && (key == L"PWD" || key == L"HOME")) { + if (!vals.empty() && (key == L"PWD" || key == L"HOME")) { // Canonicalize our path; if it changes, recurse and try again. - wcstring val_canonical = val; + assert(vals.size() == 1); + wcstring val_canonical = vals[0]; path_make_canonical(val_canonical); - if (val != val_canonical) { - return env_set(key, var_mode, val_canonical.c_str()); + if (vals[0] != val_canonical) { + return env_set_one(key, var_mode, val_canonical); } } - if ((var_mode & (ENV_LOCAL | ENV_UNIVERSAL)) && (is_read_only(key) || is_electric(key))) { + if ((var_mode & ENV_LOCAL || var_mode & ENV_UNIVERSAL) && + (is_read_only(key) || is_electric(key))) { return ENV_SCOPE; } if ((var_mode & ENV_EXPORT) && is_electric(key)) { @@ -1011,8 +1040,8 @@ int env_set(const wcstring &key, env_mode_flags_t var_mode, const wchar_t *val) if (key == L"umask") { // Set the new umask. - if (val && wcslen(val)) { - long mask = fish_wcstol(val, NULL, 8); + if (!vals.empty() && vals[0].size()) { + long mask = fish_wcstol(vals[0].c_str(), NULL, 8); if (!errno && mask <= 0777 && mask >= 0) { umask(mask); // Do not actually create a umask variable, on env_get, it will be calculated @@ -1020,15 +1049,9 @@ int env_set(const wcstring &key, env_mode_flags_t var_mode, const wchar_t *val) return ENV_OK; } } - return ENV_INVALID; } - // Zero element arrays are internally not coded as an empty string but this placeholder string. - if (!val) { - val = ENV_NULL; //!OCLINT(parameter reassignment) - } - if (var_mode & ENV_UNIVERSAL) { const bool old_export = uvars() && uvars()->get_export(key); bool new_export; @@ -1043,7 +1066,7 @@ int env_set(const wcstring &key, env_mode_flags_t var_mode, const wchar_t *val) new_export = old_export; } if (uvars()) { - uvars()->set(key, val, new_export); + uvars()->set(key, vals, new_export); env_universal_barrier(); if (old_export || new_export) { vars_stack().mark_changed_exported(); @@ -1092,7 +1115,7 @@ int env_set(const wcstring &key, env_mode_flags_t var_mode, const wchar_t *val) exportv = uvars()->get_export(key); } - uvars()->set(key, val, exportv); + uvars()->set(key, vals, exportv); env_universal_barrier(); done = 1; @@ -1112,7 +1135,7 @@ int env_set(const wcstring &key, env_mode_flags_t var_mode, const wchar_t *val) has_changed_new = true; } - var.set_val(val); + var.set_vals(vals); if (var_mode & ENV_EXPORT) { // The new variable is exported. var.exportv = true; @@ -1135,9 +1158,9 @@ int env_set(const wcstring &key, env_mode_flags_t var_mode, const wchar_t *val) ev.arguments.push_back(L"SET"); ev.arguments.push_back(key); - // debug( 1, L"env_set: fire events on variable %ls", key ); + // debug(1, L"env_set: fire events on variable |%ls|", key); event_fire(&ev); - // debug( 1, L"env_set: return from event firing" ); + // debug(1, L"env_set: return from event firing"); react_to_variable_change(L"SET", key); return ENV_OK; @@ -1217,19 +1240,29 @@ int env_remove(const wcstring &key, int var_mode) { return !erased; } -wcstring env_var_t::as_string(void) const { - assert(!is_missing); - return val; -} +wcstring_list_t &env_var_t::as_list(void) { return vals; } -const wchar_t *env_var_t::c_str(void) const { - assert(!is_missing); - return val.c_str(); +const wcstring_list_t &env_var_t::as_const_list(void) const { return vals; } + +/// Return a string representation of the var. At the present time this uses the legacy 2.x +/// encoding. +wcstring env_var_t::as_string(void) const { + assert(!this->is_missing); + if (this->vals.empty()) return wcstring(ENV_NULL); + + wchar_t sep = variable_is_colon_delimited_var(this->name) ? L':' : ARRAY_SEP; + auto it = this->vals.cbegin(); + wcstring result(*it); + while (++it != vals.end()) { + result.push_back(sep); + result.append(*it); + } + return result; } void env_var_t::to_list(wcstring_list_t &out) const { assert(!is_missing); - tokenize_variable_array(val, out); + out = vals; } env_var_t env_get(const wcstring &key, env_mode_flags_t mode) { @@ -1258,34 +1291,33 @@ env_var_t env_get(const wcstring &key, env_mode_flags_t mode) { } wcstring result; if (history) history->get_string_representation(&result, ARRAY_SEP_STR); - return env_var_t(result); + return env_var_t(L"history", result); } else if (key == L"status") { - return env_var_t(to_string(proc_get_last_status())); + return env_var_t(L"status", to_string(proc_get_last_status())); } else if (key == L"umask") { - return env_var_t(format_string(L"0%0.3o", get_umask())); + return env_var_t(L"umask", format_string(L"0%0.3o", get_umask())); } // We should never get here unless the electric var list is out of sync with the above code. DIE("unerecognized electric var name"); } if (search_local || search_global) { - /* Lock around a local region */ - scoped_lock locker(env_lock); - + scoped_lock locker(env_lock); // lock around a local region env_node_t *env = search_local ? vars_stack().top.get() : vars_stack().global_env; while (env != NULL) { - const env_var_t var = env->find_entry(key); - if (!var.missing() && (var.exportv ? search_exported : search_unexported)) { - return var; + if (env == vars_stack().global_env && !search_global) { + break; } - if (has_scope) { - if (!search_global || env == vars_stack().global_env) break; - env = vars_stack().global_env; - } else { - env = vars_stack().next_scope_to_search(env); + var_table_t::const_iterator result = env->env.find(key); + if (result != env->env.end()) { + const env_var_t var = result->second; + if (!var.missing() && (var.exportv ? search_exported : search_unexported)) { + return var; + } } + env = vars_stack().next_scope_to_search(env); } } @@ -1301,66 +1333,15 @@ env_var_t env_get(const wcstring &key, env_mode_flags_t mode) { // Okay, we couldn't find a local or global var given the requirements. If there is a matching // universal var return that. if (uvars()) { - env_var_t env_var = uvars()->get(key); - if (!env_var.missing() && - (uvars()->get_export(key) ? search_exported : search_unexported)) { - return env_var; + env_var_t var = uvars()->get(key); + if (!var.missing() && (uvars()->get_export(key) ? search_exported : search_unexported)) { + return var; } } return missing_var; } -bool env_exist(const wchar_t *key, env_mode_flags_t mode) { - CHECK(key, false); - - const bool has_scope = mode & (ENV_LOCAL | ENV_GLOBAL | ENV_UNIVERSAL); - const bool test_local = !has_scope || (mode & ENV_LOCAL); - const bool test_global = !has_scope || (mode & ENV_GLOBAL); - const bool test_universal = !has_scope || (mode & ENV_UNIVERSAL); - - const bool test_exported = (mode & ENV_EXPORT) || !(mode & ENV_UNEXPORT); - const bool test_unexported = (mode & ENV_UNEXPORT) || !(mode & ENV_EXPORT); - - if (is_electric(key)) { - // Electric variables all exist, and they are all global. A local or universal version can - // not exist. They are also never exported. - if (test_global && test_unexported) { - return true; - } - return false; - } - - if (test_local || test_global) { - const env_node_t *env = test_local ? vars_stack().top.get() : vars_stack().global_env; - while (env != NULL) { - if (env == vars_stack().global_env && !test_global) { - break; - } - - var_table_t::const_iterator result = env->env.find(key); - if (result != env->env.end()) { - const env_var_t var = result->second; - return var.exportv ? test_exported : test_unexported; - } - env = vars_stack().next_scope_to_search(env); - } - } - - if (test_universal) { - if (!get_proc_had_barrier()) { - set_proc_had_barrier(true); - env_universal_barrier(); - } - - if (uvars() && !uvars()->get(key).missing()) { - return uvars()->get_export(key) ? test_exported : test_unexported; - } - } - - return 0; -} - /// Returns true if the specified scope or any non-shadowed non-global subscopes contain an exported /// variable. static bool local_scope_exports(const env_node_t *n) { @@ -1506,10 +1487,10 @@ void var_stack_t::update_export_array_if_necessary() { const wcstring &key = uni.at(i); const env_var_t var = uvars()->get(key); - if (!var.missing() && var.as_string() != ENV_NULL) { + if (!var.missing() && !var.empty()) { // Note that std::map::insert does NOT overwrite a value already in the map, // which we depend on here. - vals.insert(std::pair(key, var.as_string())); + vals.insert(std::pair(key, var)); } } } @@ -1528,20 +1509,15 @@ const char *const *env_export_arr() { } void env_set_argv(const wchar_t *const *argv) { - if (*argv) { - const wchar_t *const *arg; - wcstring sb; - - for (arg = argv; *arg; arg++) { - if (arg != argv) { - sb.append(ARRAY_SEP_STR); - } - sb.append(*arg); + if (argv && *argv) { + wcstring_list_t list; + for (auto arg = argv; *arg; arg++) { + list.push_back(*arg); } - env_set(L"argv", ENV_LOCAL, sb.c_str()); + env_set(L"argv", ENV_LOCAL, list); } else { - env_set(L"argv", ENV_LOCAL, NULL); + env_set_empty(L"argv", ENV_LOCAL); } } @@ -1552,7 +1528,7 @@ env_vars_snapshot_t::env_vars_snapshot_t(const wchar_t *const *keys) { key.assign(keys[i]); const env_var_t var = env_get(key); if (!var.missing()) { - vars[key] = var.as_string(); + vars[key] = var; } } } @@ -1571,7 +1547,7 @@ env_var_t env_vars_snapshot_t::get(const wcstring &key) const { if (this->is_current()) { return env_get(key); } - std::map::const_iterator iter = vars.find(key); + std::map::const_iterator iter = vars.find(key); return iter == vars.end() ? missing_var : env_var_t(iter->second); } @@ -1579,61 +1555,3 @@ const wchar_t *const env_vars_snapshot_t::highlighting_keys[] = {L"PATH", L"CDPA L"fish_function_path", NULL}; const wchar_t *const env_vars_snapshot_t::completing_keys[] = {L"PATH", L"CDPATH", NULL}; - -// The next set of functions convert between a flat string and an actual list of strings using -// the encoding employed by fish internally for "arrays". - -std::unique_ptr list_to_array_val(const wcstring_list_t &list) { - auto val = std::unique_ptr(new wcstring()); - - if (list.size() == 0) { - // Zero element arrays are internally encoded as this placeholder string. - val->append(ENV_NULL); - } else { - bool need_sep = false; - for (auto it : list) { - if (need_sep) { - val->push_back(ARRAY_SEP); - } else { - need_sep = true; - } - val->append(it); - } - } - - return val; -} - -std::unique_ptr list_to_array_val(const wchar_t **list) { - auto val = std::unique_ptr(new wcstring()); - - if (!*list) { - // Zero element arrays are internally encoded as this placeholder string. - val->append(ENV_NULL); - } else { - for (auto it = list; *it; it++) { - if (it != list) val->push_back(ARRAY_SEP); - val->append(*it); - } - } - - return val; -} - -static void tokenize_variable_array(const wcstring &val, wcstring_list_t &out) { - out.clear(); // ensure the output var is empty -- this will normally be a no-op - - // Zero element arrays are internally encoded as this placeholder string. - if (val == ENV_NULL) return; - - size_t pos = 0, end = val.size(); - while (pos <= end) { - size_t next_pos = val.find(ARRAY_SEP, pos); - if (next_pos == wcstring::npos) { - next_pos = end; - } - out.resize(out.size() + 1); - out.back().assign(val, pos, next_pos - pos); - pos = next_pos + 1; // skip the separator, or skip past the end - } -} diff --git a/src/env.h b/src/env.h index e29dda483..65e68cf3f 100644 --- a/src/env.h +++ b/src/env.h @@ -1,4 +1,4 @@ -// Prototypes for functions for setting and getting environment variables. +// Prototypes for functions for manipulating fish script variables. #ifndef FISH_ENV_H #define FISH_ENV_H @@ -8,6 +8,7 @@ #include #include #include +#include #include "common.h" @@ -26,27 +27,24 @@ extern bool curses_initialized; // Flags that may be passed as the 'mode' in env_set / env_get. enum { - /// Default mode. + /// Default mode. Used with `env_get()` to indicate the caller doesn't care what scope the var + /// is in or whether it is exported or unexported. ENV_DEFAULT = 0, - /// Flag for local (to the current block) variable. - ENV_LOCAL = 1, - - /// Flag for exported (to commands) variable. - ENV_EXPORT = 2, - - /// Flag for unexported variable. - ENV_UNEXPORT = 16, - + ENV_LOCAL = 1 << 0, /// Flag for global variable. - ENV_GLOBAL = 4, - - /// Flag for variable update request from the user. All variable changes that are made directly - /// by the user, such as those from the 'set' builtin must have this flag set. - ENV_USER = 8, - + ENV_GLOBAL = 1 << 1, /// Flag for universal variable. - ENV_UNIVERSAL = 32 + ENV_UNIVERSAL = 1 << 2, + /// Flag for exported (to commands) variable. + ENV_EXPORT = 1 << 3, + /// Flag for unexported variable. + ENV_UNEXPORT = 1 << 4, + /// Flag for variable update request from the user. All variable changes that are made directly + /// by the user, such as those from the `read` and `set` builtin must have this flag set. It + /// serves one purpose: to indicate that an error should be returned if the user is attempting + /// to modify a var that should not be modified by direct user action; e.g., a read-only var. + ENV_USER = 1 << 5, }; typedef uint32_t env_mode_flags_t; @@ -56,10 +54,10 @@ enum { ENV_OK, ENV_PERM, ENV_SCOPE, ENV_INVALID }; /// A struct of configuration directories, determined in main() that fish will optionally pass to /// env_init. struct config_paths_t { - wcstring data; // e.g. /usr/local/share - wcstring sysconf; // e.g. /usr/local/etc - wcstring doc; // e.g. /usr/local/share/doc/fish - wcstring bin; // e.g. /usr/local/bin + wcstring data; // e.g., /usr/local/share + wcstring sysconf; // e.g., /usr/local/etc + wcstring doc; // e.g., /usr/local/share/doc/fish + wcstring bin; // e.g., /usr/local/bin }; /// Initialize environment variable data. @@ -71,68 +69,101 @@ void misc_init(); class env_var_t { private: - wcstring val; - bool is_missing; + wcstring name; // name of the var + wcstring_list_t vals; // list of values assigned to the var + bool is_missing; // true if the var is not set in the requested scope public: bool exportv; // whether the variable should be exported - env_var_t(const wcstring &x) : val(x), is_missing(false), exportv(false) {} - env_var_t(const wchar_t *x) : val(x), is_missing(false), exportv(false) {} - env_var_t() : val(L""), is_missing(false), exportv(false) {} - void set_missing(void) { is_missing = true; } - bool empty(void) const { return val.empty() || val == ENV_NULL; } + // Constructors. + env_var_t(const env_var_t &v) + : name(v.name), vals(v.vals), is_missing(v.is_missing), exportv(v.exportv) {} + env_var_t(const wcstring &our_name, const wcstring_list_t &l) + : name(our_name), vals(l), is_missing(false), exportv(false) {} + env_var_t(const wcstring &our_name, const wcstring &s) + : name(our_name), + vals({ + s, + }), + is_missing(false), + exportv(false) {} + env_var_t(const wcstring &our_name, const wchar_t *s) + : name(our_name), + vals({ + wcstring(s), + }), + is_missing(false), + exportv(false) {} + env_var_t() : name(), vals(), is_missing(false), exportv(false) {} + + bool empty(void) const { return vals.empty() || (vals.size() == 1 && vals[0].empty()); }; bool missing(void) const { return is_missing; } bool missing_or_empty(void) const { return missing() || empty(); } - const wchar_t *c_str(void) const; - void to_list(wcstring_list_t &out) const; + bool matches_string(const wcstring &str) { + if (is_missing) return false; + if (vals.size() > 1) return false; + return vals[0] == str; + } + wcstring as_string() const; + void to_list(wcstring_list_t &out) const; + wcstring_list_t &as_list(); + const wcstring_list_t &as_const_list() const; + + const wcstring get_name() const { return name; } - // You cannot convert a missing var to a non-missing var. You can only change the value of a var - // that is not missing. void set_val(const wcstring &s) { - assert(!is_missing); - val = s; + vals = { + s, + }; } - void set_val(const wchar_t *s) { - assert(!is_missing); - val = s; + void set_vals(const wcstring_list_t &l) { vals = l; } + + env_var_t &operator=(const env_var_t &var) { + this->vals = var.vals; + this->exportv = var.exportv; + this->is_missing = var.is_missing; + return *this; } - bool operator==(const env_var_t &s) const { return is_missing == s.is_missing && val == s.val; } + /// Compare a simple string to the var. Returns true iff the var is not missing, has a single + /// value and that value matches the string being compared to. + bool operator==(const wcstring &str) const { + if (is_missing) return false; + if (vals.size() > 1) return false; + return vals[0] == str; + } - bool operator==(const wcstring &s) const { return !is_missing && val == s; } + bool operator==(const env_var_t &var) const { + return this->is_missing == var.is_missing && vals == var.vals; + } - bool operator==(const wchar_t *s) const { return !is_missing && val == s; } + bool operator==(const wcstring_list_t &values) const { return !is_missing && vals == values; } - bool operator!=(const env_var_t &v) const { return val != v.val; } - - bool operator!=(const wcstring &s) const { return val != s; } - - bool operator!=(const wchar_t *s) const { return val != s; } + bool operator!=(const env_var_t &var) const { return vals != var.vals; } }; env_var_t create_missing_var(); extern env_var_t missing_var; -/// Gets the variable with the specified name, or env_var_t::missing_var if it does not exist or is -/// an empty array. -/// -/// \param key The name of the variable to get -/// \param mode An optional scope to search in. All scopes are searched if unset +/// This is used to convert a serialized env_var_t back into a list. +wcstring_list_t decode_serialized(const wcstring &s); + +/// Gets the variable with the specified name, or env_var_t::missing_var if it does not exist. env_var_t env_get(const wcstring &key, env_mode_flags_t mode = ENV_DEFAULT); -int env_set(const wcstring &key, env_mode_flags_t mode, const wchar_t *val); +/// Sets the variable with the specified name to the given values. +int env_set(const wcstring &key, env_mode_flags_t mode, const wcstring_list_t &vals); -/// Returns true if the specified key exists. This can't be reliably done using env_get, since -/// env_get returns null for 0-element arrays. -/// -/// \param key The name of the variable to remove -/// \param mode the scope to search in. All scopes are searched if set to default -bool env_exist(const wchar_t *key, env_mode_flags_t mode); +/// Sets the variable with the specified name to a single value. +int env_set_one(const wcstring &key, env_mode_flags_t mode, const wcstring &val); + +/// Sets the variable with the specified name to no values. +int env_set_empty(const wcstring &key, env_mode_flags_t mode); /// Remove environment variable. /// @@ -172,7 +203,7 @@ wcstring env_get_pwd_slash(); void env_set_read_limit(); class env_vars_snapshot_t { - std::map vars; + std::map vars; bool is_current() const; public: @@ -203,8 +234,4 @@ extern bool term_has_xn; // does the terminal have the "eat_newline_glitch" /// Returns true if we think the terminal supports setting its title. bool term_supports_setting_title(); - -/// Returns the fish internal representation for an array of strings. -std::unique_ptr list_to_array_val(const wcstring_list_t &list); -std::unique_ptr list_to_array_val(const wchar_t **list); #endif diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 59ca57800..94da580a9 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -284,7 +284,7 @@ bool env_universal_t::get_export(const wcstring &name) const { return result; } -void env_universal_t::set_internal(const wcstring &key, const wcstring &val, bool exportv, +void env_universal_t::set_internal(const wcstring &key, const wcstring_list_t &vals, bool exportv, bool overwrite) { ASSERT_IS_LOCKED(lock); if (!overwrite && this->modified.find(key) != this->modified.end()) { @@ -293,8 +293,8 @@ void env_universal_t::set_internal(const wcstring &key, const wcstring &val, boo } env_var_t &entry = vars[key]; - if (entry.exportv != exportv || entry.as_string() != val) { - entry.set_val(val); + if (entry.exportv != exportv || entry.as_list() != vals) { + entry.set_vals(vals); entry.exportv = exportv; // If we are overwriting, then this is now modified. @@ -304,9 +304,9 @@ void env_universal_t::set_internal(const wcstring &key, const wcstring &val, boo } } -void env_universal_t::set(const wcstring &key, const wcstring &val, bool exportv) { +void env_universal_t::set(const wcstring &key, const wcstring_list_t &vals, bool exportv) { scoped_lock locker(lock); - this->set_internal(key, val, exportv, true /* overwrite */); + this->set_internal(key, vals, exportv, true /* overwrite */); } bool env_universal_t::remove_internal(const wcstring &key) { @@ -391,8 +391,7 @@ void env_universal_t::acquire_variables(var_table_t &vars_to_acquire) { // source entry in vars since we are about to get rid of this->vars entirely. env_var_t &src = src_iter->second; env_var_t &dst = vars_to_acquire[key]; - dst.set_val(src.as_string()); - dst.exportv = src.exportv; + dst = src; } } @@ -533,7 +532,7 @@ bool env_universal_t::open_temporary_file(const wcstring &directory, wcstring *o const wcstring tmp_name_template = directory + L"/fishd.tmp.XXXXXX"; for (size_t attempt = 0; attempt < 10 && !success; attempt++) { - char *narrow_str = wcs2str(tmp_name_template.c_str()); + char *narrow_str = wcs2str(tmp_name_template); int result_fd = fish_mkstemp_cloexec(narrow_str); saved_errno = errno; success = result_fd != -1; @@ -845,7 +844,8 @@ void env_universal_t::parse_message_internal(const wcstring &msgstr, var_table_t if (unescape_string(tmp + 1, &val, 0)) { env_var_t &entry = (*vars)[key]; entry.exportv = exportv; - entry.set_val(val); // acquire the value + wcstring_list_t values = decode_serialized(val); + entry.set_vals(values); } } else { debug(1, PARSE_ERR, msg); diff --git a/src/env_universal_common.h b/src/env_universal_common.h index bca672ac7..2c9b768a1 100644 --- a/src/env_universal_common.h +++ b/src/env_universal_common.h @@ -44,7 +44,8 @@ class env_universal_t { bool load_from_path(const wcstring &path, callback_data_list_t &callbacks); void load_from_fd(int fd, callback_data_list_t &callbacks); - void set_internal(const wcstring &key, const wcstring &val, bool exportv, bool overwrite); + void set_internal(const wcstring &key, const wcstring_list_t &val, bool exportv, + bool overwrite); bool remove_internal(const wcstring &name); // Functions concerned with saving. @@ -78,7 +79,7 @@ class env_universal_t { bool get_export(const wcstring &name) const; // Sets a variable. - void set(const wcstring &key, const wcstring &val, bool exportv); + void set(const wcstring &key, const wcstring_list_t &val, bool exportv); // Removes a variable. Returns true if it was found, false if not. bool remove(const wcstring &name); diff --git a/src/exec.cpp b/src/exec.cpp index aa82f1bf0..d584642ba 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -211,7 +211,7 @@ static void launch_process_nofork(process_t *p) { convert_wide_array_to_narrow(p->get_argv_array(), &argv_array); const char *const *envv = env_export_arr(); - char *actual_cmd = wcs2str(p->actual_cmd.c_str()); + char *actual_cmd = wcs2str(p->actual_cmd); // Ensure the terminal modes are what they were before we changed them. restore_term_mode(); @@ -379,12 +379,12 @@ void internal_exec(job_t *j, const io_chain_t &&all_ios) { const env_var_t shlvl_var = env_get(L"SHLVL", ENV_GLOBAL | ENV_EXPORT); wcstring shlvl_str = L"0"; if (!shlvl_var.missing()) { - long shlvl = fish_wcstol(shlvl_var.c_str()); + long shlvl = fish_wcstol(shlvl_var.as_string().c_str()); if (!errno && shlvl > 0) { shlvl_str = to_string(shlvl - 1); } } - env_set(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, shlvl_str.c_str()); + env_set_one(L"SHLVL", ENV_GLOBAL | ENV_EXPORT, shlvl_str); // launch_process _never_ returns. launch_process_nofork(j->processes.front().get()); diff --git a/src/expand.cpp b/src/expand.cpp index 9de0a38d7..50168222f 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -142,12 +142,6 @@ static void append_cmdsub_error(parse_error_list_t *errors, size_t source_start, errors->push_back(error); } -/// Return the environment variable value for the string starting at \c in. -static env_var_t expand_var(const wchar_t *in) { - if (!in) return missing_var; - return env_get(in); -} - /// Test if the specified string does not contain character which can not be used inside a quoted /// string. static int is_quotable(const wchar_t *str) { @@ -779,7 +773,7 @@ static int expand_variables(const wcstring &instr, std::vector *ou if (var_len == 1 && var_tmp[0] == VARIABLE_EXPAND_EMPTY) { var = missing_var; } else { - var = expand_var(var_tmp.c_str()); + var = env_get(var_tmp); } if (!var.missing()) { @@ -1187,7 +1181,7 @@ static void expand_home_directory(wcstring &input) { if (retval || !result) { tilde_error = true; } else { - home = str2wcstring(userinfo.pw_dir); + home = env_var_t(L"HOME", str2wcstring(userinfo.pw_dir)); } } @@ -1421,15 +1415,14 @@ static expand_error_t expand_stage_wildcards(const wcstring &input, std::vector< (for_command && path_to_expand.find(L'/') != wcstring::npos)) { effective_working_dirs.push_back(working_dir); } else { - // Get the PATH/CDPATH and cwd. Perhaps these should be passed in. An empty CDPATH + // Get the PATH/CDPATH and CWD. Perhaps these should be passed in. An empty CDPATH // implies just the current directory, while an empty PATH is left empty. env_var_t paths = env_get(for_cd ? L"CDPATH" : L"PATH"); - if (paths.missing_or_empty()) paths = for_cd ? L"." : L""; + if (paths.missing_or_empty()) { + paths = env_var_t(paths.get_name(), for_cd ? L"." : L""); + } - // Tokenize it into path names. - std::vector pathsv; - paths.to_list(pathsv); - for (auto next_path : pathsv) { + for (auto next_path : paths.as_list()) { effective_working_dirs.push_back( path_apply_working_directory(next_path, working_dir)); } diff --git a/src/expand.h b/src/expand.h index 20181a59f..251f02b04 100644 --- a/src/expand.h +++ b/src/expand.h @@ -13,9 +13,10 @@ #include #include "common.h" -#include "env.h" #include "parse_constants.h" +class env_var_t; + enum { /// Flag specifying that cmdsubst expansion should be skipped. EXPAND_SKIP_CMDSUBST = 1 << 0, diff --git a/src/fish.cpp b/src/fish.cpp index 1a45cb9f4..64d2c8e88 100644 --- a/src/fish.cpp +++ b/src/fish.cpp @@ -420,8 +420,7 @@ int main(int argc, char **argv) { for (char **ptr = argv + my_optind; *ptr; ptr++) { list.push_back(str2wcstring(*ptr)); } - auto val = list_to_array_val(list); - env_set(L"argv", ENV_DEFAULT, val->c_str()); + env_set(L"argv", ENV_DEFAULT, list); const wcstring rel_filename = str2wcstring(file); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index c7e6c200f..5b9ab1e86 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -423,7 +423,7 @@ static void test_convert() { o = &sb.at(0); const wcstring w = str2wcstring(o); - n = wcs2str(w.c_str()); + n = wcs2str(w); if (!o || !n) { err(L"Line %d - Conversion cycle of string %s produced null pointer on %s", __LINE__, o, @@ -941,70 +941,6 @@ static void test_parse_util_cmdsubst_extent() { } } -/// Verify the behavior of the `list_to_aray_val()` family of functions. -static void test_list_to_array() { - auto list = wcstring_list_t(); - auto val = list_to_array_val(list); - if (*val != ENV_NULL) { - err(L"test_list_to_array failed on line %lu", __LINE__); - } - - list.push_back(L""); - val = list_to_array_val(list); - if (*val != list[0]) { - err(L"test_list_to_array failed on line %lu", __LINE__); - } - - list.push_back(L"abc"); - val = list_to_array_val(list); - if (*val != L"" ARRAY_SEP_STR L"abc") { - err(L"test_list_to_array failed on line %lu", __LINE__); - } - - list.insert(list.begin(), L"ghi"); - val = list_to_array_val(list); - if (*val != L"ghi" ARRAY_SEP_STR L"" ARRAY_SEP_STR L"abc") { - err(L"test_list_to_array failed on line %lu", __LINE__); - } - - list.push_back(L""); - val = list_to_array_val(list); - if (*val != L"ghi" ARRAY_SEP_STR L"" ARRAY_SEP_STR L"abc" ARRAY_SEP_STR L"") { - err(L"test_list_to_array failed on line %lu", __LINE__); - } - - list.push_back(L"def"); - val = list_to_array_val(list); - if (*val != - L"ghi" ARRAY_SEP_STR L"" ARRAY_SEP_STR L"abc" ARRAY_SEP_STR L"" ARRAY_SEP_STR L"def") { - err(L"test_list_to_array failed on line %lu", __LINE__); - } - - const wchar_t *array1[] = {NULL}; - val = list_to_array_val(array1); - if (*val != ENV_NULL) { - err(L"test_list_to_array failed on line %lu", __LINE__); - } - - const wchar_t *array2[] = {L"abc", NULL}; - val = list_to_array_val(array2); - if (wcscmp(val->c_str(), L"abc") != 0) { - err(L"test_list_to_array failed on line %lu", __LINE__); - } - - const wchar_t *array3[] = {L"abc", L"def", NULL}; - val = list_to_array_val(array3); - if (wcscmp(val->c_str(), L"abc" ARRAY_SEP_STR L"def") != 0) { - err(L"test_list_to_array failed on line %lu", __LINE__); - } - - const wchar_t *array4[] = {L"abc", L"def", L"ghi", NULL}; - val = list_to_array_val(array4); - if (wcscmp(val->c_str(), L"abc" ARRAY_SEP_STR L"def" ARRAY_SEP_STR L"ghi") != 0) { - err(L"test_list_to_array failed on line %lu", __LINE__); - } -} - static struct wcsfilecmp_test { const wchar_t *str1; const wchar_t *str2; @@ -1062,7 +998,6 @@ static void test_wcsfilecmp() { static void test_utility_functions() { say(L"Testing utility functions"); - test_list_to_array(); test_wcsfilecmp(); test_parse_util_cmdsubst_extent(); } @@ -1617,7 +1552,7 @@ static void test_abbreviations(void) { {L"gc", L"git checkout"}, {L"foo", L"bar"}, {L"gx", L"git checkout"}, }; for (auto it : abbreviations) { - int ret = env_set(L"_fish_abbr_" + it.first, ENV_LOCAL, it.second.c_str()); + int ret = env_set_one(L"_fish_abbr_" + it.first, ENV_LOCAL, it.second); if (ret != 0) err(L"Unable to set abbreviation variable"); } @@ -2479,7 +2414,7 @@ static void test_autosuggest_suggest_special() { perform_one_autosuggestion_cd_test(L"cd 'test/autosuggest_test/5", vars, L"foo\"bar/", __LINE__); - env_set(L"AUTOSUGGEST_TEST_LOC", ENV_LOCAL, wd.c_str()); + env_set_one(L"AUTOSUGGEST_TEST_LOC", ENV_LOCAL, wd); perform_one_autosuggestion_cd_test(L"cd $AUTOSUGGEST_TEST_LOC/0", vars, L"foobar/", __LINE__); perform_one_autosuggestion_cd_test(L"cd ~/test_autosuggest_suggest_specia", vars, L"l/", __LINE__); @@ -2621,7 +2556,11 @@ static int test_universal_helper(int x) { for (int j = 0; j < UVARS_PER_THREAD; j++) { const wcstring key = format_string(L"key_%d_%d", x, j); const wcstring val = format_string(L"val_%d_%d", x, j); - uvars.set(key, val, false); + uvars.set(key, + wcstring_list_t({ + val, + }), + false); bool synced = uvars.sync(callbacks); if (!synced) { err(L"Failed to sync universal variables after modification"); @@ -2660,17 +2599,15 @@ static void test_universal() { if (j == 0) { expected_val = missing_var; } else { - expected_val = format_string(L"val_%d_%d", i, j); + expected_val = env_var_t(L"", format_string(L"val_%d_%d", i, j)); } const env_var_t var = uvars.get(key); - if (j == 0) { - assert(expected_val.missing()); - } + if (j == 0) assert(expected_val.missing()); if (var != expected_val) { const wchar_t *missing_desc = L""; err(L"Wrong value for key %ls: expected %ls, got %ls\n", key.c_str(), - (expected_val.missing() ? missing_desc : expected_val.c_str()), - (var.missing() ? missing_desc : var.c_str())); + (expected_val.missing() ? missing_desc : expected_val.as_string().c_str()), + (var.missing() ? missing_desc : var.as_string().c_str())); } } } @@ -2689,27 +2626,75 @@ static void test_universal_callbacks() { env_universal_t uvars2(UVARS_TEST_PATH); // Put some variables into both. - uvars1.set(L"alpha", L"1", false); - uvars1.set(L"beta", L"1", false); - uvars1.set(L"delta", L"1", false); - uvars1.set(L"epsilon", L"1", false); - uvars1.set(L"lambda", L"1", false); - uvars1.set(L"kappa", L"1", false); - uvars1.set(L"omicron", L"1", false); + uvars1.set(L"alpha", + wcstring_list_t({ + L"1", + }), + false); + uvars1.set(L"beta", + wcstring_list_t({ + L"1", + }), + false); + uvars1.set(L"delta", + wcstring_list_t({ + L"1", + }), + false); + uvars1.set(L"epsilon", + wcstring_list_t({ + L"1", + }), + false); + uvars1.set(L"lambda", + wcstring_list_t({ + L"1", + }), + false); + uvars1.set(L"kappa", + wcstring_list_t({ + L"1", + }), + false); + uvars1.set(L"omicron", + wcstring_list_t({ + L"1", + }), + false); uvars1.sync(callbacks); uvars2.sync(callbacks); // Change uvars1. - uvars1.set(L"alpha", L"2", false); // changes value - uvars1.set(L"beta", L"1", true); // changes export - uvars1.remove(L"delta"); // erases value - uvars1.set(L"epsilon", L"1", false); // changes nothing + uvars1.set(L"alpha", + wcstring_list_t({ + L"2", + }), + false); // changes value + uvars1.set(L"beta", + wcstring_list_t({ + L"1", + }), + true); // changes export + uvars1.remove(L"delta"); // erases value + uvars1.set(L"epsilon", + wcstring_list_t({ + L"1", + }), + false); // changes nothing uvars1.sync(callbacks); // Change uvars2. It should treat its value as correct and ignore changes from uvars1. - uvars2.set(L"lambda", L"1", false); // same value - uvars2.set(L"kappa", L"2", false); // different value + uvars2.set(L"lambda", + wcstring_list_t({ + L"1", + }), + false); // same value + uvars2.set(L"kappa", + wcstring_list_t({ + L"2", + }), + false); // different value // Now see what uvars2 sees. callbacks.clear(); @@ -4255,7 +4240,10 @@ long return_timezone_hour(time_t tstamp, const wchar_t *timezone) { char *str_ptr; size_t n; - env_set(L"TZ", ENV_EXPORT, timezone); + env_set_one(L"TZ", ENV_EXPORT, timezone); + + const env_var_t var = env_get(L"TZ", ENV_DEFAULT); + localtime_r(&tstamp, <ime); n = strftime(ltime_str, 3, "%H", <ime); if (n != 2) { diff --git a/src/function.cpp b/src/function.cpp index 9dd3d8264..0eb281325 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -327,28 +327,35 @@ int function_get_definition_offset(const wcstring &name) { return func ? func->definition_offset : -1; } +// Setup the environment for the function. There are three components of the environment: +// 1. argv +// 2. named arguments +// 3. inherited variables void function_prepare_environment(const wcstring &name, const wchar_t *const *argv, const std::map &inherited_vars) { - // Three components of the environment: - // 1. argv - // 2. named arguments - // 3. inherited variables env_set_argv(argv); const wcstring_list_t named_arguments = function_get_named_arguments(name); if (!named_arguments.empty()) { - const wchar_t *const *arg; - size_t i; - for (i = 0, arg = argv; i < named_arguments.size(); i++) { - env_set(named_arguments.at(i).c_str(), ENV_LOCAL | ENV_USER, *arg); - - if (*arg) arg++; + const wchar_t *const *arg = argv; + for (size_t i = 0; i < named_arguments.size(); i++) { + if (*arg) { + env_set_one(named_arguments.at(i), ENV_LOCAL | ENV_USER, *arg); + arg++; + } else { + env_set_empty(named_arguments.at(i), ENV_LOCAL | ENV_USER); + } } } - for (std::map::const_iterator it = inherited_vars.begin(), - end = inherited_vars.end(); - it != end; ++it) { - env_set(it->first, ENV_LOCAL | ENV_USER, it->second.missing() ? NULL : it->second.c_str()); + for (auto it = inherited_vars.begin(), end = inherited_vars.end(); it != end; ++it) { + // Note: Prior to my rewrite to address issue #4200 this code did the equivalent of this: + // if (it->second.missing()) { + // env_set_empty(it->first, ENV_LOCAL | ENV_USER); + // } else { + // It should be impossible for the var to be missing since we're inheriting it from an outer + // scope. So we now die horribly if it is missing. + assert(!it->second.missing()); + env_set(it->first, ENV_LOCAL | ENV_USER, it->second.as_const_list()); } } diff --git a/src/highlight.cpp b/src/highlight.cpp index 7186db127..adefd969b 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -222,7 +222,7 @@ static bool is_potential_cd_path(const wcstring &path, const wcstring &working_d } else { // Get the CDPATH. env_var_t cdpath = env_get(L"CDPATH"); - if (cdpath.missing_or_empty()) cdpath = env_var_t(L"."); + if (cdpath.missing_or_empty()) cdpath = env_var_t(cdpath.get_name(), L"."); // Tokenize it into directories. std::vector pathsv; @@ -359,7 +359,8 @@ bool autosuggest_validate_from_history(const history_item_t &item, string_prefixes_string(dir, L"--help") || string_prefixes_string(dir, L"-h"); if (!is_help) { wcstring path; - bool can_cd = path_get_cdpath(dir, &path, working_directory.c_str(), vars); + env_var_t dir_var(L"n/a", dir); + bool can_cd = path_get_cdpath(dir_var, &path, working_directory.c_str(), vars); if (can_cd && !paths_are_same_file(working_directory, path)) { suggestionOK = true; } diff --git a/src/history.cpp b/src/history.cpp index a293933b9..09fe05a88 100644 --- a/src/history.cpp +++ b/src/history.cpp @@ -1266,7 +1266,7 @@ bool history_t::rewrite_to_temporary_file(int existing_fd, int dst_fd) const { static int create_temporary_file(const wcstring &name_template, wcstring *out_path) { int out_fd = -1; for (size_t attempt = 0; attempt < 10 && out_fd == -1; attempt++) { - char *narrow_str = wcs2str(name_template.c_str()); + char *narrow_str = wcs2str(name_template); out_fd = fish_mkstemp_cloexec(narrow_str); if (out_fd >= 0) { *out_path = str2wcstring(narrow_str); diff --git a/src/input.cpp b/src/input.cpp index 456e07766..ee578160c 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -211,7 +211,7 @@ void input_set_bind_mode(const wcstring &bm) { // modes may not be empty - empty is a sentinel value meaning to not change the mode assert(!bm.empty()); if (input_get_bind_mode() != bm.c_str()) { - env_set(FISH_BIND_MODE_VAR, ENV_GLOBAL, bm.c_str()); + env_set_one(FISH_BIND_MODE_VAR, ENV_GLOBAL, bm); } } diff --git a/src/input_common.cpp b/src/input_common.cpp index 9cb994fe8..1cf7ce357 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -164,12 +164,12 @@ void update_wait_on_escape_ms() { return; } - long tmp = fish_wcstol(escape_time_ms.c_str()); + long tmp = fish_wcstol(escape_time_ms.as_string().c_str()); if (errno || tmp < 10 || tmp >= 5000) { fwprintf(stderr, L"ignoring fish_escape_delay_ms: value '%ls' " L"is not an integer or is < 10 or >= 5000 ms\n", - escape_time_ms.c_str()); + escape_time_ms.as_string().c_str()); } else { wait_on_escape_ms = (int)tmp; } diff --git a/src/output.cpp b/src/output.cpp index 9ec4a29cd..b160822c5 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -558,6 +558,6 @@ void writembs_check(char *mbs, const char *mbs_name, const char *file, long line const wchar_t *fmt = _(L"Tried to use terminfo string %s on line %ld of %s, which is " L"undefined in terminal of type \"%ls\". Please report this error to %s"); - debug(0, fmt, mbs_name, line, file, term.c_str(), PACKAGE_BUGREPORT); + debug(0, fmt, mbs_name, line, file, term.as_string().c_str(), PACKAGE_BUGREPORT); } } diff --git a/src/output.h b/src/output.h index 58a871450..3348183a8 100644 --- a/src/output.h +++ b/src/output.h @@ -10,9 +10,10 @@ #include #include "color.h" -#include "common.h" #include "fallback.h" // IWYU pragma: keep +class env_var_t; + /// Constants for various colors as used by the set_color function. enum { FISH_COLOR_BLACK, // 0 diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 1b1557711..b243ef466 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -473,7 +473,9 @@ parse_execution_result_t parse_execution_context_t::run_for_statement( } const wcstring &val = argument_sequence.at(i); - env_set(for_var_name, ENV_LOCAL, val.c_str()); + // This is wrong. It should or in ENV_USER and test if ENV_PERM is returned. + // TODO: Fix this so it correctly handles read-only vars. + env_set_one(for_var_name, ENV_LOCAL, val); fb->loop_status = LOOP_NORMAL; this->run_job_list(block_contents, fb); diff --git a/src/path.cpp b/src/path.cpp index 360975355..13bf23778 100644 --- a/src/path.cpp +++ b/src/path.cpp @@ -24,6 +24,13 @@ /// Unexpected error in path_get_path(). #define MISSING_COMMAND_ERR_MSG _(L"Error while searching for command '%ls'") +// Note that PREFIX is defined in the `Makefile` and is thus defined when this module is compiled. +// This ensures we always default to "/bin", "/usr/bin" and the bin dir defined for the fish +// programs. Possibly with a duplicate dir if PREFIX is empty, "/", "/usr" or "/usr/". If the PREFIX +// duplicates /bin or /usr/bin that is harmless other than a trivial amount of time testing a path +// we've already tested. +const wcstring_list_t dflt_pathsv({L"/bin", L"/usr/bin", PREFIX L"/bin"}); + static bool path_get_path_core(const wcstring &cmd, wcstring *out_path, const env_var_t &bin_path_var) { debug(3, L"path_get_path( '%ls' )", cmd.c_str()); @@ -47,20 +54,15 @@ static bool path_get_path_core(const wcstring &cmd, wcstring *out_path, return false; } - int err = ENOENT; - wcstring_list_t pathsv; + const wcstring_list_t *pathsv; if (!bin_path_var.missing()) { - bin_path_var.to_list(pathsv); + pathsv = &bin_path_var.as_const_list(); } else { - // Note that PREFIX is defined in the `Makefile` and is thus defined when this module is - // compiled. This ensures we always default to "/bin", "/usr/bin" and the bin dir defined - // for the fish programs. Possibly with a duplicate dir if PREFIX is empty, "/", "/usr" or - // "/usr/". If the PREFIX duplicates /bin or /usr/bin that is harmless other than a trivial - // amount of time testing a path we've already tested. - pathsv = wcstring_list_t({L"/bin", L"/usr/bin", PREFIX L"/bin"}); + pathsv = &dflt_pathsv; } - for (auto next_path : pathsv) { + int err = ENOENT; + for (auto next_path : *pathsv) { if (next_path.empty()) continue; append_path_component(next_path, cmd); if (waccess(next_path, X_OK) == 0) { @@ -164,7 +166,7 @@ bool path_get_cdpath(const env_var_t &dir_var, wcstring *out, const wchar_t *wd, } else { // Respect CDPATH. env_var_t cdpaths = env_vars.get(L"CDPATH"); - if (cdpaths.missing_or_empty()) cdpaths = L"."; + if (cdpaths.missing_or_empty()) cdpaths = env_var_t(cdpaths.get_name(), L"."); std::vector cdpathsv; cdpaths.to_list(cdpathsv); @@ -214,7 +216,8 @@ bool path_can_be_implicit_cd(const wcstring &path, wcstring *out_path, const wch exp_path == L"..") { // These paths can be implicit cd, so see if you cd to the path. Note that a single period // cannot (that's used for sourcing files anyways). - result = path_get_cdpath(exp_path, out_path, wd, vars); + env_var_t path_var(L"n/a", exp_path); + result = path_get_cdpath(path_var, out_path, wd, vars); } return result; } @@ -257,10 +260,9 @@ static void maybe_issue_path_warning(const wcstring &which_dir, const wcstring & bool using_xdg, const wcstring &xdg_var, const wcstring &path, int saved_errno) { wcstring warning_var_name = L"_FISH_WARNED_" + which_dir; - if (env_exist(warning_var_name.c_str(), ENV_GLOBAL | ENV_EXPORT)) { - return; - } - env_set(warning_var_name, ENV_GLOBAL | ENV_EXPORT, L"1"); + env_var_t var = env_get(warning_var_name, ENV_GLOBAL | ENV_EXPORT); + if (var.missing()) return; + env_set_one(warning_var_name, ENV_GLOBAL | ENV_EXPORT, L"1"); debug(0, custom_error_msg.c_str()); if (path.empty()) { diff --git a/src/reader.cpp b/src/reader.cpp index 67daf89ff..c480ecc73 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -696,7 +696,7 @@ void reader_write_title(const wcstring &cmd, bool reset_cursor_position) { for (size_t i = 0; i < lst.size(); i++) { fputws(lst.at(i).c_str(), stdout); } - ignore_result(write(STDOUT_FILENO, "\a", 1)); + ignore_result(write(STDOUT_FILENO, "\a", 1)); } proc_pop_interactive(); @@ -766,7 +766,7 @@ void reader_init() { // Ensure this var is present even before an interactive command is run so that if it is used // in a function like `fish_prompt` or `fish_right_prompt` it is defined at the time the first // prompt is written. - env_set(ENV_CMD_DURATION, ENV_UNEXPORT, L"0"); + env_set_one(ENV_CMD_DURATION, ENV_UNEXPORT, L"0"); // Save the initial terminal mode. tcgetattr(STDIN_FILENO, &terminal_mode_on_startup); @@ -1650,7 +1650,7 @@ static void reader_interactive_init() { wperror(L"tcsetattr"); } - env_set(L"_", ENV_GLOBAL, L"fish"); + env_set_one(L"_", ENV_GLOBAL, L"fish"); } /// Destroy data for interactive use. @@ -1917,7 +1917,7 @@ void set_env_cmd_duration(struct timeval *after, struct timeval *before) { } swprintf(buf, 16, L"%d", (secs * 1000) + (usecs / 1000)); - env_set(ENV_CMD_DURATION, ENV_UNEXPORT, buf); + env_set_one(ENV_CMD_DURATION, ENV_UNEXPORT, buf); } void reader_run_command(parser_t &parser, const wcstring &cmd) { @@ -1925,7 +1925,7 @@ void reader_run_command(parser_t &parser, const wcstring &cmd) { wcstring ft = tok_first(cmd); - if (!ft.empty()) env_set(L"_", ENV_GLOBAL, ft.c_str()); + if (!ft.empty()) env_set_one(L"_", ENV_GLOBAL, ft); reader_write_title(cmd); @@ -1941,7 +1941,7 @@ void reader_run_command(parser_t &parser, const wcstring &cmd) { term_steal(); - env_set(L"_", ENV_GLOBAL, program_name); + env_set_one(L"_", ENV_GLOBAL, program_name); #ifdef HAVE__PROC_SELF_STAT proc_update_jiffies(); diff --git a/src/wutil.cpp b/src/wutil.cpp index d777555bf..30d7ff2bd 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -398,7 +398,7 @@ wchar_t *wrealpath(const wcstring &pathname, wchar_t *resolved_path) { } wcstring wdirname(const wcstring &path) { - char *tmp = wcs2str(path.c_str()); + char *tmp = wcs2str(path); char *narrow_res = dirname(tmp); wcstring result = format_string(L"%s", narrow_res); free(tmp); @@ -406,7 +406,7 @@ wcstring wdirname(const wcstring &path) { } wcstring wbasename(const wcstring &path) { - char *tmp = wcs2str(path.c_str()); + char *tmp = wcs2str(path); char *narrow_res = basename(tmp); wcstring result = format_string(L"%s", narrow_res); free(tmp); diff --git a/tests/read.in b/tests/read.in index cd5bfa07e..50d28df3b 100644 --- a/tests/read.in +++ b/tests/read.in @@ -91,8 +91,6 @@ echo '' | read -la ary print_vars ary set -le IFS -# read -n tests - logmsg read -n tests echo 'testing' | read -n 3 foo echo $foo @@ -106,8 +104,6 @@ echo $bar echo 'test' | read -n 1 foo echo $foo -# read -0 tests - logmsg read -z tests echo -n 'testing' | read -lz foo echo $foo diff --git a/tests/test3.in b/tests/test3.in index 4387f9e94..dacf46144 100644 --- a/tests/test3.in +++ b/tests/test3.in @@ -301,7 +301,8 @@ env DISPLAY="localhost:0.0" ../test/root/bin/fish -c 'echo Elements in DISPLAY: # We can't use PATH for this because the global configuration will modify PATH # based on /etc/paths and /etc/paths.d. # Exported arrays should use record separator, with a few exceptions. So we can use an arbitrary variable for this. -env FOO=one\x1etwo\x1ethree\x1efour ../test/root/bin/fish -c 'echo Elements in FOO: (count $FOO)' +set -gx FOO one two three four +../test/root/bin/fish -c 'echo Elements in FOO: (count $FOO)' # some must use colon separators! set -lx MANPATH man1 man2 man3 ; env | grep '^MANPATH='