diff --git a/src/common.cpp b/src/common.cpp index c51e0ccdf..ce6e95238 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -1935,12 +1936,12 @@ long convert_digit(wchar_t d, int base) { return res; } -// Test if the specified character is in a range that fish uses interally to store special tokens. -// -// NOTE: This is used when tokenizing the input. It is also used when reading input, before -// tokenization, to replace such chars with REPLACEMENT_WCHAR if they're not part of a quoted -// string. We don't want external input to be able to feed reserved characters into our lexer/parser -// or code evaluator. +/// Test if the specified character is in a range that fish uses interally to store special tokens. +/// +/// NOTE: This is used when tokenizing the input. It is also used when reading input, before +/// tokenization, to replace such chars with REPLACEMENT_WCHAR if they're not part of a quoted +/// string. We don't want external input to be able to feed reserved characters into our +/// lexer/parser or code evaluator. // // TODO: Actually implement the replacement as documented above. bool fish_reserved_codepoint(wchar_t c) { @@ -1948,3 +1949,14 @@ bool fish_reserved_codepoint(wchar_t c) { (c >= ENCODE_DIRECT_BASE && c < ENCODE_DIRECT_END) || (c >= INPUT_COMMON_BASE && c < INPUT_COMMON_END); } + +/// Reopen stdout and/or stderr on /dev/null. This is invoked when we find that our tty has become +/// invalid. +void redirect_tty_output() { + struct termios t; + int fd = open("/dev/null", O_WRONLY); + if (tcgetattr(STDIN_FILENO, &t) == -1) dup2(fd, STDIN_FILENO); + if (tcgetattr(STDOUT_FILENO, &t) == -1) dup2(fd, STDOUT_FILENO); + if (tcgetattr(STDERR_FILENO, &t) == -1) dup2(fd, STDERR_FILENO); + close(fd); +} diff --git a/src/common.h b/src/common.h index 6d324aca9..500f209ee 100644 --- a/src/common.h +++ b/src/common.h @@ -843,3 +843,5 @@ using std::wcsncasecmp; #endif #endif + +void redirect_tty_output(void); diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 6408c361b..712626fab 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -1288,7 +1288,7 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t j->tmodes = tmodes; job_set_flag(j, JOB_CONTROL, (job_control_mode == JOB_CONTROL_ALL) || - ((job_control_mode == JOB_CONTROL_INTERACTIVE) && (shell_is_interactive()))); + ((job_control_mode == JOB_CONTROL_INTERACTIVE) && shell_is_interactive())); job_set_flag(j, JOB_FOREGROUND, !tree.job_should_be_backgrounded(job_node)); diff --git a/src/postfork.cpp b/src/postfork.cpp index 792ee2917..366c5ced8 100644 --- a/src/postfork.cpp +++ b/src/postfork.cpp @@ -58,8 +58,15 @@ static void debug_safe_int(int level, const char *format, int val) { debug_safe(level, format, buff); } -int set_child_group(job_t *j, process_t *p, int print_errors) { - int res = 0; +/// This function should be called by both the parent process and the child right after fork() has +/// been called. If job control is enabled, the child is put in the jobs group, and if the child is +/// also in the foreground, it is also given control of the terminal. When called in the parent +/// process, this function may fail, since the child might have already finished and called exit. +/// The parent process may safely ignore the exit status of this call. +/// +/// Returns true on sucess, false on failiure. +bool set_child_group(job_t *j, process_t *p, int print_errors) { + bool retval = true; if (job_get_flag(j, JOB_CONTROL)) { if (!j->pgid) { @@ -89,27 +96,31 @@ int set_child_group(job_t *j, process_t *p, int print_errors) { pid_buff, argv0, job_id_buff, command, getpgid_buff, job_pgid_buff); safe_perror("setpgid"); - res = -1; + retval = false; } } } else { j->pgid = getpid(); } - if (job_get_flag(j, JOB_TERMINAL) && job_get_flag(j, JOB_FOREGROUND)) { - int result = tcsetpgrp(0, j->pgid); - if (result == -1 && print_errors) { - char job_id_buff[64]; - char command_buff[64]; - format_long_safe(job_id_buff, j->job_id); - narrow_string_safe(command_buff, j->command_wcstr()); - debug_safe(1, "Could not send job %s ('%s') to foreground", job_id_buff, command_buff); - safe_perror("tcsetpgrp"); - res = -1; + if (job_get_flag(j, JOB_TERMINAL) && job_get_flag(j, JOB_FOREGROUND)) { //!OCLINT(early exit) + int result = tcsetpgrp(STDIN_FILENO, j->pgid); // to avoid "collapsible if statements" warn + if (result == -1) { + if (errno == ENOTTY) redirect_tty_output(); + if (print_errors) { + char job_id_buff[64]; + char command_buff[64]; + format_long_safe(job_id_buff, j->job_id); + narrow_string_safe(command_buff, j->command_wcstr()); + debug_safe(1, "Could not send job %s ('%s') to foreground", job_id_buff, + command_buff); + safe_perror("tcsetpgrp"); + retval = false; + } } } - return res; + return retval; } /// Set up a childs io redirections. Should only be called by setup_child_process(). Does the @@ -217,7 +228,7 @@ int setup_child_process(job_t *j, process_t *p, const io_chain_t &io_chain) { bool ok = true; if (p) { - ok = (0 == set_child_group(j, p, 1)); + ok = set_child_group(j, p, 1); } if (ok) { diff --git a/src/postfork.h b/src/postfork.h index c12053200..df0a74cba 100644 --- a/src/postfork.h +++ b/src/postfork.h @@ -17,14 +17,7 @@ class io_chain_t; class job_t; class process_t; -/// This function should be called by both the parent process and the child right after fork() has -/// been called. If job control is enabled, the child is put in the jobs group, and if the child is -/// also in the foreground, it is also given control of the terminal. When called in the parent -/// process, this function may fail, since the child might have already finished and called exit. -/// The parent process may safely ignore the exit status of this call. -/// -/// Returns 0 on sucess, -1 on failiure. -int set_child_group(job_t *j, process_t *p, int print_errors); +bool set_child_group(job_t *j, process_t *p, int print_errors); /// Initialize a new child process. This should be called right away after forking in the child /// process. If job control is enabled for this job, the process is put in the process group of the diff --git a/src/proc.cpp b/src/proc.cpp index 42850f6b7..275ed6236 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -792,13 +792,15 @@ static void read_try(job_t *j) { /// \param cont If this variable is set, we are giving back control to a job that has previously /// been stopped. In that case, we need to set the terminal attributes to those saved in the job. static bool terminal_give_to_job(job_t *j, int cont) { - if (tcsetpgrp(0, j->pgid)) { + if (tcsetpgrp(STDIN_FILENO, j->pgid) == -1) { + if (errno == ENOTTY) redirect_tty_output(); debug(1, _(L"Could not send job %d ('%ls') to foreground"), j->job_id, j->command_wcstr()); wperror(L"tcsetpgrp"); return false; } - if (cont && tcsetattr(0, TCSADRAIN, &j->tmodes)) { + if (cont && tcsetattr(STDIN_FILENO, TCSADRAIN, &j->tmodes)) { + if (errno == ENOTTY) redirect_tty_output(); debug(1, _(L"Could not send job %d ('%ls') to foreground"), j->job_id, j->command_wcstr()); wperror(L"tcsetattr"); return false; @@ -809,16 +811,16 @@ static bool terminal_give_to_job(job_t *j, int cont) { /// Returns control of the terminal to the shell, and saves the terminal attribute state to the job, /// so that we can restore the terminal ownership to the job at a later time. static int terminal_return_from_job(job_t *j) { - if (tcsetpgrp(0, getpgrp())) { + if (tcsetpgrp(STDIN_FILENO, getpgrp()) == -1) { + if (errno == ENOTTY) redirect_tty_output(); debug(1, _(L"Could not return shell to foreground")); wperror(L"tcsetpgrp"); return 0; } - /* - Save jobs terminal modes. - */ - if (tcgetattr(0, &j->tmodes)) { + // Save jobs terminal modes. + if (tcgetattr(STDIN_FILENO, &j->tmodes)) { + if (errno == ENOTTY) redirect_tty_output(); debug(1, _(L"Could not return shell to foreground")); wperror(L"tcgetattr"); return 0; @@ -830,7 +832,7 @@ static int terminal_return_from_job(job_t *j) { // https://github.com/fish-shell/fish-shell/issues/121 #if 0 // Restore the shell's terminal modes. - if (tcsetattr(0, TCSADRAIN, &shell_modes)) { + if (tcsetattr(STDIN_FILENO, TCSADRAIN, &shell_modes)) { debug(1, _(L"Could not return shell to foreground")); wperror(L"tcsetattr"); return 0; diff --git a/src/reader.cpp b/src/reader.cpp index 58fa2496a..719c3faaf 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -312,7 +312,8 @@ static void term_donate() { set_color(rgb_color_t::normal(), rgb_color_t::normal()); while (1) { - if (tcsetattr(0, TCSANOW, &tty_modes_for_external_cmds)) { + if (tcsetattr(STDIN_FILENO, TCSANOW, &tty_modes_for_external_cmds) == -1) { + if (errno == ENOTTY) redirect_tty_output(); if (errno != EINTR) { debug(1, _(L"Could not set terminal mode for new job")); wperror(L"tcsetattr"); @@ -340,10 +341,11 @@ static void update_buff_pos(editable_line_t *el, size_t buff_pos) { /// Grab control of terminal. static void term_steal() { while (1) { - if (tcsetattr(0, TCSANOW, &shell_modes)) { + if (tcsetattr(STDIN_FILENO, TCSANOW, &shell_modes) == -1) { + if (errno == ENOTTY) redirect_tty_output(); if (errno != EINTR) { debug(1, _(L"Could not set terminal mode for shell")); - wperror(L"tcsetattr"); + perror("tcsetattr"); break; } } else @@ -808,7 +810,9 @@ void restore_term_mode() { // Restore the term mode if we own the terminal. It's important we do this before // restore_foreground_process_group, otherwise we won't think we own the terminal. if (getpid() == tcgetpgrp(STDIN_FILENO)) { - tcsetattr(STDIN_FILENO, TCSANOW, &terminal_mode_on_startup); + if (tcsetattr(STDIN_FILENO, TCSANOW, &terminal_mode_on_startup) == -1 && errno == ENOTTY) { + redirect_tty_output(); + } } } @@ -1638,22 +1642,24 @@ static void reader_interactive_init() { // Put ourselves in our own process group. shell_pgid = getpid(); if (getpgrp() != shell_pgid && setpgid(shell_pgid, shell_pgid) < 0) { - debug(1, _(L"Couldn't put the shell in its own process group")); + debug(0, _(L"Couldn't put the shell in its own process group")); wperror(L"setpgid"); exit_without_destructors(1); } // Grab control of the terminal. - if (tcsetpgrp(STDIN_FILENO, shell_pgid)) { - debug(1, _(L"Couldn't grab control of terminal")); + if (tcsetpgrp(STDIN_FILENO, shell_pgid) == -1) { + if (errno == ENOTTY) redirect_tty_output(); + debug(0, _(L"Couldn't grab control of terminal")); wperror(L"tcsetpgrp"); exit_without_destructors(1); } common_handle_winch(0); - if (tcsetattr(0, TCSANOW, &shell_modes)) // set the new modes - { + // Set the new modes. + if (tcsetattr(0, TCSANOW, &shell_modes) == -1) { + if (errno == ENOTTY) redirect_tty_output(); wperror(L"tcsetattr"); } @@ -2401,9 +2407,10 @@ const wchar_t *reader_readline(int nchars) { reader_repaint(); // Get the current terminal modes. These will be restored when the function returns. - tcgetattr(0, &old_modes); + tcgetattr(STDIN_FILENO, &old_modes); // Set the new modes. - if (tcsetattr(0, TCSANOW, &shell_modes)) { + if (tcsetattr(0, TCSANOW, &shell_modes) == -1) { + if (errno == ENOTTY) redirect_tty_output(); wperror(L"tcsetattr"); } @@ -3269,7 +3276,8 @@ const wchar_t *reader_readline(int nchars) { } if (!reader_exit_forced()) { - if (tcsetattr(0, TCSANOW, &old_modes)) { + if (tcsetattr(0, TCSANOW, &old_modes) == -1) { + if (errno == ENOTTY) redirect_tty_output(); wperror(L"tcsetattr"); // return to previous mode } set_color(rgb_color_t::reset(), rgb_color_t::reset());