[Lldb-commits] [PATCH] D30520: Make LLDB skip server-client roundtrip for signals that don't require any actions

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.





================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3766-3776
+  llvm::SmallVector<int32_t, 40> signals_to_ignore;
+  int32_t signo = m_unix_signals_sp->GetFirstSignalNumber();
+  while (signo != LLDB_INVALID_SIGNAL_NUMBER) {
+    bool should_ignore = !m_unix_signals_sp->GetShouldStop(signo) &&
+                         !m_unix_signals_sp->GetShouldNotify(signo) &&
+                         !m_unix_signals_sp->GetShouldSuppress(signo);
+
----------------
eugene wrote:
> jingham wrote:
> > This code should go into the UnixSignals class.  Any other Process plugin that wanted to implement expedited signal handling would also have to do this computation, and we shouldn't duplicate the code.
> I think it would be a mistake to over-engineer it and try to anticipate needs of any possible implementation at this point.
> Maybe another implementation would require a list of signals that need to be stopped instead of ignored. 
> 
> Whenever  we have an alternative implementation that needs to do such thins we can always go back and generalize this code, as for now I think we need to keep things simple. 
> 
> I like the concise interface that UnixSignals provides and  I don't want to pollute it with things can be easily done in the code that uses it.
I agree with Eugene. I think there are advantages to the UnixSignals class being "dumb", and having the process plugin decide what to do with the data there. If anything, what this code needs is a better signal iteration API, so you could write this as something like:
```
for (auto signal: signals) {
  if (!signal.stop && !signal.notify && !signal.supress)
    my_list.push_back(signal.number);
}
```


https://reviews.llvm.org/D30520