[Lldb-commits] [lldb] [LLDB][Minidump] Fix ProcessMinidump::GetMemoryRegions to include 64b regions when /proc/pid maps are missing. (PR #101086)

[Pavel Labath via lldb-commits]

================
@@ -553,53 +547,49 @@ static bool
 CreateRegionsCacheFromMemoryList(MinidumpParser &parser,
                                  std::vector<MemoryRegionInfo> &regions) {
   Log *log = GetLog(LLDBLog::Modules);
+  // Cache the expected memory32 into an optional
+  // because double checking the expected triggers the unchecked warning.
+  std::optional<llvm::ArrayRef<MemoryDescriptor>> memory32_list;
   auto ExpectedMemory = parser.GetMinidumpFile().getMemoryList();
   if (!ExpectedMemory) {
     LLDB_LOG_ERROR(log, ExpectedMemory.takeError(),
                    "Failed to read memory list: {0}");
----------------
labath wrote:

I don't get the double-checked comment. Something like
```
  if (auto ExpectedMemory = parser.GetMinidumpFile().getMemoryList(); !ExpectedMemory) {
    LLDB_LOG_ERROR(log, ExpectedMemory.takeError(),
                   "Failed to read memory list: {0}");
  } else {
    for (const MemoryDescriptor &memory_desc : *ExpectedMemory) {
      ...
```

ought to work just fine (other ways to structure this are also possible, but I don't know if you can do an early exit or whatever...). Once you check the success state of the expected object and find that it succeeded, the check requirement is satisfied. No further checking is needed. That would be different if you tried to copy the value into some other (expected) object, as then you would need to check that one as well, but the solution to that is simple -- just don't do that. :)

https://github.com/llvm/llvm-project/pull/101086