mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, teemperor.
mgorny requested review of this revision.

Note: this is just the initial patch, I'm doing more refactoring to dedupe the 
code a bit.


https://reviews.llvm.org/D111890

Files:
  lldb/include/lldb/Host/Terminal.h
  lldb/source/Host/common/Terminal.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
  lldb/source/Target/Process.cpp
  lldb/unittests/Host/posix/TerminalTest.cpp

Index: lldb/unittests/Host/posix/TerminalTest.cpp
===================================================================
--- lldb/unittests/Host/posix/TerminalTest.cpp
+++ lldb/unittests/Host/posix/TerminalTest.cpp
@@ -51,11 +51,11 @@
 TEST_F(TerminalTest, SetEcho) {
   struct termios terminfo;
 
-  ASSERT_EQ(m_term.SetEcho(true), true);
+  ASSERT_THAT_ERROR(m_term.SetEcho(true), llvm::Succeeded());
   ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
   EXPECT_NE(terminfo.c_lflag & ECHO, 0U);
 
-  ASSERT_EQ(m_term.SetEcho(false), true);
+  ASSERT_THAT_ERROR(m_term.SetEcho(false), llvm::Succeeded());
   ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
   EXPECT_EQ(terminfo.c_lflag & ECHO, 0U);
 }
@@ -63,11 +63,11 @@
 TEST_F(TerminalTest, SetCanonical) {
   struct termios terminfo;
 
-  ASSERT_EQ(m_term.SetCanonical(true), true);
+  ASSERT_THAT_ERROR(m_term.SetCanonical(true), llvm::Succeeded());
   ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
   EXPECT_NE(terminfo.c_lflag & ICANON, 0U);
 
-  ASSERT_EQ(m_term.SetCanonical(false), true);
+  ASSERT_THAT_ERROR(m_term.SetCanonical(false), llvm::Succeeded());
   ASSERT_EQ(tcgetattr(m_fd, &terminfo), 0);
   EXPECT_EQ(terminfo.c_lflag & ICANON, 0U);
 }
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -4353,8 +4353,9 @@
     const int read_fd = m_read_file.GetDescriptor();
     Terminal terminal(read_fd);
     TerminalState terminal_state(terminal, false);
-    terminal.SetCanonical(false);
-    terminal.SetEcho(false);
+    // FIXME: error handling?
+    llvm::consumeError(terminal.SetCanonical(false));
+    llvm::consumeError(terminal.SetEcho(false));
 // FD_ZERO, FD_SET are not supported on windows
 #ifndef _WIN32
     const int pipe_read_fd = m_pipe.GetReadFileDescriptor();
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -433,8 +433,9 @@
         TerminalState terminal_state(terminal);
 
         if (terminal.IsATerminal()) {
-          terminal.SetCanonical(false);
-          terminal.SetEcho(true);
+          // FIXME: error handling?
+          llvm::consumeError(terminal.SetCanonical(false));
+          llvm::consumeError(terminal.SetEcho(true));
         }
 
         ScriptInterpreterPythonImpl::Locker locker(
Index: lldb/source/Host/common/Terminal.cpp
===================================================================
--- lldb/source/Host/common/Terminal.cpp
+++ lldb/source/Host/common/Terminal.cpp
@@ -23,62 +23,86 @@
 
 bool Terminal::IsATerminal() const { return m_fd >= 0 && ::isatty(m_fd); }
 
-bool Terminal::SetEcho(bool enabled) {
-  if (FileDescriptorIsValid()) {
+llvm::Error Terminal::SetEcho(bool enabled) {
+  if (!FileDescriptorIsValid())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "invalid fd");
+
 #if LLDB_ENABLE_TERMIOS
-    if (IsATerminal()) {
-      struct termios fd_termios;
-      if (::tcgetattr(m_fd, &fd_termios) == 0) {
-        bool set_corectly = false;
-        if (enabled) {
-          if (fd_termios.c_lflag & ECHO)
-            set_corectly = true;
-          else
-            fd_termios.c_lflag |= ECHO;
-        } else {
-          if (fd_termios.c_lflag & ECHO)
-            fd_termios.c_lflag &= ~ECHO;
-          else
-            set_corectly = true;
-        }
-
-        if (set_corectly)
-          return true;
-        return ::tcsetattr(m_fd, TCSANOW, &fd_termios) == 0;
-      }
-    }
-#endif // #if LLDB_ENABLE_TERMIOS
+  if (!IsATerminal())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "fd not a terminal");
+
+  struct termios fd_termios;
+  if (::tcgetattr(m_fd, &fd_termios) != 0)
+    return llvm::createStringError(
+        std::error_code(errno, std::generic_category()),
+        "unable to get teletype attributes");
+
+  bool set_corectly = false;
+  if (enabled) {
+    if (fd_termios.c_lflag & ECHO)
+      set_corectly = true;
+    else
+      fd_termios.c_lflag |= ECHO;
+  } else {
+    if (fd_termios.c_lflag & ECHO)
+      fd_termios.c_lflag &= ~ECHO;
+    else
+      set_corectly = true;
   }
-  return false;
+
+  if (!set_corectly && ::tcsetattr(m_fd, TCSANOW, &fd_termios) != 0)
+    return llvm::createStringError(
+        std::error_code(errno, std::generic_category()),
+        "unable to set teletype attributes");
+
+  return llvm::Error::success();
+#else // !LLDB_ENABLE_TERMIOS
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 "termios support disabled");
+#endif // LLDB_ENABLE_TERMIOS
 }
 
