JDevlieghere accepted this revision. JDevlieghere added a comment. This revision is now accepted and ready to land.
Some code style nits but the change itself LGTM. ================ Comment at: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:819-821 + if (ondisk_object_file) + ondisk_objfile_macho = + llvm::dyn_cast<ObjectFileMachO>(ondisk_object_file); ---------------- You can use `dyn_cast_or_null` to eliminate the null check: ```ObjectFileMachO *ondisk_objfile_macho = llvm::dyn_cast_or_null<ObjectFileMachO>(ondisk_object_file);``` I hate temporaries that are used only once below, so I'd probably write: ``` ObjectFileMachO *ondisk_objfile_macho = llvm::dyn_cast_or_null<ObjectFileMachO>(m_module_sp ? m_module_sp->GetObjectFile() : nullptr); ``` but I'm probably the only one that likes that better. ================ Comment at: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:835 + m_name.c_str(), m_uuid.GetAsString().c_str()); + m_module_sp.reset(); + } ---------------- What's the purpose of reseting the shared pointer? It'll just go out of scope right after. ================ Comment at: lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:881-883 + ObjectFileMachO *memory_objfile_macho = + llvm::dyn_cast<ObjectFileMachO>(memory_object_file); + if (memory_objfile_macho) { ---------------- Since you already did it for `Section` below... ``` if(ObjectFileMachO *memory_objfile_macho = llvm::dyn_cast<ObjectFileMachO>(memory_object_file)) { ``` ================ Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:1146 + return m_header.filetype == MH_KEXT_BUNDLE; +} uint32_t ObjectFileMachO::GetAddressByteSize() const { ---------------- Newline below Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145547/new/ https://reviews.llvm.org/D145547 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits