[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling
Jonas Devlieghere via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu Feb 24 17:18:20 PST 2022
JDevlieghere 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();
----------------
andcarminati wrote:
> 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();
> }
> };
>
> ```
That's a typo on my part, the destructor needs to call `RestoreInputTerminalState` (as opposed to `SaveInputTerminalState`).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120320/new/
https://reviews.llvm.org/D120320
More information about the lldb-commits
mailing list