clayborg added inline comments.
================
Comment at: lldb/include/lldb/Target/Target.h:1421
+protected:
+ struct DummySignalValues {
+ LazyBool pass = eLazyBoolCalculate;
----------------
We should make UnixSignals::Signal a public class and then just save a
std::vector of those inside this class. That class doesn't contain a signal
number, so we should be able to re-use it here and avoid creating a new class
that mimic what is contains.
================
Comment at: lldb/include/lldb/Target/Target.h:1429
+ };
+ using DummySignalElement = std::pair<std::string, DummySignalValues>;
+ static bool UpdateSignalFromDummy(lldb::UnixSignalsSP signals_sp,
----------------
Maybe store a std::vector<UnixSignal::Signal> objects filled out as much as
possible here? Then we don't need the DummySignalElement type at all.
================
Comment at: lldb/include/lldb/Target/Target.h:1533-1536
+ std::map<std::string, DummySignalValues> m_dummy_signals;/// These are used
to
+ /// set the signal state when you don't have a process and more usefully
+ /// in the Dummy target where you can't know exactly what signals you
+ /// will have.
----------------
Switch to std::vector< UnixSignal::Signal> here and then we can prime the
UnixSignals with the vector?
================
Comment at: lldb/include/lldb/Target/UnixSignals.h:120
bool m_suppress : 1, m_stop : 1, m_notify : 1;
+ bool m_default_suppress : 1, m_default_stop : 1, m_default_notify : 1;
----------------
If we store a vector<UnixSignals::Signal> in the target, can we reset the
default values from another "Signal" struct instead of saving it here? Not a
big deal if not, but just wondering.
================
Comment at: lldb/include/lldb/Target/UnixSignals.h:126
~Signal() = default;
+ void Reset(bool reset_stop, bool reset_notify, bool reset_suppress);
};
----------------
To follow up with he above comment, this _could_ become:
```
void Reset(const UnixSignals::Signal &signal);
```
we can find the UnixSignals::Signal object for this signal first in the target
and pass it down into here?
================
Comment at: lldb/source/Commands/Options.td:754
+ def process_handle_dummy : Option<"dummy", "d">, Group<2>,
+ Desc<"Also clear the values in the dummy target so they won't be inherited
by new targets.">;
}
----------------
Do we need the "Also" in the documentation here? Is this option only available
when used with another option?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126259/new/
https://reviews.llvm.org/D126259
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
