Hi, Ingo Schwarze wrote on Wed, Sep 01, 2021 at 04:38:51PM +0200:
> Note that the h_hup() and h_term() signal handlers are still unsafe > after this commit because they also set the "killersig" (how fitting!) > field in a global struct. I like it when fixing two bugs only amounts to minus: minus 14 LOC, 1 function, 1 global variable, 2 automatic variables, 1 struct member, 9 pointer dereferences, 1 #define, and minus 1 #undef. The argument that only one single GS exists applies unchanged. OK? Ingo Index: cl/cl.h =================================================================== RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v retrieving revision 1.11 diff -u -p -r1.11 cl.h --- cl/cl.h 1 Sep 2021 14:28:15 -0000 1.11 +++ cl/cl.h 1 Sep 2021 17:15:34 -0000 @@ -11,7 +11,6 @@ * @(#)cl.h 10.19 (Berkeley) 9/24/96 */ -extern volatile sig_atomic_t cl_sighup; extern volatile sig_atomic_t cl_sigint; extern volatile sig_atomic_t cl_sigterm; extern volatile sig_atomic_t cl_sigwinch; @@ -31,7 +30,6 @@ typedef struct _cl_private { char *rmso, *smso; /* Inverse video terminal strings. */ char *smcup, *rmcup; /* Terminal start/stop strings. */ - int killersig; /* Killer signal. */ #define INDX_HUP 0 #define INDX_INT 1 #define INDX_TERM 2 Index: cl/cl_funcs.c =================================================================== RCS file: /cvs/src/usr.bin/vi/cl/cl_funcs.c,v retrieving revision 1.21 diff -u -p -r1.21 cl_funcs.c --- cl/cl_funcs.c 1 Sep 2021 14:28:15 -0000 1.21 +++ cl/cl_funcs.c 1 Sep 2021 17:15:34 -0000 @@ -437,7 +437,7 @@ cl_refresh(SCR *sp, int repaint) * If we received a killer signal, we're done, there's no point * in refreshing the screen. */ - if (clp->killersig) + if (cl_sigterm) return (0); /* @@ -582,7 +582,7 @@ cl_suspend(SCR *sp, int *allowedp) * unchanged. In addition, the terminal has already been reset * correctly, so leave it alone. */ - if (clp->killersig) { + if (cl_sigterm) { F_CLR(clp, CL_SCR_EX_INIT | CL_SCR_VI_INIT); return (0); } Index: cl/cl_main.c =================================================================== RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v retrieving revision 1.34 diff -u -p -r1.34 cl_main.c --- cl/cl_main.c 1 Sep 2021 14:28:15 -0000 1.34 +++ cl/cl_main.c 1 Sep 2021 17:15:34 -0000 @@ -33,7 +33,6 @@ GS *__global_list; /* GLOBAL: List of screens. */ -volatile sig_atomic_t cl_sighup; volatile sig_atomic_t cl_sigint; volatile sig_atomic_t cl_sigterm; volatile sig_atomic_t cl_sigwinch; @@ -120,9 +119,9 @@ main(int argc, char *argv[]) } /* If a killer signal arrived, pretend we just got it. */ - if (clp->killersig) { - (void)signal(clp->killersig, SIG_DFL); - (void)kill(getpid(), clp->killersig); + if (cl_sigterm) { + (void)signal(cl_sigterm, SIG_DFL); + (void)kill(getpid(), cl_sigterm); /* NOTREACHED */ } @@ -215,17 +214,6 @@ term_init(char *ttype) } } -#define GLOBAL_CLP \ - CL_PRIVATE *clp = GCLP(__global_list); -static void -h_hup(int signo) -{ - GLOBAL_CLP; - - cl_sighup = 1; - clp->killersig = SIGHUP; -} - static void h_int(int signo) { @@ -235,10 +223,7 @@ h_int(int signo) static void h_term(int signo) { - GLOBAL_CLP; - - cl_sigterm = 1; - clp->killersig = SIGTERM; + cl_sigterm = signo; } static void @@ -246,7 +231,6 @@ h_winch(int signo) { cl_sigwinch = 1; } -#undef GLOBAL_CLP /* * sig_init -- @@ -261,20 +245,19 @@ sig_init(GS *gp, SCR *sp) clp = GCLP(gp); - cl_sighup = 0; cl_sigint = 0; cl_sigterm = 0; cl_sigwinch = 0; if (sp == NULL) { - if (setsig(SIGHUP, &clp->oact[INDX_HUP], h_hup) || + if (setsig(SIGHUP, &clp->oact[INDX_HUP], h_term) || setsig(SIGINT, &clp->oact[INDX_INT], h_int) || setsig(SIGTERM, &clp->oact[INDX_TERM], h_term) || setsig(SIGWINCH, &clp->oact[INDX_WINCH], h_winch) ) err(1, NULL); } else - if (setsig(SIGHUP, NULL, h_hup) || + if (setsig(SIGHUP, NULL, h_term) || setsig(SIGINT, NULL, h_int) || setsig(SIGTERM, NULL, h_term) || setsig(SIGWINCH, NULL, h_winch) Index: cl/cl_read.c =================================================================== RCS file: /cvs/src/usr.bin/vi/cl/cl_read.c,v retrieving revision 1.22 diff -u -p -r1.22 cl_read.c --- cl/cl_read.c 1 Sep 2021 14:28:15 -0000 1.22 +++ cl/cl_read.c 1 Sep 2021 17:15:34 -0000 @@ -62,13 +62,15 @@ retest: if (LF_ISSET(EC_INTERRUPT) || cl evp->e_event = E_TIMEOUT; return (0); } - if (cl_sighup) { + switch (cl_sigterm) { + case SIGHUP: evp->e_event = E_SIGHUP; return (0); - } - if (cl_sigterm) { + case SIGTERM: evp->e_event = E_SIGTERM; return (0); + default: + break; } if (cl_sigwinch) { cl_sigwinch = 0;