clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed.
================ Comment at: lldb/include/lldb/API/SBTrace.h:31 + + /// Start tracing a live process using a provided configuration. /// ---------------- ================ Comment at: lldb/include/lldb/API/SBTrace.h:37-38 /// - /// \param[in] buf - /// Buffer to write the trace data to. + /// See the process tracing params from \a GetStartConfigurationHelp() for + /// more information. /// ---------------- ================ Comment at: lldb/include/lldb/API/SBTrace.h:44 + + /// Start tracing a live thread using a provided configuration. /// ---------------- ================ Comment at: lldb/include/lldb/API/SBTrace.h:50-51 /// - /// \param[in] thread_id - /// Tracing could be started for the complete process or a - /// single thread, in the first case the traceid obtained would - /// map to all the threads existing within the process and the - /// ones spawning later. The thread_id parameter can be used in - /// such a scenario to select the trace data for a specific - /// thread. + /// See the thread tracing params from \a GetStartConfigurationHelp() for + /// more information. /// ---------------- Modify to match the suggested process header doc mentioned above. ================ Comment at: lldb/include/lldb/API/SBTrace.h:57 - /// Obtain any meta data as raw bytes for the tracing instance. - /// The input parameter definition is similar to the previous - /// function. - size_t GetMetaData(SBError &error, void *buf, size_t size, size_t offset = 0, - lldb::tid_t thread_id = LLDB_INVALID_THREAD_ID); - - /// Stop the tracing instance. Stopping the trace will also - /// lead to deletion of any gathered trace data. - /// - /// \param[out] error - /// An error explaining what went wrong. + /// Stop tracing a live process. /// ---------------- ================ Comment at: lldb/include/lldb/API/SBTrace.h:63 - /// Get the trace configuration being used for the trace instance. - /// The threadid in the SBTraceOptions needs to be set when the - /// configuration used by a specific thread is being requested. - /// - /// \param[out] options - /// The trace options actually used by the trace instance - /// would be filled by the API. + /// Stop tracing a live thread. /// ---------------- Can this only be used if we started tracing this specific thread? Or can we start tracing all threads in a process and then turn off tracing for individual threads? Might be good to specify this in these docs. ================ Comment at: lldb/include/lldb/API/SBTrace.h:91-92 + /// deprecated lldb::ProcessWP m_opaque_wp; + lldb::TraceSP m_opaque_sp; }; ---------------- Reverse these to match how they were laid out before. That way if anyone accesses m_opaque_wp it will match the type. ================ Comment at: lldb/include/lldb/Target/Target.h:1137-1141 + llvm::Expected<lldb::TraceSP &> CreateTrace(); + + /// If a \a Trace object is present, this returns it, otherwise a new Trace is + /// created with \a Trace::CreateTrace. llvm::Expected<lldb::TraceSP &> GetTraceOrCreate(); ---------------- Do we need both of these? ================ Comment at: lldb/source/Plugins/Trace/intel-pt/constants.h:1 +//===-- constants.h ---------------------------------------------*- C++ -*-===// +// ---------------- This header file name is a bit too generic and could end up causing include issues. I would rename to "TraceIntelPTConstants.h" to be safe. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103500/new/ https://reviews.llvm.org/D103500 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
