[Lldb-commits] [PATCH] D32585: Implementation of remote packets for Trace data.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

This patch does nicely follow the way other GDB remote packets are implemented. I wonder if we should just have a "jTrace" packet that is JSON from the start? This is more of a question to anyone that cares about the direction of the GDB remote protocol we are using. We might not multiple flavors of the qTrace packet in that case. Just add a key/value pair that say what the packet command is (start, stop, get trace data, get metadata, etc). I am curious to see what others think. I don't have any objections to this patch as is, but just wanted to check.



================
Comment at: docs/lldb-gdb-remote.txt:212
 //----------------------------------------------------------------------
+// QTrace:1:type:<type>;
+//
----------------
Should we make all these new packets JSON based to start with? "jTrace"? If we have any need for JSON at all in this or the other new packets I would say lets just go with JSON packets. They are currently prefixed with "j". If we go this route we should specify the mandatory key/value pairs in the header doc. We should also allow a JSON dictionary from the trace config up at the SBTrace layer to make it into this packet? 


https://reviews.llvm.org/D32585