labath added a comment.
Just some layering remarks. Other than that, I think this is fine.
================
Comment at: lldb/include/lldb/Host/common/NativeProcessProtocol.h:17
#include "lldb/Host/MainLoop.h"
+#include "lldb/Target/Trace.h"
#include "lldb/Utility/ArchSpec.h"
----------------
You shouldn't include `lldb/Target` from here. We already have
Utility/TraceOptions.h. Maybe TraceTypeInfo could go there instead?
================
Comment at: lldb/include/lldb/Target/Process.h:2547-2548
+ /// See the information on the jLLDBTraceSupportedType packet in
+ /// lldb-gdb-remote.txt.
+ ///
----------------
This code shouldn't refer the the gdb packets. Theoretically we could have
other process plugins supporting this.
================
Comment at: lldb/include/lldb/Target/Trace.h:184-185
+/// This struct represents a return value from the jLLDBTraceSupportedType
+/// packet in lldb-gdb-remote.txt
+struct TraceTypeInfo {
----------------
Neither should this.
================
Comment at: lldb/source/Plugins/Process/Linux/ProcessorTrace.cpp:60
+ StringRef buffer = intel_pt_type_text.get()->getBuffer();
+ if (buffer.empty() || buffer.trim().getAsInteger(10, intel_pt_type))
+ return createStringError(
----------------
the empty check is superfluous.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90490/new/
https://reviews.llvm.org/D90490
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
