Hi,
the analysis and the patch are correct.
Besides, even though hi() does not contain anything that isn't
signal safe on OpenBSD, it is still nice to have that lengthy
function no longer be a signal handler.
I don't really like the warnx(3) call from the bye() ALRM handler
either, but that's a separate matter.
OK to commit?
Ingo
----- Forwarded message from Jean-Philippe Ouellet -----
From: Jean-Philippe Ouellet <jean-phili...@ouellet.biz>
Date: Wed, 12 Mar 2014 16:08:19 -0400
To: tech@openbsd.org
Subject: lock(1) timeout message deduplication
Hello,
When lock(1) receives SIGINT, SIGQUIT, or SIGTSTP, it calls hi()
twice, once because it's the signal handler, and once after
readpassphrase() errors because the read was interrupted.
Since hi() gets called when readpassphrase() fails anyway, this
patch ignores the signals instead of using hi() as a handler.
=== behavior before ===
$ lock
Key: [foo\n]
Again: [foo\n]
lock: /dev/ttyp0 on netboot.realconnect.net. timeout in 15 minutes
time now is Wed Mar 12 15:46:49 2014
Key: [asdf\n]
Key: [\n]
lock: type in the unlock key. timeout in 14:53 minutes
Key: [^C]
lock: type in the unlock key. timeout in 14:52 minutes
lock: type in the unlock key. timeout in 14:52 minutes
Key: [foo\n]
=== behavior after ===
$ obj/lock
Key: [foo\n]
Again: [foo\n]
lock: /dev/ttyp0 on netboot.realconnect.net. timeout in 15 minutes
time now is Wed Mar 12 15:47:03 2014
Key: [asdf\n]
Key: [\n]
lock: type in the unlock key. timeout in 14:54 minutes
Key: [^C]
lock: type in the unlock key. timeout in 14:53 minutes
Key: [foo\n]
Index: lock.c
===================================================================
RCS file: /cvs/src/usr.bin/lock/lock.c,v
retrieving revision 1.27
diff -u -p -r1.27 lock.c
--- lock.c 22 Aug 2013 04:43:40 -0000 1.27
+++ lock.c 12 Mar 2014 19:59:48 -0000
@@ -62,7 +62,7 @@
#define TIMEOUT 15
void bye(int);
-void hi(int);
+void hi(void);
struct timeval timeout;
struct timeval zerotime;
@@ -162,9 +162,9 @@ main(int argc, char *argv[])
}
/* set signal handlers */
- (void)signal(SIGINT, hi);
- (void)signal(SIGQUIT, hi);
- (void)signal(SIGTSTP, hi);
+ (void)signal(SIGINT, SIG_IGN);
+ (void)signal(SIGQUIT, SIG_IGN);
+ (void)signal(SIGTSTP, SIG_IGN);
(void)signal(SIGALRM, bye);
ntimer.it_interval = zerotime;
@@ -186,7 +186,7 @@ main(int argc, char *argv[])
for (cnt = 0;;) {
if (!readpassphrase("Key: ", s, sizeof(s), RPP_ECHO_OFF) ||
*s == '\0') {
- hi(0);
+ hi();
continue;
}
if (usemine) {
@@ -219,9 +219,8 @@ main(int argc, char *argv[])
exit(0);
}
-/*ARGSUSED*/
void
-hi(int signo)
+hi(void)
{
char buf[1024], buf2[1024];
time_t left;
----- End forwarded message -----