-bool Terminal::SetCanonical(bool enabled) {
-  if (FileDescriptorIsValid()) {
+llvm::Error Terminal::SetCanonical(bool enabled) {
+  if (!FileDescriptorIsValid())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "invalid fd");
+
 #if LLDB_ENABLE_TERMIOS
-    if (IsATerminal()) {
-      struct termios fd_termios;
-      if (::tcgetattr(m_fd, &fd_termios) == 0) {
-        bool set_corectly = false;
-        if (enabled) {
-          if (fd_termios.c_lflag & ICANON)
-            set_corectly = true;
-          else
-            fd_termios.c_lflag |= ICANON;
-        } else {
-          if (fd_termios.c_lflag & ICANON)
-            fd_termios.c_lflag &= ~ICANON;
-          else
-            set_corectly = true;
-        }
-
-        if (set_corectly)
-          return true;
-        return ::tcsetattr(m_fd, TCSANOW, &fd_termios) == 0;
-      }
-    }
-#endif // #if LLDB_ENABLE_TERMIOS
+  if (!IsATerminal())
+    return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                   "fd not a terminal");
+
+  struct termios fd_termios;
+  if (::tcgetattr(m_fd, &fd_termios) != 0)
+    return llvm::createStringError(
+        std::error_code(errno, std::generic_category()),
+        "unable to get teletype attributes");
+
+  bool set_corectly = false;
+  if (enabled) {
+    if (fd_termios.c_lflag & ICANON)
+      set_corectly = true;
+    else
+      fd_termios.c_lflag |= ICANON;
+  } else {
+    if (fd_termios.c_lflag & ICANON)
+      fd_termios.c_lflag &= ~ICANON;
+    else
+      set_corectly = true;
   }
-  return false;
+
+  if (!set_corectly && ::tcsetattr(m_fd, TCSANOW, &fd_termios) != 0)
+    return llvm::createStringError(
+        std::error_code(errno, std::generic_category()),
+        "unable to set teletype attributes");
+
+  return llvm::Error::success();
+#else  // !LLDB_ENABLE_TERMIOS
+  return llvm::createStringError(llvm::inconvertibleErrorCode(),
+                                 "termios support disabled");
+#endif // #if LLDB_ENABLE_TERMIOS
 }
 
 struct TerminalState::Data {
Index: lldb/include/lldb/Host/Terminal.h
===================================================================
--- lldb/include/lldb/Host/Terminal.h
+++ lldb/include/lldb/Host/Terminal.h
@@ -12,6 +12,7 @@
 
 #include "lldb/Host/Config.h"
 #include "lldb/lldb-private.h"
+#include "llvm/Support/Error.h"
 
 namespace lldb_private {
 
@@ -31,9 +32,9 @@
 
   void Clear() { m_fd = -1; }
 
-  bool SetEcho(bool enabled);
+  llvm::Error SetEcho(bool enabled);
 
-  bool SetCanonical(bool enabled);
+  llvm::Error SetCanonical(bool enabled);
 
 protected:
   int m_fd; // This may or may not be a terminal file descriptor
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] D11... Michał Górny via Phabricator via lldb-commits

Reply via email to