[Lldb-commits] [lldb] 58b4501 - [lldb] [Host] Refactor TerminalState
Michał Górny via lldb-commits
lldb-commits at lists.llvm.org
Fri Oct 1 03:53:31 PDT 2021
Author: Michał Górny
Date: 2021-10-01T12:53:21+02:00
New Revision: 58b4501eeabb2728a5c48e05295f3636db0ecee1
URL: https://github.com/llvm/llvm-project/commit/58b4501eeabb2728a5c48e05295f3636db0ecee1
DIFF: https://github.com/llvm/llvm-project/commit/58b4501eeabb2728a5c48e05295f3636db0ecee1.diff
LOG: [lldb] [Host] Refactor TerminalState
Refactor TerminalState to make the code simpler. Move 'struct termios'
to a PImpl-style subclass. Add an RAII interface to automatically store
and restore the state.
Differential revision: https://reviews.llvm.org/D110721
Added:
Modified:
lldb/include/lldb/Host/Terminal.h
lldb/source/Host/common/Terminal.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
lldb/source/Target/Process.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/Terminal.h b/lldb/include/lldb/Host/Terminal.h
index b22b96776c335..081a427fdb50d 100644
--- a/lldb/include/lldb/Host/Terminal.h
+++ b/lldb/include/lldb/Host/Terminal.h
@@ -13,8 +13,6 @@
#include "lldb/Host/Config.h"
#include "lldb/lldb-private.h"
-struct termios;
-
namespace lldb_private {
class Terminal {
@@ -41,17 +39,29 @@ class Terminal {
int m_fd; // This may or may not be a terminal file descriptor
};
-/// \class State Terminal.h "lldb/Host/Terminal.h"
-/// A terminal state saving/restoring class.
+/// \class TerminalState Terminal.h "lldb/Host/Terminal.h"
+/// A RAII-friendly terminal state saving/restoring class.
///
/// 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 {
+ class Data;
+
public:
- /// Default constructor
- TerminalState();
+ /// Construct a new instance and optionally save terminal state.
+ ///
+ /// \param[in] term
+ /// The Terminal instance holding the file descriptor to save the state
+ /// of. If the instance is not associated with a fd, no state will
+ /// be saved.
+ ///
+ /// \param[in] save_process_group
+ /// If \b true, save the process group settings, else do not
+ /// save the process group settings for a TTY.
+ TerminalState(Terminal term = -1, bool save_process_group = false);
- /// Destructor
+ /// Destroy the instance, restoring terminal state if saved. If restoring
+ /// state is undesirable, the instance needs to be reset before destruction.
~TerminalState();
/// Save the TTY state for \a fd.
@@ -60,8 +70,8 @@ class TerminalState {
/// "save_process_group" is true, attempt to save the process group info for
/// the TTY.
///
- /// \param[in] fd
- /// The file descriptor to save the state of.
+ /// \param[in] term
+ /// The Terminal instance holding fd to save.
///
/// \param[in] save_process_group
/// If \b true, save the process group settings, else do not
@@ -70,7 +80,7 @@ class TerminalState {
/// \return
/// Returns \b true if \a fd describes a TTY and if the state
/// was able to be saved, \b false otherwise.
- bool Save(int fd, bool save_process_group);
+ bool Save(Terminal term, bool save_process_group);
/// Restore the TTY state to the cached state.
///
@@ -115,12 +125,9 @@ class TerminalState {
bool ProcessGroupIsValid() const;
// Member variables
- Terminal m_tty; ///< A terminal
- int m_tflags = -1; ///< Cached tflags information.
-#if LLDB_ENABLE_TERMIOS
- std::unique_ptr<struct termios>
- m_termios_up; ///< Cached terminal state information.
-#endif
+ 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.
};
diff --git a/lldb/source/Host/common/Terminal.cpp b/lldb/source/Host/common/Terminal.cpp
index 147bbdcde137d..6adcfa6a92c41 100644
--- a/lldb/source/Host/common/Terminal.cpp
+++ b/lldb/source/Host/common/Terminal.cpp
@@ -81,63 +81,45 @@ bool Terminal::SetCanonical(bool enabled) {
return false;
}
-// Default constructor
-TerminalState::TerminalState()
- : m_tty()
+struct TerminalState::Data {
#if LLDB_ENABLE_TERMIOS
- ,
- m_termios_up()
+ struct termios m_termios; ///< Cached terminal state information.
#endif
-{
+};
+
+TerminalState::TerminalState(Terminal term, bool save_process_group)
+ : m_tty(term) {
+ Save(term, save_process_group);
}
-// Destructor
-TerminalState::~TerminalState() = default;
+TerminalState::~TerminalState() { Restore(); }
void TerminalState::Clear() {
m_tty.Clear();
m_tflags = -1;
-#if LLDB_ENABLE_TERMIOS
- m_termios_up.reset();
-#endif
+ m_data.reset();
m_process_group = -1;
}
-// Save the current state of the TTY for the file descriptor "fd" and if
-// "save_process_group" is true, attempt to save the process group info for the
-// TTY.
-bool TerminalState::Save(int fd, bool save_process_group) {
- m_tty.SetFileDescriptor(fd);
+bool TerminalState::Save(Terminal term, bool save_process_group) {
+ Clear();
+ m_tty = term;
if (m_tty.IsATerminal()) {
+ int fd = m_tty.GetFileDescriptor();
#if LLDB_ENABLE_POSIX
m_tflags = ::fcntl(fd, F_GETFL, 0);
-#endif
#if LLDB_ENABLE_TERMIOS
- if (m_termios_up == nullptr)
- m_termios_up.reset(new struct termios);
- int err = ::tcgetattr(fd, m_termios_up.get());
- if (err != 0)
- m_termios_up.reset();
-#endif // #if LLDB_ENABLE_TERMIOS
-#if LLDB_ENABLE_POSIX
+ std::unique_ptr<Data> new_data{new Data()};
+ if (::tcgetattr(fd, &new_data->m_termios) != 0)
+ m_data = std::move(new_data);
+#endif // LLDB_ENABLE_TERMIOS
if (save_process_group)
- m_process_group = ::tcgetpgrp(0);
- else
- m_process_group = -1;
-#endif
- } else {
- m_tty.Clear();
- m_tflags = -1;
-#if LLDB_ENABLE_TERMIOS
- m_termios_up.reset();
-#endif
- m_process_group = -1;
+ m_process_group = ::tcgetpgrp(fd);
+#endif // LLDB_ENABLE_POSIX
}
return IsValid();
}
-// Restore the state of the TTY using the cached values from a previous call to
-// Save().
bool TerminalState::Restore() const {
#if LLDB_ENABLE_POSIX
if (IsValid()) {
@@ -147,8 +129,8 @@ bool TerminalState::Restore() const {
#if LLDB_ENABLE_TERMIOS
if (TTYStateIsValid())
- tcsetattr(fd, TCSANOW, m_termios_up.get());
-#endif // #if LLDB_ENABLE_TERMIOS
+ tcsetattr(fd, TCSANOW, &m_data->m_termios);
+#endif // LLDB_ENABLE_TERMIOS
if (ProcessGroupIsValid()) {
// Save the original signal handler.
@@ -161,30 +143,19 @@ bool TerminalState::Restore() const {
}
return true;
}
-#endif
+#endif // LLDB_ENABLE_POSIX
return false;
}
-// Returns true if this object has valid saved TTY state settings that can be
-// used to restore a previous state.
bool TerminalState::IsValid() const {
return m_tty.FileDescriptorIsValid() &&
- (TFlagsIsValid() || TTYStateIsValid());
+ (TFlagsIsValid() || TTYStateIsValid() || ProcessGroupIsValid());
}
-// Returns true if m_tflags is valid
bool TerminalState::TFlagsIsValid() const { return m_tflags != -1; }
-// Returns true if m_ttystate is valid
-bool TerminalState::TTYStateIsValid() const {
-#if LLDB_ENABLE_TERMIOS
- return m_termios_up != nullptr;
-#else
- return false;
-#endif
-}
+bool TerminalState::TTYStateIsValid() const { return bool(m_data); }
-// Returns true if m_process_group is valid
bool TerminalState::ProcessGroupIsValid() const {
return static_cast<int32_t>(m_process_group) != -1;
}
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 5bbb67313a261..67595069323c3 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -355,7 +355,6 @@ struct InitializePythonRAII {
PyEval_InitThreads();
}
- TerminalState m_stdin_tty_state;
PyGILState_STATE m_gil_state = PyGILState_UNLOCKED;
bool m_was_already_initialized = false;
};
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
index 33afd02bbdee3..b6926b61ca06a 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h
@@ -430,11 +430,9 @@ class IOHandlerPythonInterpreter : public IOHandler {
int stdin_fd = GetInputFD();
if (stdin_fd >= 0) {
Terminal terminal(stdin_fd);
- TerminalState terminal_state;
- const bool is_a_tty = terminal.IsATerminal();
+ TerminalState terminal_state(terminal);
- if (is_a_tty) {
- terminal_state.Save(stdin_fd, false);
+ if (terminal.IsATerminal()) {
terminal.SetCanonical(false);
terminal.SetEcho(true);
}
@@ -464,9 +462,6 @@ class IOHandlerPythonInterpreter : public IOHandler {
run_string.Printf("run_python_interpreter (%s)",
m_python->GetDictionaryName());
PyRun_SimpleString(run_string.GetData());
-
- if (is_a_tty)
- terminal_state.Restore();
}
}
SetIsDone(true);
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 540f58d2ebc9e..59fea136c12d0 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4351,9 +4351,8 @@ class IOHandlerProcessSTDIO : public IOHandler {
SetIsDone(false);
const int read_fd = m_read_file.GetDescriptor();
- TerminalState terminal_state;
- terminal_state.Save(read_fd, false);
Terminal terminal(read_fd);
+ TerminalState terminal_state(terminal, false);
terminal.SetCanonical(false);
terminal.SetEcho(false);
// FD_ZERO, FD_SET are not supported on windows
@@ -4399,7 +4398,6 @@ class IOHandlerProcessSTDIO : public IOHandler {
}
m_is_running = false;
#endif
- terminal_state.Restore();
}
void Cancel() override {
More information about the lldb-commits
mailing list