From 52a720ec331dc79fd16280fc9efd51309666c3ba Mon Sep 17 00:00:00 2001 From: Andrew Wood Date: Thu, 3 Oct 2024 22:08:11 +0100 Subject: [PATCH] Improve the readability of the code that handles SIGTTOU. --- docs/NEWS.md | 1 + src/include/pv-internal.h | 4 ++-- src/pv/cursor.c | 10 +++++----- src/pv/signal.c | 15 ++++++++++----- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/docs/NEWS.md b/docs/NEWS.md index 1e902cb..6699865 100644 --- a/docs/NEWS.md +++ b/docs/NEWS.md @@ -3,6 +3,7 @@ * fix: complete set of German translations supplied by Hartmut Goebel ([#98](https://codeberg.org/a-j-wood/pv/pulls/98)) * cleanup: removed TODO.md, since it's just an outdated copy of the issue tracker * cleanup: re-ordered structure members to reduce padding + * cleanup: improved readability of SIGTTOU handling code ### 1.8.14 - 7 September 2024 diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index 7e3faad..ec5cc63 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -160,8 +160,7 @@ struct pvstate_s { volatile sig_atomic_t rxusr2; /* whether SIGUSR2 was received */ volatile pid_t sender; /* PID of sending process for SIGUSR2 */ #endif - int old_stderr; /* see pv_sig_ttou() */ - bool pv_tty_tostop_added; /* whether we had to set TOSTOP on the terminal */ + int old_stderr; /* see pv_sig_ttou() */ } signal; /******************* @@ -171,6 +170,7 @@ struct pvstate_s { volatile sig_atomic_t reparse_display; /* whether to re-check format string */ volatile sig_atomic_t terminal_resized; /* whether we need to get term size again */ volatile sig_atomic_t trigger_exit; /* whether we need to abort right now */ + volatile sig_atomic_t clear_tty_tostop_on_exit; /* whether to clear tty TOSTOP on exit */ } flag; /***************** diff --git a/src/pv/cursor.c b/src/pv/cursor.c index a14c35e..ea87916 100644 --- a/src/pv/cursor.c +++ b/src/pv/cursor.c @@ -437,8 +437,8 @@ void pv_crs_init(pvstate_t state) * If we have already set the terminal TOSTOP attribute, set the * flag in shared memory to let the other instances know. */ - if ((!state->cursor.noipc) && state->signal.pv_tty_tostop_added && (NULL != state->cursor.shared)) { - debug("%s", "propagating local pv_tty_tostop_added true value to shared flag"); + if ((!state->cursor.noipc) && (1 == state->flag.clear_tty_tostop_on_exit) && (NULL != state->cursor.shared)) { + debug("%s", "propagating local clear_tty_tostop_on_exit true value to shared tty_tostop_added flag"); state->cursor.shared->tty_tostop_added = true; } @@ -674,9 +674,9 @@ void pv_crs_fini(pvstate_t state) * it. */ if ((!state->cursor.noipc) && (NULL != state->cursor.shared) && state->cursor.shared->tty_tostop_added) { - if (!state->signal.pv_tty_tostop_added) { - debug("%s", "propagating shared tty_tostop_added true value to local flag"); - state->signal.pv_tty_tostop_added = true; + if (0 == state->flag.clear_tty_tostop_on_exit) { + debug("%s", "propagating shared tty_tostop_added true value to local clear_tty_tostop_on_exit flag"); + state->flag.clear_tty_tostop_on_exit = 1; } } diff --git a/src/pv/signal.c b/src/pv/signal.c index 9640f66..ec1592c 100644 --- a/src/pv/signal.c +++ b/src/pv/signal.c @@ -27,8 +27,13 @@ void pv_crs_needreinit(pvstate_t); /* * Ensure that terminal attribute TOSTOP is set. If we have to set it, - * record that fact by setting the state boolean "pv_tty_tostop_added" to - * true, so that in pv_sig_fini() we can turn it back off again. + * record that fact by setting "clear_tty_tostop_on_exit" to 1, so that in + * pv_sig_fini() we can turn it back off again. + * + * In "-c" mode with IPC, then if we have to set TOSTOP, we also tell the + * other PV instances about it via the shared "tty_tostop_added" flag, so + * those instances can set their own on-exit flag, meaning that if any of + * the PV instances set it, the last one to exit will clear it. */ static void pv_sig_ensure_tty_tostop() { @@ -45,7 +50,7 @@ static void pv_sig_ensure_tty_tostop() if (0 == (terminal_attributes.c_lflag & TOSTOP)) { terminal_attributes.c_lflag |= TOSTOP; if (0 == tcsetattr(STDERR_FILENO, TCSANOW, &terminal_attributes)) { - pv_sig_state->signal.pv_tty_tostop_added = true; + pv_sig_state->flag.clear_tty_tostop_on_exit = 1; debug("%s", "set terminal TOSTOP attribute"); } else { debug("%s: %s", "failed to set terminal TOSTOP attribute", strerror(errno)); @@ -416,7 +421,7 @@ void pv_sig_fini( /*@unused@ */ __attribute__((unused)) pvstate_t state) #endif (void) sigaction(SIGALRM, &(pv_sig_state->signal.old_sigalrm), NULL); - need_to_clear_tostop = pv_sig_state->signal.pv_tty_tostop_added; + need_to_clear_tostop = 1 == pv_sig_state->flag.clear_tty_tostop_on_exit ? true : false; if (pv_sig_state->control.cursor) { #ifdef HAVE_IPC @@ -457,7 +462,7 @@ void pv_sig_fini( /*@unused@ */ __attribute__((unused)) pvstate_t state) } } - pv_sig_state->signal.pv_tty_tostop_added = false; + pv_sig_state->flag.clear_tty_tostop_on_exit = 0; } }