[Lldb-commits] [PATCH] D132191: Treat a UUID of all zeros consistently to mean "Invalid UUID"

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/source/API/SBModuleSpec.cpp:137
   LLDB_INSTRUMENT_VA(this, uuid, uuid_len)
-  m_opaque_up->GetUUID() = UUID::fromOptionalData(uuid, uuid_len);
+  m_opaque_up->GetUUID() = UUID::fromData(uuid, uuid_len);
   return m_opaque_up->GetUUID().IsValid();
----------------
If we want this API to be consistent to what it was doing before, then we need to check for all zeroes here too? 


================
Comment at: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:1392
       image_infos[i].SetName((const char *)name_data);
-      UUID uuid = UUID::fromOptionalData(extractor.GetData(&offset, 16), 16);
+      UUID uuid = UUID::fromData(extractor.GetData(&offset, 16), 16);
       image_infos[i].SetUUID(uuid);
----------------
Have you checked if the kernel ever just specifies all zeroes here? It it does, this will change things right?


================
Comment at: lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp:888
       case llvm::MachO::LC_UUID:
-        dylib_info.uuid = UUID::fromOptionalData(data.GetData(&offset, 16), 16);
+        dylib_info.uuid = UUID::fromData(data.GetData(&offset, 16), 16);
         break;
----------------
More LC_UUID values are valid AFAIK, so this should be ok.


================
Comment at: lldb/source/Plugins/Process/minidump/MinidumpParser.cpp:70-72
       if (pdb70_uuid->Age != 0)
-        return UUID::fromOptionalData(pdb70_uuid, sizeof(*pdb70_uuid));
-      return UUID::fromOptionalData(&pdb70_uuid->Uuid,
+        return UUID::fromData(pdb70_uuid, sizeof(*pdb70_uuid));
+      return UUID::fromData(&pdb70_uuid->Uuid,
----------------
Fix the UUID UUID::fromCvRecord(UUID::CvRecordPdb70) function and call that function from here.


================
Comment at: lldb/source/Utility/UUID.cpp:45
+    return UUID::fromData(&debug_info, sizeof(debug_info));
+  return UUID::fromData(&debug_info.Uuid, sizeof(debug_info.Uuid));
 }
----------------
Ditto above comment from the MinidumpParser. Actually this function should be used, the implementation from the MinidumpParser should use this function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132191