xiaobai added inline comments.
================ Comment at: source/Breakpoint/BreakpointSiteList.cpp:191 if (lower != m_bp_site_list.begin()) { - collection::const_iterator prev_pos = lower; - prev_pos--; + auto prev_pos = std::prev(lower); const BreakpointSiteSP &prev_bp = (*prev_pos).second; ---------------- If `m_bp_site_list` is empty, `prev_pos` could be `m_bp_site_list.end()`, I believe, so you would have to check for that here. This is assuming that this method can be invoked when `m_bp_site_list` is empty, which I'm not entirely sure about. ================ Comment at: source/Target/Process.cpp:1817 + m_breakpoint_site_list.ForEach([this](BreakpointSite &bp_site) { // bp_site->SetEnabled(true); + DisableBreakpointSite(&bp_site); ---------------- Good opportunity to delete dead code? ================ Comment at: source/Target/Process.cpp:2437 + for (auto &bp_site : enabled_bp_sites_in_range) + update_status(DisableSoftwareBreakpoint(bp_site.get())); ---------------- If disabling any breakpoint fails, the status's error string will be "failed to re-enable one or more breakpoints". This is kind of confusing, imo. ================ Comment at: unittests/Process/common/ProcessWriteMemoryTest.cpp:1 +//===-- ProcessorTraceMonitorTest.cpp ------------------------- -*- C++ -*-===// +// ---------------- `ProcessTraceMonitorTest.cpp` -> `ProcessWriteMemoryTest.cpp` https://reviews.llvm.org/D39967 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits