[Lldb-commits] [PATCH] D90490: [intel-pt][trace] Implement a "get supported trace type" packet

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

So down to just one question about the packet definition about if we should have an error code in JSON or if an error code even makes sense. Other than that this LGTM



================
Comment at: lldb/docs/lldb-gdb-remote.txt:262
+send packet: jLLDBTraceSupportedType
+read packet: {"name": <name>, "description", <description>}/E<error code>
+
----------------
I know the deprecated trace packets allows an error code to be returned, but since we already have JSON being used here, I would be a shame to not return an error in the JSON with a string that is human readable instead of a EXX where XX are two hex digits. can we say the response is either:
```
{"name": <name>, "description", <description>}
```
or
```
{"error": <error-string>}
```
Or just have no error code? What would the error be able to tell us?


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:3462
+  StreamGDBRemote escaped_packet;
+  escaped_packet.PutCString("jLLDBTraceSupportedType");
+
----------------
I agree Pavel, lets remove the old code when we can.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90490