diff --git a/lldb/include/lldb/Host/Terminal.h b/lldb/include/lldb/Host/Terminal.h --- a/lldb/include/lldb/Host/Terminal.h +++ b/lldb/include/lldb/Host/Terminal.h @@ -12,9 +12,12 @@ #include "lldb/Host/Config.h" #include "lldb/lldb-private.h" +#include "llvm/Support/Error.h" namespace lldb_private { +class TerminalState; + class Terminal { public: Terminal(int fd = -1) : m_fd(fd) {} @@ -31,12 +34,19 @@ void Clear() { m_fd = -1; } - bool SetEcho(bool enabled); + llvm::Error SetEcho(bool enabled); - bool SetCanonical(bool enabled); + llvm::Error SetCanonical(bool enabled); protected: + struct Data; + int m_fd; // This may or may not be a terminal file descriptor + + llvm::Expected GetData(); + llvm::Error SetData(const Data &data); + + friend class TerminalState; }; /// \class TerminalState Terminal.h "lldb/Host/Terminal.h" @@ -45,8 +55,6 @@ /// This class can be used to remember the terminal state for a file /// descriptor and later restore that state as it originally was. class TerminalState { - struct Data; - public: /// Construct a new instance and optionally save terminal state. /// @@ -125,10 +133,10 @@ bool ProcessGroupIsValid() const; // Member variables - Terminal m_tty; ///< A terminal - int m_tflags = -1; ///< Cached tflags information. - std::unique_ptr m_data; ///< Platform-specific implementation. - lldb::pid_t m_process_group = -1; ///< Cached process group information. + Terminal m_tty; ///< A terminal + int m_tflags = -1; ///< Cached tflags information. + std::unique_ptr m_data; ///< Platform-specific implementation. + lldb::pid_t m_process_group = -1; ///< Cached process group information. }; } // namespace lldb_private diff --git a/lldb/source/Host/common/Terminal.cpp b/lldb/source/Host/common/Terminal.cpp --- a/lldb/source/Host/common/Terminal.cpp +++ b/lldb/source/Host/common/Terminal.cpp @@ -21,71 +21,78 @@ using namespace lldb_private; +struct Terminal::Data { +#if LLDB_ENABLE_TERMIOS + struct termios m_termios; ///< Cached terminal state information. +#endif +}; + bool Terminal::IsATerminal() const { return m_fd >= 0 && ::isatty(m_fd); } -bool Terminal::SetEcho(bool enabled) { - if (FileDescriptorIsValid()) { +llvm::Expected Terminal::GetData() { + 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 - } - return false; + if (!IsATerminal()) + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "fd not a terminal"); + + Data data; + if (::tcgetattr(m_fd, &data.m_termios) != 0) + return llvm::createStringError( + std::error_code(errno, std::generic_category()), + "unable to get teletype attributes"); + return data; +#else // !LLDB_ENABLE_TERMIOS + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "termios support missing in LLDB"); +#endif // LLDB_ENABLE_TERMIOS } -bool Terminal::SetCanonical(bool enabled) { - if (FileDescriptorIsValid()) { +llvm::Error Terminal::SetData(const Terminal::Data &data) { #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 - } - return false; + assert(FileDescriptorIsValid()); + assert(IsATerminal()); + + if (::tcsetattr(m_fd, TCSANOW, &data.m_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 + llvm_unreachable("SetData() should not be called if !LLDB_ENABLE_TERMIOS"); +#endif // LLDB_ENABLE_TERMIOS } -struct TerminalState::Data { +llvm::Error Terminal::SetEcho(bool enabled) { + llvm::Expected data = GetData(); + if (!data) + return data.takeError(); + #if LLDB_ENABLE_TERMIOS - struct termios m_termios; ///< Cached terminal state information. -#endif -}; + struct termios &fd_termios = data->m_termios; + fd_termios.c_lflag &= ~ECHO; + if (enabled) + fd_termios.c_lflag |= ECHO; + return SetData(data.get()); +#endif // LLDB_ENABLE_TERMIOS +} + +llvm::Error Terminal::SetCanonical(bool enabled) { + llvm::Expected data = GetData(); + if (!data) + return data.takeError(); + +#if LLDB_ENABLE_TERMIOS + struct termios &fd_termios = data->m_termios; + fd_termios.c_lflag &= ~ICANON; + if (enabled) + fd_termios.c_lflag |= ICANON; + return SetData(data.get()); +#endif // LLDB_ENABLE_TERMIOS +} TerminalState::TerminalState(Terminal term, bool save_process_group) : m_tty(term) { @@ -109,7 +116,7 @@ #if LLDB_ENABLE_POSIX m_tflags = ::fcntl(fd, F_GETFL, 0); #if LLDB_ENABLE_TERMIOS - std::unique_ptr new_data{new Data()}; + std::unique_ptr new_data{new Terminal::Data()}; if (::tcgetattr(fd, &new_data->m_termios) == 0) m_data = std::move(new_data); #endif // LLDB_ENABLE_TERMIOS diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h @@ -435,8 +435,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( diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -4349,8 +4349,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(); diff --git a/lldb/unittests/Host/posix/TerminalTest.cpp b/lldb/unittests/Host/posix/TerminalTest.cpp --- a/lldb/unittests/Host/posix/TerminalTest.cpp +++ b/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); }