[translation] Rewrite and translate PushTermAttrs(), PopTermAttrs()

Add a repro bug #1629 in spec/stateful, and fix it.

Author: Andy C

core/pyos.py

@@ -236,34 +236,34 @@ def PrintTimes():
 TERM_ECHO = termios.ECHO
 
 
-class TermState(object):
+def PushTermAttrs(fd, mask):
+  # type: (int, int) -> Tuple[int, Any]
   """
-  TODO: Make this into a context manager which is a C++ destructor?
+  Returns opaque type (void* in C++) to be reused in the PopTermAttrs()
   """
-  def __init__(self, fd, mask):
-    # type: (int, int) -> None
-    self.fd = fd
-
-    # silly way to make a copy
-    # https://docs.python.org/2/library/termios.html
-    self.orig_attrs = termios.tcgetattr(fd)
-    term_attrs = termios.tcgetattr(fd)
-
-    a3 = cast(int, term_attrs[3])
-    # Disable canonical (buffered) mode.  See `man termios` for an extended
-    # discussion.
-    term_attrs[3] = a3 & mask
-    termios.tcsetattr(self.fd, termios.TCSANOW, term_attrs)
-
-  def Restore(self):
-    # type: () -> None
-    try:
-      termios.tcsetattr(self.fd, termios.TCSANOW, self.orig_attrs)
-    except termios.error as e:
-      # Superficial fix for issue #1001.  I'm not sure why we get errno.EIO,
-      # but we can't really handle it here.  In C++ I guess we ignore the
-      # error.
-      pass
+  # https://docs.python.org/2/library/termios.html
+  term_attrs = termios.tcgetattr(fd)
+
+  # Flip the bits in one field, e.g. ICANON to disable canonical (buffered)
+  # mode.
+  orig_local_modes = cast(int, term_attrs[3])
+  term_attrs[3] = orig_local_modes & mask
+
+  termios.tcsetattr(fd, termios.TCSANOW, term_attrs)
+  return orig_local_modes, term_attrs
+
+
+def PopTermAttrs(fd, orig_local_modes, term_attrs):
+  # type: (int, int, Any) -> None
+
+  term_attrs[3] = orig_local_modes
+  try:
+    termios.tcsetattr(fd, termios.TCSANOW, term_attrs)
+  except termios.error as e:
+    # Superficial fix for issue #1001.  I'm not sure why we get errno.EIO,
+    # but we can't really handle it here.  In C++ I guess we ignore the
+    # error.
+    pass
 
 
 def OsType():

cpp/core.cc

@@ -14,6 +14,7 @@
 #include <sys/times.h>     // tms / times()
 #include <sys/utsname.h>   // uname
 #include <sys/wait.h>      // waitpid()
+#include <termios.h>       // tcgetattr(), tcsetattr()
 #include <time.h>          // time()
 #include <unistd.h>        // getuid(), environ
 
@@ -265,6 +266,32 @@ Tuple2<Str*, int>* MakeDirCacheKey(Str* path) {
   return Alloc<Tuple2<Str*, int>>(path, st.st_mtime);
 }
 
+Tuple2<int, void*> PushTermAttrs(int fd, int mask) {
+  struct termios* term_attrs =
+      static_cast<struct termios*>(malloc(sizeof(struct termios)));
+
+  if (tcgetattr(fd, term_attrs) < 0) {
+    throw Alloc<OSError>(errno);
+  }
+  // Flip the bits in one field
+  int orig_local_modes = term_attrs->c_lflag;
+  term_attrs->c_lflag = orig_local_modes & mask;
+
+  if (tcsetattr(fd, TCSANOW, term_attrs) < 0) {
+    throw Alloc<OSError>(errno);
+  }
+
+  return Tuple2<int, void*>(orig_local_modes, term_attrs);
+}
+
+void PopTermAttrs(int fd, int orig_local_modes, void* term_attrs) {
+  struct termios* t = static_cast<struct termios*>(term_attrs);
+  t->c_lflag = orig_local_modes;
+  if (tcsetattr(fd, TCSANOW, t) < 0) {
+    ;  // Like Python, ignore error because of issue #1001
+  }
+}
+
 }  // namespace pyos
 
 namespace pyutil {

cpp/core.h

@@ -94,18 +94,8 @@ inline void FlushStdout() {
   fflush(stdout);
 }
 
-class TermState {
- public:
-  TermState(int fd, int mask) {
-    assert(0);
-  }
-  void Restore() {
-    assert(0);
-  }
-  static constexpr ObjHeader obj_header() {
-    return ObjHeader::ClassFixed(kZeroMask, sizeof(TermState));
-  }
-};
+Tuple2<int, void*> PushTermAttrs(int fd, int mask);
+void PopTermAttrs(int fd, int orig_local_modes, void* term_attrs);
 
 // Make the signal queue slab 4096 bytes, including the GC header.  See
 // cpp/core_test.cc.

osh/builtin_misc.py

@@ -260,6 +260,27 @@ def ReadAll():
   return ''.join(chunks)
 
 
+class ctx_TermAttrs(object):
+
+  def __init__(self, fd, local_modes):
+    # type: (int, int) -> None
+    self.fd = fd
+
+    # We change term_attrs[3] in Python, which is lflag "local modes"
+    orig_local_modes, term_attrs = pyos.PushTermAttrs(fd, local_modes)
+
+    self.orig_local_modes = orig_local_modes
+    self.term_attrs = term_attrs
+
+  def __enter__(self):
+    # type: () -> None
+    pass
+
+  def __exit__(self, type, value, traceback):
+    # type: (Any, Any, Any) -> None
+    pyos.PopTermAttrs(self.fd, self.orig_local_modes, self.term_attrs)
+
+
 class Read(vm._Builtin):
 
   def __init__(self, splitter, mem, parse_ctx, cmd_ev, errfmt):
@@ -399,11 +420,8 @@ def _Run(self, cmd_val):
     if bits == 0:
       status = self._Read(arg, names)
     else:
-      term = pyos.TermState(STDIN_FILENO, ~bits)
-      try:
+      with ctx_TermAttrs(STDIN_FILENO, ~bits):
         status = self._Read(arg, names)
-      finally:
-        term.Restore()
     return status
 
   def _Read(self, arg, names):

spec/stateful/signals.py

@@ -316,6 +316,21 @@ def t2(sh):
   sh.expect('status=130')
 
 
+@register()
+def read_d(sh):
+  'Ctrl-C during read -d'
+
+  sh.sendline('read -d :')
+
+  time.sleep(0.1)
+  sh.sendintr()  # SIGINT
+
+  expect_prompt(sh)
+
+  sh.sendline('echo status=$?')
+  sh.expect('status=130')
+
+
 @register()
 def c_wait(sh):
   'Ctrl-C (untrapped) during wait builtin'