labath added a comment.
Just a couple more details and I think we're ready.
> MinidumpParser.cpp:105
> +MinidumpParser::GetThreadContext(const MinidumpThread &td) {
> + return td.GetContext(GetData().data());
> +}
I think you have made it over-encapsulated now. :)
Either a Parser function which takes a thread or a Thread function which takes
a parser as argument should be enough. Is there a reason you need both?
> MinidumpParser.cpp:234
> +llvm::Optional<Range> MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
> + Range range_out;
> + llvm::ArrayRef<uint8_t> data = GetStream(MinidumpStreamType::MemoryList);
Not necessary.
> MinidumpParser.cpp:255
> + llvm::ArrayRef<uint8_t>(GetData().data() + loc_desc.rva,
> range_size);
> + return range_out;
> + }
`return Range(...)` is much cleaner and shorter.
Add an appropriate constructor if it is needed to make this work.
> MinidumpTypes.cpp:94
> +MinidumpThread::GetContext(const uint8_t *base_ptr) const {
> + return {base_ptr + thread_context.rva, thread_context.data_size};
> +}
This feels very unsafe. Is there anything guaranteeing that
`thread_context.rva` does not point beyond the end of the file?
> ProcessMinidump.cpp:69
> + // work-in-progress
> + if (minidump_parser &&
> + minidump_parser->GetArchitecture().GetTriple().getOS() !=
This could also be an early-return.
https://reviews.llvm.org/D25196
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
