[Lldb-commits] [PATCH] D55522: Cache memory regions in ProcessMinidump and use the linux maps as the source of the information if available.

[Greg Clayton via Phabricator via lldb-commits]

clayborg marked 6 inline comments as done.
clayborg added inline comments.


================
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:412
+  constexpr const auto yes = MemoryRegionInfo::eYes;
+  constexpr const auto no = MemoryRegionInfo::eNo;
+  while (!text.empty()) {
----------------
I agree on the Optional<bool>, but since this enum is exposed via the public API, I would leave it alone for now?


================
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:583
+  if (!m_regions.empty())
+    return FindMemoryRegion(load_addr);
 
----------------
I fixed the logic so FindMemoryRegion work with the empty region. And will set a bool to indicate we parsed the memory regions and not rely on m_regions being empty for that.


================
Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:536
+  auto pos = std::lower_bound(begin, end, load_addr);
+  if (pos != end && pos->GetRange().Contains(load_addr))
+    return *pos;
----------------
Fixed the logic in here to be able to have empty m_regions.


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

https://reviews.llvm.org/D55522