[Lldb-commits] [PATCH] D99571: Update ProcessMachCore::DoLoadCore to handle binary hints with and without addresses

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

Thanks for the feedback Shafik, updated the patch and landing in a sec.



================
Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:214
+      if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+        module_sp.reset(new Module(module_spec));
+      }
----------------
shafik wrote:
> We can use `make_shared` here instead.
Sounds good.


================
Comment at: lldb/test/API/macosx/lc-note/firmware-corefile/TestFirmwareCorefiles.py:142
+        main_addr = main_sym.GetStartAddress()
+        self.assertGreater(main_addr.GetLoadAddress(self.target), 0x70000000000)
+        self.assertNotEqual(main_addr.GetLoadAddress(self.target), lldb.LLDB_INVALID_ADDRESS)
----------------
shafik wrote:
> `0x70000000000` feels like it should be a variable with a name since we are using in multiple places.
Good suggestion, done.


================
Comment at: lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp:90
+    char buf[24];
+    sprintf(buf, "0x%llx", address);
+    ident += buf;
----------------
shafik wrote:
> `snprintf` we should also collect the return value and assert on the result.
Thanks, I switched to using a PRIx64 formatter to make it explicit how large the formatter's output will be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99571