[Lldb-commits] [PATCH] D41745: Handle O reply packets during qRcmd

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

The ProcessGdbRemote part is not really testable in it's current form, but for the rest, you should be able to write a gdbremoteclient unittest (see unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp).



================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:53
+      StringExtractorGDBRemote &response,
+      std::function<void(llvm::StringRef)> output_callback);
+
----------------
this could be an llvm::function_ref (more lightweight than std::function).


================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:5157
       Stream &output_strm = result.GetOutputStream();
+      process->GetGDBRemote().SendPacketHandlingOutput(
+          packet.GetString(), response,
----------------
I think the API would be cleaner if the output callback was an argument to the standard SendPacket function, where the (default) value of null would mean "don't do anything special with the O packets", although that might just be a personal preference.

However, even if we choose to have separate functions as the API, I believe they should delegate to a single unified implementation internally.


https://reviews.llvm.org/D41745