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;
