bin/107171: systat(1) doesn't die when it's xterm is killed while it's running

[Nate Eldredge]

The following reply was made to PR bin/107171; it has been noted by GNATS.

From: Nate Eldredge <nge at cs.hmc.edu>
To: bug-followup at FreeBSD.org, josh at tcbug.org
Cc:  
Subject: Re: bin/107171: systat(1) doesn't die when it's xterm is killed
 while it's running
Date: Thu, 8 Mar 2007 02:06:35 -0800 (PST)

 The bug is in /usr/src/usr.bin/systat/keyboard.c, where an error return by 
 getch() is ignored.  When you close an xterm, it sends SIGHUP to all 
 processes running on that terminal, and all further terminal I/O by those 
 processes fails with ENXIO.  But a process running as a different user 
 (e.g. root) can't receive the SIGHUP, and systat ignores the ENXIO and 
 retries endlessly.
 
 I presume the retry is to cope with the case where getch() was interrupted 
 by a signal.  So probably it should only retry if errno = EINTR.  This is 
 essentially what top does.  Also, ferror(stdin) is a bogus test; getch() 
 does not go through stdio and so ferror(stdin) will never be set, and the 
 clearerr() is bogus too.
 
 EINTR is probably moot for FreeBSD anyway since it appears our ncurses 
 handles that transparently, but it doesn't hurt to check.
 
 Here is a patch which fixes the problem.  It still assumes the constant 
 ERR returned by getch() is -1 which is not great style, but I'll touch as 
 little as possible here.
 
 --- /usr/src/usr.bin/systat/keyboard.c.old	Thu Mar  8 02:00:35 2007
 +++ /usr/src/usr.bin/systat/keyboard.c	Thu Mar  8 02:00:40 2007
 @@ -42,6 +42,7 @@
   #include <ctype.h>
   #include <signal.h>
   #include <termios.h>
 +#include <errno.h>
 
   #include "systat.h"
   #include "extern.h"
 @@ -58,9 +59,11 @@
                   do {
                           refresh();
                           ch = getch() & 0177;
 -                        if (ch == 0177 && ferror(stdin)) {
 -                                clearerr(stdin);
 -                                continue;
 +                        if (ch == 0177) {
 +				if (errno == EINTR)
 +                                	continue;
 +				else
 +					die(0);
                           }
                           if (ch >= 'A' && ch <= 'Z')
                                   ch += 'a' - 'A';
 
 -- 
 Nate Eldredge
 nge at cs.hmc.edu