JDevlieghere added inline comments.
================ Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5682-5693 + for (uint32_t i = 0; i < m_header.ncmds; ++i) { + const uint32_t cmd_offset = offset; + llvm::MachO::load_command lc = {}; + if (m_data.GetU32(&offset, &lc.cmd, 2) == nullptr) + break; + if (lc.cmd == LC_NOTE) { + char data_owner[17]; ---------------- There's a *lot* of duplication between reading `LC_NOTE`s. At some point we should extract the common code instead of copy-pasting it every time. ================ Comment at: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp:5723-5724 + } + const uint32_t num_threads = threads->GetSize(); + for (uint32_t i = 0; i < num_threads; i++) { + StructuredData::Dictionary *thread; ---------------- `StructuredData::Array::GetSize` returns a `size_t`. No point in turning this into a `uint32_t`. ================ Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:603 + + // populate used_tids + for (uint32_t i = 0; i < num_threads; i++) { ---------------- Capitalize + period. ================ Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:608-619 + // If any threads have an unspecified thread id, + // find an unused number, use it instead. + tid_t current_unused_tid = 0; + for (uint32_t i = 0; i < num_threads; i++) { + if (tids[i] == LLDB_INVALID_THREAD_ID) { + while (used_tids.find(current_unused_tid) != used_tids.end()) { + current_unused_tid++; ---------------- This code is (presumably on purpose) trying to find gaps in the available tids: for example if I had `[1, 2, ? ,4, ?, 6]` this is going to turn that into `[1, 2, (3), 4, (5), 6]`. If that property matters, it probably should be part of the comment. If it doesn't, you could exploit the fact that the `set` is sorted and just take the last value (6 for the previous example) and return `[1, 2, 4, 6, 7, 8]`, but again, I assume that's what you're trying to avoid here. ================ Comment at: lldb/source/Plugins/Process/mach-core/ProcessMachCore.cpp:628-629 + for (uint32_t i = 0; i < num_threads; i++) { + ThreadSP thread_sp(new ThreadMachCore(*this, tids[i], i)); new_thread_list.AddThread(thread_sp); } ---------------- `make_shared` (avoid "naked new") Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158785/new/ https://reviews.llvm.org/D158785 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits