mgorny updated this revision to Diff 380609.
mgorny marked 2 inline comments as done.
mgorny added a comment.
Make `Terminal::Data` private and befriend `TerminalState`.
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
@@ -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();
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ 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(
Index: lldb/source/Host/common/Terminal.cpp
===================================================================
--- lldb/source/Host/common/Terminal.cpp
+++ 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::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
- }
- 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> 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> 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<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,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<Data> 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<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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
