Date: Thu, 8 Mar 2007 10:40:09 GMT From: Nate Eldredge <nge@cs.hmc.edu> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/107171: systat(1) doesn't die when it's xterm is killed while it's running Message-ID: <200703081040.l28Ae9RA095807@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/107171; it has been noted by GNATS. From: Nate Eldredge <nge@cs.hmc.edu> To: bug-followup@FreeBSD.org, josh@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@cs.hmc.edu
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200703081040.l28Ae9RA095807>