From 7d399b7223bc194c0b61aab8c6bd252fd0d43ded Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
Date: Tue, 24 May 2022 12:58:01 +0800
Subject: [PATCH v5 2/4] Fix incorrect signal handling

There are problems in signal handlers. Signal handlers must not call any
non-async-signal-safe functions, and they must save-restore errno if
errno is modified inside signal handlers. This patch fixes these
problems by deferring real tasks to main loop. This patch also
introduces bogl_signal(), which wraps around sigaction() thus signal
handlers can be installed in a portable way. Since signal related
problems are fixed, the previously temporarily disabled font unmapping
is now re-enabled.
---
 bogl.c  | 50 +++++++++++++++++++++++++-------------------------
 bogl.h  |  2 ++
 boglP.h |  2 ++
 bterm.c | 36 ++++++++++++++++++++++++++----------
 4 files changed, 55 insertions(+), 35 deletions(-)

diff --git a/bogl.c b/bogl.c
index 6b9996b..86bc1e0 100644
--- a/bogl.c
+++ b/bogl.c
@@ -65,6 +65,7 @@
 /* Global variables. */
 int bogl_xres, bogl_yres, bogl_bpp;	/* bogl.h */
 int bogl_refresh;
+volatile int vt_switch_pending = 0;
 volatile char *bogl_frame;		/* boglP.h */
 int bogl_drawing;
 int bogl_line_len;
@@ -120,7 +121,7 @@ static void draw_disable (void);
 static void kbd_init (void);
 static void kbd_done (void);
 
-static void vt_switch (int);
+static void sigusr2 (int);
 
 static struct fb_fix_screeninfo fb_fix;
 /* Initialize BOGL. */
@@ -181,7 +182,7 @@ bogl_init (void)
   mode.relsig = SIGUSR2;
   mode.acqsig = SIGUSR2;
 
-  signal (SIGUSR2, vt_switch);
+  bogl_signal (SIGUSR2, sigusr2);
 
   if (-1 == ioctl (tty, VT_SETMODE, &mode))
     return bogl_fail ("can't set VT mode: %s", strerror (errno));
@@ -295,7 +296,7 @@ bogl_done (void)
 
   munmap ((void *) bogl_frame, fb_fix.smem_len);
   
-  signal (SIGUSR2, SIG_DFL);
+  bogl_signal (SIGUSR2, SIG_DFL);
 
   ioctl (tty, KDSETMODE, KD_TEXT);
 
@@ -583,32 +584,18 @@ draw_disable (void)
 /* Signal handler called whenever the kernel wants to switch to or
    from our tty. */
 static void
-vt_switch (int sig unused)
+sigusr2 (int sig unused)
 {
-  signal (SIGUSR2, vt_switch);
+  vt_switch_pending = 1;
+}
+void
+vt_switch (void)
+{
+  vt_switch_pending = 0;
 
-  /* If a BOGL drawing function is in progress then we cannot mode
-     switch right now because the drawing function would continue to
-     scribble on the screen after the switch.  So disable further
-     drawing and schedule an alarm to try again in .1 second. */
   if (bogl_drawing)
     {
-      draw_disable ();
-
-      signal (SIGALRM, vt_switch);
-      
-      {
-	struct itimerval duration;
-	
-	duration.it_interval.tv_sec = 0;
-	duration.it_interval.tv_usec = 0;
-	duration.it_value.tv_sec = 0;
-	duration.it_value.tv_usec = 100000;
-	if (-1 == setitimer (ITIMER_REAL, &duration, NULL))
-	  bogl_fail ("can't set timer: %s", strerror (errno));
-      }
-      
-      return;
+      abort();
     }
       
   if (visible)
@@ -666,3 +653,16 @@ bogl_cloexec(int fd)
 
   return 0;
 }
+
+/* Install signal handler in a portable way (i.e. sigaction wrapper) */
+int
+bogl_signal(int signum, void (*handler) (int))
+{
+  struct sigaction sa;
+
+  sa.sa_handler = handler;
+  sigemptyset (&sa.sa_mask);
+  sa.sa_flags = SA_RESTART;
+
+  return sigaction (signum, &sa, NULL);
+}
diff --git a/bogl.h b/bogl.h
index 3b2cb83..4d99788 100644
--- a/bogl.h
+++ b/bogl.h
@@ -69,6 +69,8 @@ extern int bogl_xres, bogl_yres, bogl_ncols;
 
 /* 1=Must refresh screen due to tty change. */
 extern int bogl_refresh;
