jasonmolenda added a comment.
Thanks for the feedback!
================
Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5567
uuid = UUID::fromOptionalData(raw_uuid, sizeof(uuid_t));
+ switch (binspec_type) {
+ case 0:
----------------
JDevlieghere wrote:
> It seems like this could be outlined into a little static helper function.
> Something along the lines of
>
> ```
> BinaryType GetBinaryType(uint32_t binspec_type);
> ```
I don't feel strongly, but all of the "main bin spec" specific knowledge is
centralized here in GetCorefileMainBinaryInfo, and converting the constant
values this LC_NOTE uses for binary type into the enumerated values seems best
kept local to this area.
================
Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:471
+ if (m_mach_kernel_addr != LLDB_INVALID_ADDRESS) {
+ LLDB_LOGF(log,
+ "ProcessMachCore::DoLoadCore: Using kernel corefile image "
----------------
JDevlieghere wrote:
> Maybe have a little lambda for the two bodies to reduce the duplication?
Yeah, it's not great - but we're checking them in opposite order in the
duplicated blocks, so I could make a lambda that did both, but it would need to
take an arg about which to prefer and wouldn't be that much better I think.
================
Comment at:
lldb/test/API/macosx/lc-note/firmware-corefile/create-empty-corefile.cpp:39
+add_uint64(std::vector<uint8_t> &buf, uint64_t val)
+{
+ uint64_buf conv;
----------------
JDevlieghere wrote:
> This does not look clang-formatted 🤐
oh my how did that happen.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88282/new/
https://reviews.llvm.org/D88282
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
