[Lldb-commits] [PATCH] D69105: minidump: Create memory regions from the sections of loaded modules

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:348-349
+  bool is_complete;
+  std::tie(*m_memory_regions, is_complete) =
+      m_minidump_parser->BuildMemoryRegions();
+
----------------
Might be nice to just assign memory regions and then ask the minidump parser what its source of memory regions was? I am thinking of:

```
m_memory_regions = m_minidump_parser->BuildMemoryRegions();
switch (m_minidump_parser->GetMemoryRegionsSource()) {
  case MemoryInfoList:
  case LinuxProcMaps:
    break;
  case MemoryList:
    // Memory list is not always a exhaustive list of memory regions in a process...
    // Insert code below the "if (is_complete)" in switch?
```

Actually thinking of this a bit more, we should really just do this work inside "m_minidump_parser->BuildMemoryRegions();". The minidump itself can then make the call and we don't need to return the "is_complete". Otherwise if anyone else uses the info from "m_minidump_parser->BuildMemoryRegions();" they would have to do the same thing as this code which would be bad. 



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

https://reviews.llvm.org/D69105