zturner added inline comments.
> MinidumpParser.cpp:81-82
>
> - return llvm::ArrayRef<uint8_t>(m_data_sp->GetBytes() + iter->second.rva,
> + return llvm::ArrayRef<uint8_t>(GetData().data() + iter->second.rva,
> iter->second.data_size);
> }
Change this line to `return GetData().slice(iter->second.rva,
iter->second.data_size);`
> MinidumpParser.cpp:106-107
> + return llvm::None;
> + return {GetData().data() + td.thread_context.rva,
> + td.thread_context.data_size};
> +}
Same as above, use `slice()`
> MinidumpParser.cpp:255
> + range_start,
> + llvm::ArrayRef<uint8_t>(GetData().data() + loc_desc.rva,
> range_size));
> + }
`slice()`
> MinidumpParser.h:35-36
> +struct Range {
> + lldb::addr_t start; // virtual address of the beginning of the range
> + // absolute pointer to the first byte of the range and size
> + llvm::ArrayRef<uint8_t> range_ref;
If the comment is long enough to wrap, maybe better to just put it on the line
before. Looks awkward this way.
> MinidumpParser.h:39-40
> +
> + Range(lldb::addr_t start_, llvm::ArrayRef<uint8_t> range_ref_)
> + : start(start_), range_ref(range_ref_) {}
> +};
You don't need the underscores here. It might look awkward, but the usual LLVM
pattern is to just call the constructor parameters and member variables the
same name.
> MinidumpTypes.cpp:188-190
> + return llvm::ArrayRef<MinidumpMemoryDescriptor>(
> + reinterpret_cast<const MinidumpMemoryDescriptor *>(data.data()),
> + *mem_ranges_count);
you can write `return llvm::makeArrayRef(reinterpret_cast<const
MinidumpMemoryDescriptor*>(data.data()), *mem_ranges_count));` to avoid
specifying the type name twice. It's a little shorter (admittedly not much
though).
https://reviews.llvm.org/D25196
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
