mgorny updated this revision to Diff 380011. mgorny added a comment. Move common code and error handling to `GetData()` and `SetData()` methods.
CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111890/new/ 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 @@ -21,71 +21,102 @@ 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::Data> 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 + 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 +} + +llvm::Error Terminal::SetEcho(bool enabled) { + llvm::Expected<Data> data = GetData(); + if (!data) + return data.takeError(); + +#if LLDB_ENABLE_TERMIOS + struct termios &fd_termios = data->m_termios; + 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) + return llvm::Error::success(); + return SetData(data.get()); +#endif // LLDB_ENABLE_TERMIOS } -struct TerminalState::Data { +llvm::Error Terminal::SetCanonical(bool enabled) { + llvm::Expected<Data> 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; + 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 llvm::Error::success(); + return SetData(data.get()); +#endif // LLDB_ENABLE_TERMIOS +} TerminalState::TerminalState(Terminal term, bool save_process_group) : m_tty(term) { @@ -109,7 +140,7 @@ #if LLDB_ENABLE_POSIX m_tflags = ::fcntl(fd, F_GETFL, 0); #if LLDB_ENABLE_TERMIOS - std::unique_ptr<Data> new_data{new Data()}; + std::unique_ptr<Terminal::Data> new_data{new Terminal::Data()}; if (::tcgetattr(fd, &new_data->m_termios) == 0) m_data = std::move(new_data); #endif // LLDB_ENABLE_TERMIOS Index: lldb/include/lldb/Host/Terminal.h =================================================================== --- lldb/include/lldb/Host/Terminal.h +++ lldb/include/lldb/Host/Terminal.h @@ -12,11 +12,14 @@ #include "lldb/Host/Config.h" #include "lldb/lldb-private.h" +#include "llvm/Support/Error.h" namespace lldb_private { class Terminal { public: + struct Data; + Terminal(int fd = -1) : m_fd(fd) {} ~Terminal() = default; @@ -31,12 +34,15 @@ 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 + + llvm::Expected<Data> GetData(); + llvm::Error SetData(const Data &data); }; /// \class TerminalState Terminal.h "lldb/Host/Terminal.h" @@ -45,8 +51,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 +129,10 @@ bool ProcessGroupIsValid() const; // Member variables - Terminal m_tty; ///< A terminal - int m_tflags = -1; ///< Cached tflags information. - std::unique_ptr<Data> 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<Terminal::Data> m_data; ///< Platform-specific implementation. + lldb::pid_t m_process_group = -1; ///< Cached process group information. }; } // namespace lldb_private
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits