From c44dae2d73a9131e5fe7b82e11eedb8358dada03 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 12 May 2019 15:48:00 -0700 Subject: [PATCH] Migrate certain runtime flags to atomics hidden behind functions --- src/builtin.cpp | 2 +- src/builtin_commandline.cpp | 2 +- src/builtin_status.cpp | 6 +++--- src/env_dispatch.cpp | 6 +++--- src/fish.cpp | 31 +++++++++++++++++++++---------- src/fish_key_reader.cpp | 2 +- src/fish_tests.cpp | 5 +++-- src/proc.cpp | 16 +++++++++++----- src/proc.h | 20 +++++++++++--------- src/reader.cpp | 6 +++--- 10 files changed, 58 insertions(+), 38 deletions(-) diff --git a/src/builtin.cpp b/src/builtin.cpp index a87b7ef5e..ffc2be4d9 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -468,7 +468,7 @@ proc_status_t builtin_run(parser_t &parser, int job_pgid, wchar_t **argv, io_str if (const builtin_data_t *data = builtin_lookup(argv[0])) { // If we are interactive, save the foreground pgroup and restore it after in case the // builtin needs to read from the terminal. See #4540. - bool grab_tty = is_interactive_session && isatty(streams.stdin_fd); + bool grab_tty = is_interactive_session() && isatty(streams.stdin_fd); pid_t pgroup_to_restore = grab_tty ? terminal_acquire_before_builtin(job_pgid) : -1; int ret = data->func(parser, streams, argv); if (pgroup_to_restore >= 0) { diff --git a/src/builtin_commandline.cpp b/src/builtin_commandline.cpp index d37e87384..b8ab7e82b 100644 --- a/src/builtin_commandline.cpp +++ b/src/builtin_commandline.cpp @@ -188,7 +188,7 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv) } if (!current_buffer) { - if (is_interactive_session) { + if (is_interactive_session()) { // Prompt change requested while we don't have a prompt, most probably while reading the // init files. Just ignore it. return STATUS_CMD_ERROR; diff --git a/src/builtin_status.cpp b/src/builtin_status.cpp index 1230d6e5c..e800ac1d2 100644 --- a/src/builtin_status.cpp +++ b/src/builtin_status.cpp @@ -304,7 +304,7 @@ int builtin_status(parser_t &parser, io_streams_t &streams, wchar_t **argv) { switch (opts.status_cmd) { case STATUS_UNDEF: { CHECK_FOR_UNEXPECTED_STATUS_ARGS(opts.status_cmd) - if (is_login) { + if (get_login()) { streams.out.append_format(_(L"This is a login shell\n")); } else { streams.out.append_format(_(L"This is not a login shell\n")); @@ -384,7 +384,7 @@ int builtin_status(parser_t &parser, io_streams_t &streams, wchar_t **argv) { } case STATUS_IS_INTERACTIVE: { CHECK_FOR_UNEXPECTED_STATUS_ARGS(opts.status_cmd) - retval = !is_interactive_session; + retval = !is_interactive_session(); break; } case STATUS_IS_COMMAND_SUB: { @@ -404,7 +404,7 @@ int builtin_status(parser_t &parser, io_streams_t &streams, wchar_t **argv) { } case STATUS_IS_LOGIN: { CHECK_FOR_UNEXPECTED_STATUS_ARGS(opts.status_cmd) - retval = !is_login; + retval = !get_login(); break; } case STATUS_IS_FULL_JOB_CTRL: { diff --git a/src/env_dispatch.cpp b/src/env_dispatch.cpp index ce21444d8..cae845b6a 100644 --- a/src/env_dispatch.cpp +++ b/src/env_dispatch.cpp @@ -402,11 +402,11 @@ static bool initialize_curses_using_fallback(const char *term) { auto term_env = wcs2string(term_var->as_string()); if (term_env == DEFAULT_TERM1 || term_env == DEFAULT_TERM2) return false; - if (is_interactive_session) debug(1, _(L"Using fallback terminal type '%s'."), term); + if (is_interactive_session()) debug(1, _(L"Using fallback terminal type '%s'."), term); int err_ret; if (setupterm((char *)term, STDOUT_FILENO, &err_ret) == OK) return true; - if (is_interactive_session) { + if (is_interactive_session()) { debug(1, _(L"Could not set up terminal using the fallback terminal type '%s'."), term); } return false; @@ -465,7 +465,7 @@ static void init_curses(const environment_t &vars) { int err_ret; if (setupterm(NULL, STDOUT_FILENO, &err_ret) == ERR) { auto term = vars.get(L"TERM"); - if (is_interactive_session) { + if (is_interactive_session()) { debug(1, _(L"Could not set up terminal.")); if (term.missing_or_empty()) { debug(1, _(L"TERM environment variable not set.")); diff --git a/src/fish.cpp b/src/fish.cpp index afc58c4bd..e79b96811 100644 --- a/src/fish.cpp +++ b/src/fish.cpp @@ -66,6 +66,12 @@ class fish_cmd_opts_t { std::vector postconfig_cmds; /// Whether to print rusage-self stats after execution. bool print_rusage_self{false}; + /// Whether no-exec is set. + bool no_exec{false}; + /// Whether this is a login shell. + bool is_login{false}; + /// Whether this is an interactive session. + bool is_interactive_session{false}; }; /// If we are doing profiling, the filename to output to. @@ -302,15 +308,15 @@ static int fish_parse_opt(int argc, char **argv, fish_cmd_opts_t *opts) { break; } case 'i': { - is_interactive_session = true; + opts->is_interactive_session = true; break; } case 'l': { - is_login = true; + opts->is_login = true; break; } case 'n': { - set_no_exec(true); + opts->no_exec = true; break; } case 1: { @@ -356,13 +362,13 @@ static int fish_parse_opt(int argc, char **argv, fish_cmd_opts_t *opts) { } // If our command name begins with a dash that implies we're a login shell. - is_login |= argv[0][0] == '-'; + opts->is_login |= argv[0][0] == '-'; // We are an interactive session if we have not been given an explicit // command or file to execute and stdin is a tty. Note that the -i or // --interactive options also force interactive mode. if (opts->batch_cmds.size() == 0 && optind == argc && isatty(STDIN_FILENO)) { - is_interactive_session = 1; + set_interactive_session(true); } return optind; @@ -386,18 +392,23 @@ int main(int argc, char **argv) { argv = (char **)dummy_argv; //!OCLINT(parameter reassignment) argc = 1; //!OCLINT(parameter reassignment) } - fish_cmd_opts_t opts; + fish_cmd_opts_t opts{}; my_optind = fish_parse_opt(argc, argv, &opts); // No-exec is prohibited when in interactive mode. - if (is_interactive_session && no_exec()) { + if (opts.is_interactive_session && opts.no_exec) { debug(1, _(L"Can not use the no-execute mode when running an interactive session")); - set_no_exec(false); + opts.no_exec = false; } + // Apply our options. + if (opts.is_login) mark_login(); + if (opts.no_exec) mark_no_exec(); + if (opts.is_interactive_session) set_interactive_session(true); + // Only save (and therefore restore) the fg process group if we are interactive. See issues // #197 and #1002. - if (is_interactive_session) { + if (is_interactive_session()) { save_term_foreground_process_group(); } @@ -430,7 +441,7 @@ int main(int argc, char **argv) { if (!opts.batch_cmds.empty()) { // Run the commands specified as arguments, if any. - if (is_login) { + if (get_login()) { // Do something nasty to support OpenSUSE assuming we're bash. This may modify cmds. fish_xdm_login_hack_hack_hack_hack(&opts.batch_cmds, argc - my_optind, argv + my_optind); diff --git a/src/fish_key_reader.cpp b/src/fish_key_reader.cpp index 0100d690d..4f802eb69 100644 --- a/src/fish_key_reader.cpp +++ b/src/fish_key_reader.cpp @@ -281,7 +281,7 @@ static void install_our_signal_handlers() { /// Setup our environment (e.g., tty modes), process key strokes, then reset the environment. static void setup_and_process_keys(bool continuous_mode) { - is_interactive_session = 1; // by definition this program is interactive + set_interactive_session(true); // by definition this program is interactive set_main_thread(); setup_fork_guards(); proc_push_interactive(1); diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index c74dc0974..77c26b494 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -1031,10 +1031,11 @@ static void test_cancellation() { // Test for #3780 // Ugly hack - temporarily set is_interactive_session // else we will SIGINT ourselves in response to our child death - scoped_push iis(&is_interactive_session, true); + bool iis = is_interactive_session(); + set_interactive_session(true); const wchar_t *child_self_destructor = L"while true ; sh -c 'sleep .25; kill -s INT $$' ; end"; parser_t::principal_parser().eval(child_self_destructor, io_chain_t(), TOP); - iis.restore(); + set_interactive_session(iis); // Restore signal handling. proc_pop_interactive(); diff --git a/src/proc.cpp b/src/proc.cpp index b6f284e3e..cb0284eaa 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -52,16 +52,22 @@ /// The signals that signify crashes to us. static const int crashsignals[] = {SIGABRT, SIGBUS, SIGFPE, SIGILL, SIGSEGV, SIGSYS}; -bool is_interactive_session = false; bool is_subshell = false; bool is_block = false; bool is_breakpoint = false; -bool is_login = false; int is_event = 0; -static relaxed_atomic_bool_t s_no_exec; +static relaxed_atomic_bool_t s_is_interactive_session{false}; +bool is_interactive_session() { return s_is_interactive_session; } +void set_interactive_session(bool flag) { s_is_interactive_session = flag; } + +static relaxed_atomic_bool_t s_is_login{false}; +bool get_login() { return s_is_login; } +void mark_login() { s_is_login = true; } + +static relaxed_atomic_bool_t s_no_exec{false}; bool no_exec() { return s_no_exec; } -void set_no_exec(bool flag) { s_no_exec = flag; } +void mark_no_exec() { s_no_exec = true; } bool have_proc_stat() { // Check for /proc/self/stat to see if we are running with Linux-style procfs. @@ -245,7 +251,7 @@ static void handle_child_status(process_t *proc, proc_status_t status) { if (status.signal_exited()) { int sig = status.signal_code(); if (sig == SIGINT || sig == SIGQUIT) { - if (is_interactive_session) { + if (is_interactive_session()) { // In an interactive session, tell the principal parser to skip all blocks we're // executing so control-C returns control to the user. parser_t::skip_all_blocks(); diff --git a/src/proc.h b/src/proc.h index d07a5ecb4..24f1336d3 100644 --- a/src/proc.h +++ b/src/proc.h @@ -415,7 +415,7 @@ class job_t { }; /// Whether we are reading from the keyboard right now. -bool shell_is_interactive(void); +bool shell_is_interactive(); /// Whether we are running a subshell command. extern bool is_subshell; @@ -427,10 +427,18 @@ extern bool is_block; extern bool is_breakpoint; /// Whether this shell is attached to the keyboard at all. -extern bool is_interactive_session; +bool is_interactive_session(); +void set_interactive_session(bool flag); /// Whether we are a login shell. -extern bool is_login; +bool get_login(); +void mark_login(); + +/// If this flag is set, fish will never fork or run execve. It is used to put fish into a syntax +/// verifier mode where fish tries to validate the syntax of a file but doesn't actually do +/// anything. +bool no_exec(); +void mark_no_exec(); /// Whether we are running an event handler. This is not a bool because we keep count of the event /// nesting level. @@ -445,12 +453,6 @@ typedef std::deque> job_list_t; job_control_t get_job_control_mode(); void set_job_control_mode(job_control_t mode); -/// If this flag is set, fish will never fork or run execve. It is used to put fish into a syntax -/// verifier mode where fish tries to validate the syntax of a file but doesn't actually do -/// anything. -bool no_exec(); -void set_no_exec(bool flag); - /// Notify the user about stopped or terminated jobs, and delete completed jobs from the job list. /// If \p interactive is set, allow removing interactive jobs; otherwise skip them. /// \return whether text was printed to stdout. diff --git a/src/reader.cpp b/src/reader.cpp index e6998bc25..203bfd2e9 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1731,7 +1731,7 @@ static void reader_interactive_init() { owner = tcgetpgrp(STDIN_FILENO); } if (owner == -1 && errno == ENOTTY) { - if (!is_interactive_session) { + if (!is_interactive_session()) { // It's OK if we're not able to take control of the terminal. We handle // the fallout from this in a few other places. break; @@ -3229,7 +3229,7 @@ maybe_t reader_data_t::readline(int nchars_or_0) { // This check is required to work around certain issues with fish's approach to // terminal control when launching interactive processes while in non-interactive // mode. See #4178 for one such example. - if (err != ENOTTY || is_interactive_session) { + if (err != ENOTTY || is_interactive_session()) { wperror(L"tcsetattr"); } } @@ -3333,7 +3333,7 @@ maybe_t reader_data_t::readline(int nchars_or_0) { if (!reader_exit_forced()) { // The order of the two conditions below is important. Try to restore the mode // in all cases, but only complain if interactive. - if (tcsetattr(0, TCSANOW, &old_modes) == -1 && is_interactive_session) { + if (tcsetattr(0, TCSANOW, &old_modes) == -1 && is_interactive_session()) { if (errno == EIO) redirect_tty_output(); wperror(L"tcsetattr"); // return to previous mode }