zturner added inline comments.
================ Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:242 + + for (const auto &module : modules) { + name = GetMinidumpString(module.module_name_rva); ---------------- I don't know how big the minidumps you're working with are or if performance is a concern, but `std::map<>` is more or less a piece of junk. The only time it should really be used is if you need a deterministic iteration order. `std::unordered_map<>` is slightly better. Better still is `llvm::StringMap<>` ================ Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:281 + + if (data.size() == 0 && data64.size() == 0) + return llvm::None; ---------------- `data.empty()`? ================ Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:284 + + if (data.size() > 0) { + llvm::ArrayRef<MinidumpMemoryDescriptor> memory_list = ---------------- `!data.empty()`? ================ Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:291 + + for (auto memory_desc : memory_list) { + const MinidumpLocationDescriptor &loc_desc = memory_desc.memory; ---------------- `auto &` to avoid a copy. ================ Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:320 + + for (auto memory_desc64 : memory64_list) { + const lldb::addr_t range_start = memory_desc64.start_of_memory_range; ---------------- `auto &` ================ Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:222 + + if (header->size_of_header > sizeof(MinidumpMemoryInfoListHeader)) { + data = data.drop_front(header->size_of_header - ---------------- I don't think you need the conditional here do you? Just `data = data.drop_front(sizeof_MinidumpMemoryInfoListHeader)`. ================ Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:268 + MemFree = 0x10000, + MemReserve = 0x2000, +}; ---------------- Can you use `llvm/ADT/BitmaskEnum.h` here to give this enum bitwise operators? ================ Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:274 + MemMapped = 0x40000, + MemPrivate = 0x20000, +}; ---------------- Same ================ Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:294 + PageExecutable = PageExecute | PageExecuteRead | PageExecuteReadWrite | + PageExecuteWriteCopy, +}; ---------------- Same https://reviews.llvm.org/D25569 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits