labath added a comment.
Thank you for writing the tests. I have two stylistic comments, but otherwise
looks great.
================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:113
@@ -105,4 +112,3 @@
case 'J':
- // Asynchronous JSON packet, destined for a
- // StructuredDataPlugin.
{
+ // Verify this J packet is a JSON-async: packet.
----------------
I'd like to move this code to a separate function. The main job of
`SendContinuePacketAndWaitForResponse` is dealing with the thread
synchronization issues, which is tricky enough without having json parsing in
the middle of it.
================
Comment at: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp:371
@@ +370,3 @@
+ ASSERT_EQ("T01", response.GetStringRef());
+ ASSERT_EQ(fix.delegate.structured_data_entries.size(), 1);
+
----------------
Please put the "expected" values first (`ASSERT_NE(expected, actual)`).
Otherwise the error message will come out wrong when the comparison fails. The
same issue is present in a number of other checks.
https://reviews.llvm.org/D23884
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