+extern volatile int vt_switch_pending;
+extern void vt_switch (void);
 
 /* Generic routines. */
 int bogl_init (void);
diff --git a/boglP.h b/boglP.h
index be99979..9a2b4aa 100644
--- a/boglP.h
+++ b/boglP.h
@@ -27,4 +27,6 @@ int bogl_fail (const char *, ...);
 
 int bogl_cloexec (int fd);
 
+int bogl_signal (int signum, void (*handler) (int));
+
 #endif /* boglP_h */
diff --git a/bterm.c b/bterm.c
index dfae8b9..e86a748 100644
--- a/bterm.c
+++ b/bterm.c
@@ -39,6 +39,7 @@
 #include <unistd.h>
 
 #include "bogl.h"
+#include "boglP.h"
 #include "bogl-bgf.h"
 #include "bogl-term.h"
 
@@ -160,7 +161,6 @@ void sigchld(int sig)
       quit_status = status;
     quit = 1;
   }
-  signal(SIGCHLD, sigchld);
   errno = errsv;
 }
 
@@ -176,7 +176,7 @@ void spawn_shell(int ptyfd, int ttyfd, char * const *command_args)
   child_pid = fork();
   if (child_pid) {
     /* Change ownership and permissions of ttyfd device! */
-    signal(SIGCHLD, sigchld);
+    bogl_signal(SIGCHLD, sigchld);
     return;
   }
 
@@ -212,10 +212,16 @@ void set_window_size(int ttyfd, int x, int y)
 
 static char *font_name;
 static struct bogl_term *term;
+static volatile int reload_font_pending = 0;
 
-void reload_font(int sig)
+void sighup(int sig)
+{
+  reload_font_pending = 1;
+}
+void reload_font(void)
 {
   struct bogl_font *font;
+  reload_font_pending = 0;
 
   font = bogl_mmap_font (font_name);
   if (font == NULL)
@@ -224,14 +230,14 @@ void reload_font(int sig)
       return;
     }
   
-  /* BUG: Unmapping old font in this signal handler may cause crash if
-     drawing is in progress, so disable this temporarily until we fix
-     async-signal-safety problems completely. */
-  //bogl_munmap_font(term->font);
+  bogl_munmap_font(term->font);
 
   term->font = font;
   term->xstep = bogl_font_glyph(term->font, ' ', 0);
   term->ystep = bogl_font_height(term->font);
+
+  /* Request redraw */
+  bogl_refresh = 2;
 }
 
 /*
@@ -243,6 +249,7 @@ int main(int argc, char *argv[])
 {
   struct termios ntio;
   int ret;
+  int errsv;
   char buf[8192];
   struct timeval tv;
   int ptyfd, ttyfd;
@@ -342,8 +349,8 @@ int main(int argc, char *argv[])
   }
   spawn_shell(ptyfd, ttyfd, command_args);
 
-  signal(SIGHUP, reload_font);
-  signal(SIGTERM, sigterm);
+  bogl_signal(SIGHUP, sighup);
+  bogl_signal(SIGTERM, sigterm);
 
   ntio = ttysave;
   ntio.c_lflag &= ~(ECHO|ISIG|ICANON|XCASE);
@@ -363,6 +370,10 @@ int main(int argc, char *argv[])
 
     if (quit)
 	    break;
+    if (reload_font_pending)
+	    reload_font();
+    if (vt_switch_pending)
+	    vt_switch();
     
     if(pending)
     {
@@ -380,9 +391,14 @@ int main(int argc, char *argv[])
     if (ptyfd > max)
       max = ptyfd;
     ret = select(max+1, &fds, NULL, NULL, &tv);
+    errsv = errno;
 
     if (quit)
 	    break;
+    if (reload_font_pending)
+	    reload_font();
+    if (vt_switch_pending)
+	    vt_switch();
 
     if (bogl_refresh) {
       /* Handle VT switching.  */
@@ -397,7 +413,7 @@ int main(int argc, char *argv[])
       bogl_refresh = 0;
       bogl_term_redraw(term);
     }
-    if (ret == 0 || (ret < 0 && errno == EINTR))
+    if (ret == 0 || (ret < 0 && errsv == EINTR))
     {
       if(pending)
       {
-- 
2.30.2