[Lldb-commits] [PATCH] D34945: Adding Support for Error Strings in Remote Packets

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Thu Jul 6 02:42:14 PDT 2017

labath added a comment.

I am generally happy with this, just a couple of things I noticed below:

Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:3329
     } else {
-      error.SetError(response.GetError(), eErrorTypeGeneric);
+        error = response.GetStatus();
The indenting look wrong. Please run the patch through clang-format before submission.

Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:158
+  PacketResult Handle_QErrorStringInPacketSupported(StringExtractorGDBRemote &packet);
It doesn't look like you're implementing this anywhere. Please remove.

Comment at: source/Utility/StringExtractorGDBRemote.cpp:26
+      if (m_packet[3] == ';') {
+        auto err_string = m_packet.substr(4);
+        for(auto e : err_string)
Please use StringRef here. No need to copy the string just to examine it's contents.

Comment at: source/Utility/StringExtractorGDBRemote.cpp:29
+          if (!isxdigit(e))
+            break;
+        return eError;
This will break out of the inner for loop, which is probably not what you intended.


More information about the lldb-commits mailing list