pomo

commit 3678854ad1944d936cfe124134d80debd1ec6e94
parent 50ed6952f2ceb18a49e9b24697172dc5c0d7b437
Author: Simon Lieb <simon@e5150.fr>
Date:   Tue, 25 Oct 2016 20:23:05 +0200

Fix separate USR1 handling & time display logic

Make remaining_time() more atomic, in order to avoid receiving a signal
during execution of this function.

Receiving a signal during a signal handler is an undefined behavior.

Diffstat:
M
spt.c
 | 
36
++++++++++++++++++++++++------------

1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/spt.c b/spt.c
@@ -24,14 +24,15 @@ typedef struct {
 
 #include "config.h"
 
-static int i, timecount, suspend;
+volatile static sig_atomic_t display, suspend;
 
 /* function declarations */
 static void die(const char *errstr, ...);
 static void spawn(char *cmd, char *cmt);
 static void notify_send(char *cmt);
-static void remaining_time(int sigint);
-static void toggle(int sigint);
+static void display_time(int timecount);
+static void toggle_display(int sigint);
+static void toggle_suspend(int sigint);
 static void usage(void);
 
 /* functions implementations */
@@ -78,21 +79,27 @@ notify_send(char *cmt)
 }
 
 void
-remaining_time(int sigint)
+display_time(int timecount)
 {
 	char buf[17];
 
-	// FIXME: signal handlers should only do very few things, like
-	// setting volatile sig_atomic_t
 	snprintf(buf, 17, "Remaining: %02d:%02d\n",
-		 (timers[i].tmr - timecount) / 60,
-		 (timers[i].tmr - timecount) % 60);
+		 timecount / 60,
+		 timecount % 60);
 
 	notify_send(buf);
+	display = 0;
 }
 
 void
-toggle(int sigint) {
+toggle_display(int sigint)
+{
+	display = 1;
+}
+
+void
+toggle_suspend(int sigint)
+{
 	suspend ^= 1;
 }
 
@@ -107,6 +114,7 @@ main(int argc, char *argv[])
 {
 	struct sigaction sa;
 	sigset_t emptymask;
+	int i, timecount;
 
 	ARGBEGIN {
 		case 'e':
@@ -124,7 +132,7 @@ main(int argc, char *argv[])
 	} ARGEND;
 
 	/* add SIGUSR1 handler: remaining_time */
-	sa.sa_handler = remaining_time;
+	sa.sa_handler = toggle_display;
 	sigemptyset(&sa.sa_mask);
 	sa.sa_flags = 0;
 
@@ -132,7 +140,7 @@ main(int argc, char *argv[])
 		die("cannot associate SIGUSR1 to handler\n");
 
 	/* add SIGUSR2 handler: toggle */
-	sa.sa_handler = toggle;
+	sa.sa_handler = toggle_suspend;
 	sigemptyset(&sa.sa_mask);
 	sa.sa_flags = 0;
 
@@ -142,13 +150,17 @@ main(int argc, char *argv[])
 	for (i = 0; ; i = (i + 1) % LEN(timers)) {
 		notify_send(timers[i].cmt);
 		timecount = 0;
-		while (timecount < timers[i].tmr)
+		while (timecount < timers[i].tmr) {
+			if (display)
+				display_time(timecount);
+
 			if (suspend)
 				sigsuspend(&emptymask);
 			else {
 				sleep(1);
 				timecount++;
 			}
+		}
 	}
 
 	return 0;