[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

[Andreu Carminati via Phabricator via lldb-commits]

andcarminati added inline comments.


================
Comment at: lldb/tools/driver/Driver.cpp:673-674
+static void sigtstp_handler(int signo) {
   if (g_driver != nullptr)
     g_driver->GetDebugger().SaveInputTerminalState();
 
----------------
JDevlieghere wrote:
> labath wrote:
> > JDevlieghere wrote:
> > > I see an opportunity for a little RAII helper.
> > What kind of a helper did you have in mind? Practically the entire function consists of setup and teardown in preparation for the `raise(signo)` call. If I wanted to be fancy I could put all of that in a helper, but I don't think that would make it cleaner. Plus, we also need to be careful about the functions we call from a signal handler, and I really don't know whether e.g. `llvm::make_scope_exit` is guaranteed to not allocate (heap) memory.
> I was only referring to the Save/RestoreInputTerminalState() part of this function. Something like:
> 
> ```
> class TerminalStateRAII() {
> public:
>   TerminalStateRAII(Driver* driver) : driver(m_driver) {
>     if (m_driver)
>       m_driver->GetDebugger().SaveInputTerminalState();
>   }
> 
>   ~SignalHelper() {
>     if (m_driver)
>       m_driver->GetDebugger().SaveInputTerminalState();
>   }
> 
> private:
>   Driver* m_driver;
> };
> ```
> 
> Obviously, this isn't at all important, just something that came to mind.
I think this is a good idea to reduce code duplication. Another approach:

```
class TerminalStateRAII() {
public:
  TerminalStateRAII(Driver* driver) : driver(m_driver) {
    SaveInputTerminalState();
  }

  ~TerminalStateRAII() {
    SaveInputTerminalState();
  }

private:
  Driver* m_driver;
  void SaveInputTerminalState(){
    if (m_driver)
      m_driver->GetDebugger().SaveInputTerminalState();
  }
};

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120320