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

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/docs/lldb-gdb-remote.txt:248
+//  {
+//    "name": <tracing technology name, e.g. intel-pt, arm-coresight>
+//    "description": <description string for this technology>
----------------
Should this specify that this name needs to match the name of the LLDB plug-in?


================
Comment at: lldb/include/lldb/Utility/StringExtractorGDBRemote.h:165-169
     eServerPacketType_jTraceStart,
     eServerPacketType_jTraceBufferRead,
     eServerPacketType_jTraceMetaRead,
     eServerPacketType_jTraceStop,
     eServerPacketType_jTraceConfigRead,
----------------
Mark as deprecated?


================
Comment at: lldb/include/lldb/Utility/StringExtractorGDBRemote.h:170
     eServerPacketType_jTraceConfigRead,
+    eServerPacketType_jLLDBTraceSupportedType,
   };
----------------
Maybe add a space to separate the above deprecated stuff from this new stuff


================
Comment at: lldb/include/lldb/Utility/TraceOptions.h:21
+struct TraceTypeInfo {
+  /// The name of the technology, e.g. intel-pt or arm-coresight.
+  std::string name;
----------------
We should specify that this needs to match the LLDB trace plug-in name on the header doc.


================
Comment at: lldb/include/lldb/lldb-enumerations.h:772
 
 enum TraceType {
   eTraceTypeNone = 0,
----------------
Mark as deprecated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90490