labath added a comment.
I think this is a pretty good start. The main thing bugging me is the two bool
flags on the stop reply packet functions. I wonder if it would be more readable
to split this into two functions. Something like:
SendStopReplyForThread(bool force_synchronous /*seems better than
allow_async, but that could just be me*/, bool enqueue /*enqueues just this
thread*/);
EnqueueStopReplyPackets(NativeThreadProtocol &thread_to_skip);
perhaps?
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3749
+ return SendErrorResponse(Status("No pending notification to ack"));
+ m_stop_notification_queue.pop_front();
+ if (!m_stop_notification_queue.empty())
----------------
Is this correct? I would expect to this to first read (`front()`) the packet
before popping it.
================
Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1414
+
+ def test_QNonStop(self):
+ self.build()
----------------
Could you put these into a new file? TestLldbGdbServer is already one of the
longest running tests...
================
Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1443
+ m = self.expect_gdbremote_sequence()
+ del m["O_content"]
+ threads = [m]
----------------
Is this necessary? (I get that it's not useful, but it seems weird...)
================
Comment at: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py:1525
+ procs = self.prep_debug_monitor_and_inferior(
+ inferior_args=["thread:new"])
+ self.test_sequence.add_log_lines(
----------------
Are you sure this is not racy (like, can the inferior terminate before we get a
chance to interrupt it)?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125575/new/
https://reviews.llvm.org/D125575
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
