about summary refs log tree commit diff
path: root/Src/Modules
diff options
context:
space:
mode:
authorSebastian Gniazdowski <psprint3@fastmail.com>2017-03-02 23:48:09 -0800
committerPeter Stephenson <pws@zsh.org>2017-03-03 10:46:23 +0000
commitc8005af3102adfd80f778a86096dbd712e3f60bc (patch)
tree5e24cefdd6889321a361b0b6d1b023699bf427ab /Src/Modules
parentefe5e6a2f035d368b62c04b559e78393e0e5322d (diff)
downloadzsh-c8005af3102adfd80f778a86096dbd712e3f60bc.tar.gz
zsh-c8005af3102adfd80f778a86096dbd712e3f60bc.tar.xz
zsh-c8005af3102adfd80f778a86096dbd712e3f60bc.zip
40170: Fix up error resetting in curses module.
Update comment to remove confusion.  The comment was based on
incorrecto expectations, and the problems referred to seem to be fixed.
Diffstat (limited to 'Src/Modules')
-rw-r--r--Src/Modules/curses.c35
1 files changed, 12 insertions, 23 deletions
diff --git a/Src/Modules/curses.c b/Src/Modules/curses.c
index 63c6748f5..d9c19bdb1 100644
--- a/Src/Modules/curses.c
+++ b/Src/Modules/curses.c
@@ -1082,15 +1082,7 @@ zccmd_input(const char *nam, char **args)
 #endif
 
     /*
-     * Some documentation for wgetch() says:
-
-       The behavior of getch and friends in the presence of  handled  signals
-       is  unspecified  in the SVr4 and XSI Curses documentation.  Under his-
-       torical curses implementations, it varied  depending  on  whether  the
-       operating system's implementation of handled signal receipt interrupts
-       a read(2) call in progress or not, and also (in some  implementations)
-       depending  on  whether  an input timeout or non-blocking mode has been
-       set.
+     * Linux, OS X, FreeBSD documentation for wgetch() mentions:
 
        Programmers concerned about portability should be prepared for  either
        of  two cases: (a) signal receipt does not interrupt getch; (b) signal
@@ -1098,21 +1090,16 @@ zccmd_input(const char *nam, char **args)
        EINTR.  Under the ncurses implementation, handled signals never inter-
        rupt getch.
 
-     * The observed behavior, however, is different:  wgetch() consistently
-     * returns ERR with EINTR when a signal is handled by the shell "trap"
-     * command mechanism.  Further, it consistently returns ERR twice, the
-     * second time without even attempting to repeat the interrupted read,
-     * which has the side-effect of NOT updating errno.  A third call will
-     * then begin reading again.
-     *
-     * Therefore, to properly implement signal trapping, we must (1) call
-     * wgetch() in a loop as long as errno remains EINTR, and (2) clear
-     * errno only before beginning the loop, not on every pass.
+     * Some observed behavior: wgetch() returns ERR with EINTR when a signal is
+     * handled by the shell "trap" command mechanism. Observed that it returns
+     * ERR twice, the second time without even attempting to repeat the
+     * interrupted read. Third call will then begin reading again.
      *
-     * There remains a potential bug here in that, if the caller has set
-     * a timeout for the read [see zccmd_timeout()] the countdown is very
-     * likely restarted on every call to wgetch(), so an interrupted call
-     * might wait much longer than desired.
+     * Because of widespread of previous implementation that called wget*ch
+     * possibly indefinitely many times after ERR/EINTR, and because of the
+     * above observation, wget_wch call is repeated after each ERR/EINTR, but
+     * errno is being reset (it wasn't) and the loop to all means should break.
+     * Problem: the timeout may be waited twice.
      */
     errno = 0;
 
@@ -1120,6 +1107,7 @@ zccmd_input(const char *nam, char **args)
     while ((ret = wget_wch(w->win, &wi)) == ERR) {
 	if (errno != EINTR || errflag || retflag || breaks || exit_pending)
 	    break;
+        errno = 0;
     }
     switch (ret) {
     case OK:
@@ -1146,6 +1134,7 @@ zccmd_input(const char *nam, char **args)
     while ((ci = wgetch(w->win)) == ERR) {
 	if (errno != EINTR || errflag || retflag || breaks || exit_pending)
 	    return 1;
+        errno = 0;
     }
     if (ci >= 256) {
 	keypadnum = ci;