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.

2 files changed, 17 insertions, 23 deletions

diff --git a/ChangeLog b/ChangeLog
index e7853f874..fcd01ab4f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2017-03-03  Peter Stephenson  <p.stephenson@samsung.com>
 
+	* Sebastain: 40170: Src/Modules/curses.c: Fix up error number
+	resetting in curses module.  This appears to resolve an issue
+	mentioned in comments but attributed elsewhere, so remove
+	confusion here.
+
 	* 40173: Test/V11db_gdbm.ztst: don't report an error if gdbm
 	module doesn't load as this simply causes the test to be skipped.
 
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;