labath added inline comments.

================
Comment at: lldb/docs/lldb-gdb-remote.txt:245
+//
+//  See lldb-enumerations for the list of available TraceType's.
+//
----------------
DavidSpickett wrote:
> Saying `lldb-enumerations.h` might save someone some grepping time, also 
> saying that it should be in decimal.
> 
> Can you clarify what TraceType means? Is it:
> The way the trace is generated/stored, like on chip vs off chip
> The thing you're tracging, e.g. instructions or power usage
> Specific trace versions e.g intel pt, arm coresight (pretty sure it's not 
> this one)
> 
> If there's the possibility of one system having multiple types then the 
> packet should at least have the format to hold them, even though we only read 
> the first one.
> 
> Which your code already does since:
> 1,2;
> Would stop parsing at the ',' anyway, but worth noting if it's relevant.
The `j` in these trace packets was supposed to refer to the fact they are 
json-formatted. Using json for this is overkill, but that means I'm now torn 
between wanting to preserve the `j` consistency and the `jTrace` consistency. 
If this were returning a list of types, using json for that might not be 
totally unreasonable, and would let us avoid this dilemma :P.


================
Comment at: lldb/docs/lldb-gdb-remote.txt:254
+send packet: jTraceSupportedType
+read packet: <trace type>/E<error code>;AAAAAAAAA
+
----------------
DavidSpickett wrote:
> What is the AAAAA for?
That's the extended (textual) error message -- lldb and gdb have independently 
invented incompatible formats for that (I share a blame it that :/).

This style of presentation is consistent with the other jTrace packets, though 
I am not sure it's that helpful really -- we support the extended error on any 
response packet and we don't document that always. (Also the error message will 
only be sent if the client actually signals support for it.)

(In fact, when I look at the implementation, I see that it will _never_ send 
the textual error message.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90490

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to