I'll see if I can fit this one in in the next few days. Feel free to remind me :-)
martijn@ On Sun, 2021-08-29 at 02:54 -0600, Theo de Raadt wrote: > This does look better. > > I appreciate that you are fixing this underlying problem first, before > overlaying your timer diff. > > Is this working for the vi crowd? > > > trondd <tro...@kagu-tsuchi.com> wrote: > > > "Theo de Raadt" <dera...@openbsd.org> wrote: > > > > > +h_alrm(int signo) > > > +{ > > > + GLOBAL_CLP; > > > + > > > + F_SET(clp, CL_SIGALRM); > > > > > > F_SET is |=, which is not atomic. > > > > > > This is unsafe. Safe signal handlers need to make single stores to > > > atomic-sized variables, which tend to be int-sized, easier to declare > > > as sig_atomic_t. Most often these are global scope with the following > > > type: > > > > > > volatile sig_atomic_t variable > > > > > > I can see you copied an existing practice. Sadly all the other > > > signal handlers are also broken in the same way. > > > > > > The flag bits should be replaced with seperate sig_atomic_t variables. > > > > Ok. Took a swing at converting the signal handling to sig_atomic_t flags. > > No additional changes added other than removing a redundant check of the > > flags in cl_read.c. Seemed to be a pretty straight-forward conversion and > > I haven't found any change in behavior. > > > > Tim. > > > > > > Index: cl/cl.h > > =================================================================== > > RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v > > retrieving revision 1.10 > > diff -u -p -r1.10 cl.h > > --- cl/cl.h 27 May 2016 09:18:11 -0000 1.10 > > +++ cl/cl.h 24 Aug 2021 23:34:27 -0000 > > @@ -11,6 +11,11 @@ > > * @(#)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; > > + > > typedef struct _cl_private { > > CHAR_T ibuf[256]; /* Input keys. */ > > > > @@ -45,11 +50,7 @@ typedef struct _cl_private { > > #define CL_RENAME_OK 0x0004 /* User wants the windows renamed. */ > > #define CL_SCR_EX_INIT 0x0008 /* Ex screen initialized. */ > > #define CL_SCR_VI_INIT 0x0010 /* Vi screen initialized. */ > > -#define CL_SIGHUP 0x0020 /* SIGHUP arrived. */ > > -#define CL_SIGINT 0x0040 /* SIGINT arrived. */ > > -#define CL_SIGTERM 0x0080 /* SIGTERM arrived. */ > > -#define CL_SIGWINCH 0x0100 /* SIGWINCH arrived. */ > > -#define CL_STDIN_TTY 0x0200 /* Talking to a terminal. */ > > +#define CL_STDIN_TTY 0x0020 /* Talking to a terminal. */ > > u_int32_t flags; > > } CL_PRIVATE; > > > > Index: cl/cl_funcs.c > > =================================================================== > > RCS file: /cvs/src/usr.bin/vi/cl/cl_funcs.c,v > > retrieving revision 1.20 > > diff -u -p -r1.20 cl_funcs.c > > --- cl/cl_funcs.c 27 May 2016 09:18:11 -0000 1.20 > > +++ cl/cl_funcs.c 24 Aug 2021 23:34:27 -0000 > > @@ -601,7 +601,7 @@ cl_suspend(SCR *sp, int *allowedp) > > if (cl_ssize(sp, 1, NULL, NULL, &changed)) > > return (1); > > if (changed) > > - F_SET(CLP(sp), CL_SIGWINCH); > > + cl_sigwinch = 1; > > > > return (0); > > } > > Index: cl/cl_main.c > > =================================================================== > > RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v > > retrieving revision 1.33 > > diff -u -p -r1.33 cl_main.c > > --- cl/cl_main.c 5 May 2016 20:36:41 -0000 1.33 > > +++ cl/cl_main.c 24 Aug 2021 23:34:27 -0000 > > @@ -33,6 +33,11 @@ > > > > 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; > > + > > static void cl_func_std(GS *); > > static CL_PRIVATE *cl_init(GS *); > > static GS *gs_init(void); > > @@ -217,16 +222,14 @@ h_hup(int signo) > > { > > GLOBAL_CLP; > > > > - F_SET(clp, CL_SIGHUP); > > + cl_sighup = 1; > > clp->killersig = SIGHUP; > > } > > > > static void > > h_int(int signo) > > { > > - GLOBAL_CLP; > > - > > - F_SET(clp, CL_SIGINT); > > + cl_sigint = 1; > > } > > > > static void > > @@ -234,16 +237,14 @@ h_term(int signo) > > { > > GLOBAL_CLP; > > > > - F_SET(clp, CL_SIGTERM); > > + cl_sigterm = 1; > > clp->killersig = SIGTERM; > > } > > > > static void > > h_winch(int signo) > > { > > - GLOBAL_CLP; > > - > > - F_SET(clp, CL_SIGWINCH); > > + cl_sigwinch = 1; > > } > > #undef GLOBAL_CLP > > > > @@ -259,6 +260,11 @@ sig_init(GS *gp, SCR *sp) > > CL_PRIVATE *clp; > > > > 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) || > > Index: cl/cl_read.c > > =================================================================== > > RCS file: /cvs/src/usr.bin/vi/cl/cl_read.c,v > > retrieving revision 1.21 > > diff -u -p -r1.21 cl_read.c > > --- cl/cl_read.c 27 May 2016 09:18:11 -0000 1.21 > > +++ cl/cl_read.c 24 Aug 2021 23:34:27 -0000 > > @@ -54,34 +54,32 @@ cl_event(SCR *sp, EVENT *evp, u_int32_t > > * so that we just keep returning them until the editor dies. > > */ > > clp = CLP(sp); > > -retest: if (LF_ISSET(EC_INTERRUPT) || F_ISSET(clp, CL_SIGINT)) { > > - if (F_ISSET(clp, CL_SIGINT)) { > > - F_CLR(clp, CL_SIGINT); > > +retest: if (LF_ISSET(EC_INTERRUPT) || cl_sigint) { > > + if (cl_sigint) { > > + cl_sigint = 0; > > evp->e_event = E_INTERRUPT; > > } else > > evp->e_event = E_TIMEOUT; > > return (0); > > } > > - if (F_ISSET(clp, CL_SIGHUP | CL_SIGTERM | CL_SIGWINCH)) { > > - if (F_ISSET(clp, CL_SIGHUP)) { > > - evp->e_event = E_SIGHUP; > > - return (0); > > - } > > - if (F_ISSET(clp, CL_SIGTERM)) { > > - evp->e_event = E_SIGTERM; > > + if (cl_sighup) { > > + evp->e_event = E_SIGHUP; > > + return (0); > > + } > > + if (cl_sigterm) { > > + evp->e_event = E_SIGTERM; > > + return (0); > > + } > > + if (cl_sigwinch) { > > + cl_sigwinch = 0; > > + if (cl_ssize(sp, 1, &lines, &columns, &changed)) > > + return (1); > > + if (changed) { > > + (void)cl_resize(sp, lines, columns); > > + evp->e_event = E_WRESIZE; > > return (0); > > } > > - if (F_ISSET(clp, CL_SIGWINCH)) { > > - F_CLR(clp, CL_SIGWINCH); > > - if (cl_ssize(sp, 1, &lines, &columns, &changed)) > > - return (1); > > - if (changed) { > > - (void)cl_resize(sp, lines, columns); > > - evp->e_event = E_WRESIZE; > > - return (0); > > - } > > - /* No real change, ignore the signal. */ > > - } > > + /* No real change, ignore the signal. */ > > } > > > > /* Set timer. */ > > >