[Lldb-commits] [lldb] 39f2b05 - [lldb] [Host] Make Terminal methods return llvm::Error
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Tue Oct 19 04:31:12 PDT 2021
Author: Michał Górny
Date: 2021-10-19T13:31:03+02:00
New Revision: 39f2b059633ec1dc51b10b3fb48b616d87c273e3
URL: https://github.com/llvm/llvm-project/commit/39f2b059633ec1dc51b10b3fb48b616d87c273e3
DIFF: https://github.com/llvm/llvm-project/commit/39f2b059633ec1dc51b10b3fb48b616d87c273e3.diff
LOG: [lldb] [Host] Make Terminal methods return llvm::Error
Differential Revision: https://reviews.llvm.org/D111890
Added:
Modified:
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
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/Terminal.h b/lldb/include/lldb/Host/Terminal.h
index ba70acf720d2d..f8f5d5d16ef90 100644
--- 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 @@ class Terminal {
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 @@ class Terminal {
/// 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 @@ class TerminalState {
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
diff --git a/lldb/source/Host/common/Terminal.cpp b/lldb/source/Host/common/Terminal.cpp
index 2be9a6d6b0ece..7050bae0c3c1c 100644
--- 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::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 @@ bool TerminalState::Save(Terminal term, bool save_process_group) {
#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
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
index 443a65a2b3801..65f5e34527140 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -435,8 +435,9 @@ class IOHandlerPythonInterpreter : public IOHandler {
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
index bb719bcbf77db..e4dfe0a6e7353 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4349,8 +4349,9 @@ class IOHandlerProcessSTDIO : public IOHandler {
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
index dc5c6e9d0f37e..cdc80bea95679 100644
--- a/lldb/unittests/Host/posix/TerminalTest.cpp
+++ b/lldb/unittests/Host/posix/TerminalTest.cpp
@@ -51,11 +51,11 @@ TEST_F(TerminalTest, PipeIsNotATerminal) {
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, SetEcho) {
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);
}
More information about the lldb-commits
mailing list