[Lldb-commits] [PATCH] D110721: [lldb] [Host] Merge TerminalState into Terminal

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.


================
Comment at: lldb/source/Host/common/Terminal.cpp:124
     if (save_process_group)
-      m_process_group = ::tcgetpgrp(0);
     else
----------------
I think passing `0` here was a mistake. The `Restore()` method passed `fd` instead.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:358
 
-  TerminalState m_stdin_tty_state;
   PyGILState_STATE m_gil_state = PyGILState_UNLOCKED;
----------------
This instance didn't seem to be used at all.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h:468
         if (is_a_tty)
-          terminal_state.Restore();
+          terminal.RestoreState();
       }
----------------
I'm wondering if we could use the RAII approach here and restore state (if saved) in the destructor. I'm just not 100% sure if that's what `Debugger` expects.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110721/new/

https://reviews.llvm.org/D110721