labath added a comment.
Jim's comment about putting the function in the parent class makes sense --
apart from remote stubs I guess you could also envision other ways a process
class could speed up signal processing in case it know we don't care about
them).
Jim: do you have a idea about how to write a test for this? These
performance-improvement kind of changes don't fit really well into the SB API
test framework, as this change should not have any observable behavior, and we
already have tests that make sure that "process handle" does the right thing
(i.e., we don't stop for ignored signals). The best thing I can come up with
(without going on a major refactoring effort) is enabling logging and then
grepping it for some string.
Actually, I can think of a potential improvement. Move the code currently in
`ProcessGDBRemote::SendSignalsToIgnoreIfNeeded` to
`GDBRemoteCommunicationClient`. The latter is unit-testable so it would give as
more coverage, and I think an API like `Error
GDBRemoteCommunicationClient::IgnoreSignalsIfSupported(const UnixSignals
&signals)` would still make sense. It won't solve the problem of testing the
whole thing end-to-end, but it would certainly increase the test coverage of
this change.
Let me know what you think.
================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:1067
"using %s %s",
- __FUNCTION__, process_arch.GetArchitectureName()
- ? process_arch.GetArchitectureName()
----------------
It looks like you reformatted the whole file instead of just your changes.
Random diff makes review harder. If you set it up correctly `git clang-format`
should only format the code around your changes, so I recommend using that.
================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3795
+ LLDB_LOG(log, "Error: {0}", error);
+ }
+ return error;
----------------
No braces.
================
Comment at:
unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp:332
+
+ HandlePacket(server, "QPassSignals:2;3;5;7;B;D;11", "OK");
+ EXPECT_TRUE(result.get().Success());
----------------
The documentation for QPassSignals
<https://sourceware.org/gdb/onlinedocs/gdb/General-Query-Packets.html> states
that "Signals are numbered identically to continue packets and stop replies".
While it does not say that directly, I would interpret that as "you should
always use two digits for the signal number" because that's how stop-reply
packets work. It's fine to keep the receiving code lax, but let's be stricter
in what we generate.
https://reviews.llvm.org/D30520
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
