[Lldb-commits] [PATCH] D60001: Allow partial UUID matching in Minidump core file plug-in

Mon Apr 1 16:30:07 PDT 2019

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

================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:391
+    // add the module to the target if it finds one, so we might need to remove
+    // it if there is an error.
     lldb::ModuleSP module_sp = GetTarget().GetSharedModule(module_spec, &error);
----------------
Will fix

================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:401
+      basename_module_spec.GetUUID().Clear();
+      basename_module_spec.GetFileSpec().GetDirectory().Clear();
+      module_sp = GetTarget().GetSharedModule(basename_module_spec, &error);
----------------
labath wrote:
> What will happen if you just delete this line? Based on my experiments, I believe we should still end up looking through the exec-search-paths list. However, the test does not seem to be modifying this setting, so it's not clear to me how it is even finding the binary in the first place. What's the mechanism you're relying on for the lookup in the test?
It doesn't work without clearing the fullpath here and I didn't want to modify the current behavior of the exec search paths in here just in case it would affect others that were depending on a certain behavior.

================
Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:408
+        // We have a match if either UUID is empty or as long as the actual
+        // UUID starts with the minidump UUID
+        const bool match = dmp_bytes.empty() || mod_bytes.empty() ||
----------------
I will change it

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

https://reviews.llvm.org/D60001