From 20cb62440c2dad61de80ba6ea2f942b720309d3a Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 29 Sep 2018 00:20:50 -0400 Subject: [PATCH] Eliminate some mutable global variables Make them const or otherwise get rid of them --- build_tools/find_globals.fish | 13 +++++++++++- src/builtin_set_color.cpp | 6 +++--- src/common.cpp | 39 ++++++++++++++++++++++++----------- src/common.h | 5 ++--- src/complete.cpp | 30 +++++++++------------------ src/env.cpp | 9 -------- src/fish.cpp | 4 ++-- src/fish_version.cpp | 2 +- 8 files changed, 57 insertions(+), 51 deletions(-) diff --git a/build_tools/find_globals.fish b/build_tools/find_globals.fish index 24338bca4..2373cdf99 100755 --- a/build_tools/find_globals.fish +++ b/build_tools/find_globals.fish @@ -5,13 +5,24 @@ # This was written for macOS nm. set total_globals 0 +set boring_files fish_key_reader.cpp.o +set whitelist \ + termsize_lock termsize \ + initial_fg_process_group \ + _debug_level \ + sitm_esc ritm_esc dim_esc \ + s_main_thread_performer_lock s_main_thread_performer_cond s_main_thread_request_q_lock for file in ./**.o set filename (basename $file) + # Skip boring files. + contains $filename $boring_files ; and continue for line in (nm -p -P -U $file) # Look in data (dD) and bss (bB) segments. set matches (string match --regex '^([^ ]+) ([dDbB])' -- $line) ; or continue - echo $filename (echo $matches[2] | c++filt) $matches[3] + set symname (echo $matches[2] | c++filt) + contains $symname $whitelist ; and continue + echo $filename $symname $matches[3] set total_globals (math $total_globals + 1) end end diff --git a/src/builtin_set_color.cpp b/src/builtin_set_color.cpp index 13ae662f7..78857285e 100644 --- a/src/builtin_set_color.cpp +++ b/src/builtin_set_color.cpp @@ -62,9 +62,9 @@ static const struct woption long_options[] = {{L"background", required_argument, {NULL, 0, NULL, 0}}; #if __APPLE__ -char sitm_esc[] = "\x1B[3m"; -char ritm_esc[] = "\x1B[23m"; -char dim_esc[] = "\x1B[2m"; +static char sitm_esc[] = "\x1B[3m"; +static char ritm_esc[] = "\x1B[23m"; +static char dim_esc[] = "\x1B[2m"; #endif /// set_color builtin. diff --git a/src/common.cpp b/src/common.cpp index b5d2a8bde..ffdbb49e0 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -64,6 +64,7 @@ int debug_stack_frames = 0; // default number of stack frames to show on debug( static pid_t initial_pid = 0; /// Be able to restore the term's foreground process group. +/// This is set during startup and not modified after. static pid_t initial_fg_process_group = -1; /// This struct maintains the current state of the terminal size. It is updated on demand after @@ -76,23 +77,34 @@ static volatile bool termsize_valid = false; static char *wcs2str_internal(const wchar_t *in, char *out); static void debug_shared(const wchar_t msg_level, const wcstring &msg); -const wcstring whitespace = L" \t\r\n\v"; -const char *whitespace_narrow = " \t\r\n\v"; - -bool is_whitespace(const wchar_t input) { - for (auto c : whitespace) { - if (c == input) { +bool is_whitespace(wchar_t c) { + switch (c) { + case ' ': + case '\t': + case '\r': + case '\n': + case '\v': return true; - } + default: + return false; } - return false; } bool is_whitespace(const wcstring &input) { - return input.find_first_not_of(whitespace) == wcstring::npos; + bool (*pred)(wchar_t c) = is_whitespace; + return std::all_of(input.begin(), input.end(), pred); } -bool has_working_tty_timestamps = true; +#if defined(OS_IS_CYGWIN) || defined(WSL) +// MS Windows tty devices do not currently have either a read or write timestamp. Those +// respective fields of `struct stat` are always the current time. Which means we can't +// use them. So we assume no external program has written to the terminal behind our +// back. This makes multiline promptusable. See issue #2859 and +// https://github.com/Microsoft/BashOnWindows/issues/545 +const bool has_working_tty_timestamps = false; +#else +const bool has_working_tty_timestamps = true; +#endif /// Convert a character to its integer equivalent if it is a valid character for the requested base. /// Return the integer value if it is valid else -1. @@ -598,7 +610,7 @@ static void debug_shared(const wchar_t level, const wcstring &msg) { } } -static wchar_t level_char[] = {L'E', L'W', L'2', L'3', L'4', L'5'}; +static const wchar_t level_char[] = {L'E', L'W', L'2', L'3', L'4', L'5'}; void __attribute__((noinline)) debug_impl(int level, const wchar_t *msg, ...) { int errno_old = errno; va_list va; @@ -2065,7 +2077,10 @@ void setup_fork_guards() { initial_pid = getpid(); } -void save_term_foreground_process_group() { initial_fg_process_group = tcgetpgrp(STDIN_FILENO); } +void save_term_foreground_process_group() { + ASSERT_IS_MAIN_THREAD(); + initial_fg_process_group = tcgetpgrp(STDIN_FILENO); +} void restore_term_foreground_process_group() { if (initial_fg_process_group == -1) return; diff --git a/src/common.h b/src/common.h index f3832bb8b..f0d18cabc 100644 --- a/src/common.h +++ b/src/common.h @@ -206,9 +206,8 @@ extern bool g_profiling_active; /// Name of the current program. Should be set at startup. Used by the debug function. extern const wchar_t *program_name; -/// Set to false at run-time if it's been determined we can't trust the last modified timestamp on -/// the tty. -extern bool has_working_tty_timestamps; +/// Set to false if it's been determined we can't trust the last modified timestamp on the tty. +extern const bool has_working_tty_timestamps; /// A list of all whitespace characters extern const wcstring whitespace; diff --git a/src/complete.cpp b/src/complete.cpp index e1d5cfe76..8b6ae652c 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -1649,18 +1649,8 @@ wcstring complete_print() { void complete_invalidate_path() { completion_autoloader.invalidate(); } /// Completion "wrapper" support. The map goes from wrapping-command to wrapped-command-list. -static fish_mutex_t wrapper_lock; -typedef std::unordered_map wrapper_map_t; -static wrapper_map_t &wrap_map() { - ASSERT_IS_LOCKED(wrapper_lock); - // A pointer is a little more efficient than an object as a static because we can elide the - // thread-safe initialization. - static wrapper_map_t *wrapper_map = NULL; - if (wrapper_map == NULL) { - wrapper_map = new wrapper_map_t(); - } - return *wrapper_map; -} +using wrapper_map_t = std::unordered_map; +static owning_lock wrapper_map; /// Add a new target that wraps a command. Example: __fish_XYZ (function) wraps XYZ (target). bool complete_add_wrapper(const wcstring &command, const wcstring &new_target) { @@ -1668,8 +1658,8 @@ bool complete_add_wrapper(const wcstring &command, const wcstring &new_target) { return false; } - scoped_lock locker(wrapper_lock); - wrapper_map_t &wraps = wrap_map(); + auto locked_map = wrapper_map.acquire(); + wrapper_map_t &wraps = *locked_map; wcstring_list_t *targets = &wraps[command]; // If it's already present, we do nothing. if (!contains(*targets, new_target)) { @@ -1683,8 +1673,8 @@ bool complete_remove_wrapper(const wcstring &command, const wcstring &target_to_ return false; } - scoped_lock locker(wrapper_lock); - wrapper_map_t &wraps = wrap_map(); + auto locked_map = wrapper_map.acquire(); + wrapper_map_t &wraps = *locked_map; bool result = false; wrapper_map_t::iterator current_targets_iter = wraps.find(command); if (current_targets_iter != wraps.end()) { @@ -1702,14 +1692,14 @@ wcstring_list_t complete_get_wrap_targets(const wcstring &command) { if (command.empty()) { return {}; } - scoped_lock locker(wrapper_lock); - const wrapper_map_t &wraps = wrap_map(); + auto locked_map = wrapper_map.acquire(); + wrapper_map_t &wraps = *locked_map; auto iter = wraps.find(command); if (iter == wraps.end()) return {}; return iter->second; } tuple_list complete_get_wrap_pairs() { - scoped_lock locker(wrapper_lock); - return flatten(wrap_map()); + auto locked_map = wrapper_map.acquire(); + return flatten(*locked_map); } diff --git a/src/env.cpp b/src/env.cpp index b3adf9a0e..73ac724da 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -736,15 +736,6 @@ void misc_init() { fflush(stdout); setvbuf(stdout, NULL, _IONBF, 0); } - -#if defined(OS_IS_CYGWIN) || defined(WSL) - // MS Windows tty devices do not currently have either a read or write timestamp. Those - // respective fields of `struct stat` are always the current time. Which means we can't - // use them. So we assume no external program has written to the terminal behind our - // back. This makes multiline promptusable. See issue #2859 and - // https://github.com/Microsoft/BashOnWindows/issues/545 - has_working_tty_timestamps = false; -#endif } static void env_universal_callbacks(callback_data_list_t &callbacks) { diff --git a/src/fish.cpp b/src/fish.cpp index baeab0dd0..0ca626425 100644 --- a/src/fish.cpp +++ b/src/fish.cpp @@ -241,7 +241,7 @@ int run_command_list(std::vector *cmds, const io_chain_t &io) { /// Parse the argument list, return the index of the first non-flag arguments. static int fish_parse_opt(int argc, char **argv, fish_cmd_opts_t *opts) { - static const char *short_opts = "+hilnvc:C:p:d:f:D:"; + static const char * const short_opts = "+hilnvc:C:p:d:f:D:"; static const struct option long_opts[] = {{"command", required_argument, NULL, 'c'}, {"init-command", required_argument, NULL, 'C'}, {"features", required_argument, NULL, 'f'}, @@ -361,8 +361,8 @@ int main(int argc, char **argv) { // struct stat tmp; // stat("----------FISH_HIT_MAIN----------", &tmp); + const char *dummy_argv[2] = {"fish", NULL}; if (!argv[0]) { - static const char *dummy_argv[2] = {"fish", NULL}; argv = (char **)dummy_argv; //!OCLINT(parameter reassignment) argc = 1; //!OCLINT(parameter reassignment) } diff --git a/src/fish_version.cpp b/src/fish_version.cpp index e64ba878e..d3095575b 100644 --- a/src/fish_version.cpp +++ b/src/fish_version.cpp @@ -8,7 +8,7 @@ // The contents of FISH-BUILD-VERSION-FILE looks like: // FISH_BUILD_VERSION="2.7.1-62-gc0480092-dirty" // Arrange for it to become a variable. -static const char * +static const char * const #include "FISH-BUILD-VERSION-FILE" ; #endif