[Lldb-commits] [PATCH] D104422: [trace] Add a TraceCursor class

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Just a few nits!



================
Comment at: lldb/include/lldb/Target/Trace.h:292-293
+  /// \return
+  ///     The stop ID of the live process being traced, or \b 1 if this is a
+  ///     post-mortem trace session.
+  uint32_t GetStopID();
----------------
No need to differentiate between line and post mortem stop IDs. Just say "The stop ID of the process being traced, or an invalid stop ID of zero if the trace is in an error state."


================
Comment at: lldb/include/lldb/Target/Trace.h:367
 
+  /// We use 1 as a default stop id for post portem processes.
+  uint32_t m_stop_id = 1;
----------------
We really shouldn't have to set this manually. Probably best to start it at zero. It might even be better to create a LLDB_INVALID_STOPID define next to the LLDB_INVALID_ADDRESS define and use it. 


================
Comment at: lldb/source/Target/Trace.cpp:478
+uint32_t Trace::GetStopID() {
+  RefreshLiveProcessState();
+  return m_stop_id;
----------------
I would just allow the core file to set the process state like any other and start the m_stop_id at zero, the invalid value.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104422