[Lldb-commits] [PATCH] D62499: Create a generic handler for Xfer packets

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

Thanks for doing this work! I'd love to see a faster LLDB. :)



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2697-2717
+  std::string xfer_object;
+  if (packet.GetStringBeforeChar(xfer_object, ':') == 0)
+    return SendIllFormedResponse(packet, "qXfer packet missing object name");
+  // Consume ':'
+  packet.GetChar();
+  // Parse action (read/write).
+  std::string xfer_action;
----------------
labath wrote:
> I think it would be simpler to just forget about the StringExtractor here, and do something like
> ```
> SmallVector<StringRef, 4> fields;
> StringRef(packet.GetStringRef()).split(fields, 3);
> if (fields.size() != 4) return "Malformed packet";
> // After this, "object" is in fields[0], "action" is in fields[1], annex is in fields[2], and the rest is in fields[3]
> std::string buffer_key = fields[0] + fields[2];
> ...
> ```
I agree, that would make it simpler.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2738
+  if (!memory_buffer_sp) {
+    if (xfer_object == "auxv") {
+// *BSD impls should be able to do this too.
----------------
labath wrote:
> Given that this function is going to grow, it would be good to split it in smaller chunks. Maybe move this code into something like `ErrorOr<xfer_map::iterator> ReadObject(StringRef object)`?
+1

You could have smaller methods like "ParseAuxvPacket" and "ReadObject"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62499/new/

https://reviews.llvm.org/D62499