owenpshaw updated this revision to Diff 128991.
owenpshaw added a comment.
Updated patch with new function name suggested by @clayborg. Added unit test
and changed to llvm::function_ref as suggested by @labath.
Based on Greg's comments in the other thread, I've kept the new function
separate, rather than use a flag in SendPacketAndWaitForResponse.
Is it worth creating a function to share the locking code that's common to
SendPacketAndWaitForResponse and SendPacketAndReceiveResponseWithOutputSupport?
Alternatively, should the lock-related error logging be unique to distinguish
between the two functions?
https://reviews.llvm.org/D41745
Files:
source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
Index: unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
===================================================================
--- unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
+++ unittests/Process/gdb-remote/GDBRemoteClientBaseTest.cpp
@@ -339,3 +339,25 @@
ASSERT_TRUE(async_result.get());
ASSERT_EQ(eStateInvalid, continue_state.get());
}
+
+TEST_F(GDBRemoteClientBaseTest, SendPacketAndReceiveResponseWithOutputSupport) {
+ StringExtractorGDBRemote response;
+ StreamString command_output;
+
+ ASSERT_EQ(PacketResult::Success, server.SendPacket("O"));
+ ASSERT_EQ(PacketResult::Success, server.SendPacket("O48656c6c6f2c"));
+ ASSERT_EQ(PacketResult::Success, server.SendPacket("O20"));
+ ASSERT_EQ(PacketResult::Success, server.SendPacket("O"));
+ ASSERT_EQ(PacketResult::Success, server.SendPacket("O776f726c64"));
+ ASSERT_EQ(PacketResult::Success, server.SendPacket("OK"));
+
+ PacketResult result = client.SendPacketAndReceiveResponseWithOutputSupport(
+ "qRcmd,test", response, true,
+ [&command_output](llvm::StringRef output) {
+ command_output << output;
+ });
+
+ ASSERT_EQ(PacketResult::Success, result);
+ ASSERT_EQ("OK", response.GetStringRef());
+ ASSERT_EQ("Hello, world", command_output.GetString().str());
+}
Index: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -5154,10 +5154,13 @@
bool send_async = true;
StringExtractorGDBRemote response;
- process->GetGDBRemote().SendPacketAndWaitForResponse(
- packet.GetString(), response, send_async);
- result.SetStatus(eReturnStatusSuccessFinishResult);
Stream &output_strm = result.GetOutputStream();
+ process->GetGDBRemote().SendPacketAndReceiveResponseWithOutputSupport(
+ packet.GetString(), response, send_async,
+ [&output_strm](llvm::StringRef output) {
+ output_strm << output;
+ });
+ result.SetStatus(eReturnStatusSuccessFinishResult);
output_strm.Printf(" packet: %s\n", packet.GetData());
const std::string &response_str = response.GetStringRef();
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -232,6 +232,10 @@
PacketResult ReadPacket(StringExtractorGDBRemote &response,
Timeout<std::micro> timeout, bool sync_on_timeout);
+ PacketResult ReadPacketWithOutputSupport(StringExtractorGDBRemote &response,
+ Timeout<std::micro> timeout, bool sync_on_timeout,
+ std::function<void(llvm::StringRef)> output_callback);
+
// Pop a packet from the queue in a thread safe manner
PacketResult PopPacketFromQueue(StringExtractorGDBRemote &response,
Timeout<std::micro> timeout);
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
@@ -274,6 +274,24 @@
return result;
}
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunication::ReadPacketWithOutputSupport(
+ StringExtractorGDBRemote &response,
+ Timeout<std::micro> timeout,
+ bool sync_on_timeout,
+ std::function<void(llvm::StringRef)> output_callback) {
+ auto result = ReadPacket(response, timeout, sync_on_timeout);
+ while (result == PacketResult::Success && response.IsNormalResponse() &&
+ response.PeekChar() == 'O') {
+ response.GetChar();
+ std::string output;
+ if (response.GetHexByteString(output))
+ output_callback(output);
+ result = ReadPacket(response, timeout, sync_on_timeout);
+ }
+ return result;
+}
+
GDBRemoteCommunication::PacketResult
GDBRemoteCommunication::ReadPacket(StringExtractorGDBRemote &response,
Timeout<std::micro> timeout,
Index: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h
@@ -48,6 +48,12 @@
StringExtractorGDBRemote &response,
bool send_async);
+ PacketResult SendPacketAndReceiveResponseWithOutputSupport(
+ llvm::StringRef payload,
+ StringExtractorGDBRemote &response,
+ bool send_async,
+ llvm::function_ref<void(llvm::StringRef)> output_callback);
+
bool SendvContPacket(llvm::StringRef payload,
StringExtractorGDBRemote &response);
Index: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
===================================================================
--- source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
@@ -176,6 +176,31 @@
return SendPacketAndWaitForResponseNoLock(payload, response);
}
+GDBRemoteCommunication::PacketResult
+GDBRemoteClientBase::SendPacketAndReceiveResponseWithOutputSupport(
+ llvm::StringRef payload,
+ StringExtractorGDBRemote &response,
+ bool send_async,
+ llvm::function_ref<void(llvm::StringRef)> output_callback) {
+ Lock lock(*this, send_async);
+ if (!lock) {
+ if (Log *log =
+ ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS))
+ log->Printf("GDBRemoteClientBase::%s failed to get mutex, not sending "
+ "packet '%.*s' (send_async=%d)",
+ __FUNCTION__, int(payload.size()), payload.data(),
+ send_async);
+ return PacketResult::ErrorSendFailed;
+ }
+
+ PacketResult packet_result = SendPacketNoLock(payload);
+ if (packet_result != PacketResult::Success)
+ return packet_result;
+
+ return ReadPacketWithOutputSupport(response, GetPacketTimeout(), true,
+ output_callback);
+}
+
GDBRemoteCommunication::PacketResult
GDBRemoteClientBase::SendPacketAndWaitForResponseNoLock(
llvm::StringRef payload, StringExtractorGDBRemote &response) {
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
