jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.
Aside from a couple of nits, LGTM.
================
Comment at: lib/Object/Minidump.cpp:69
size_t ListOffset = 4;
- // Some producers insert additional padding bytes to align the module list to
- // 8-byte boundary. Check for that by comparing the module list size with the
- // overall stream size.
- if (ListOffset + sizeof(Module) * ListSize < OptionalStream->size())
+ // Some producers insert additional padding bytes to align the list to 8-byte
+ // boundary. Check for that by comparing the list size with the overall
stream
----------------
Nit (was there before): to 8-byte -> to an 8-byte
================
Comment at: unittests/Object/MinidumpTest.cpp:446
+
+ for (const std::vector<uint8_t> &Data : {OneThread, PaddedThread}) {
+ auto ExpectedFile = create(Data);
----------------
I missed this in the other tests, but this could be an `ArrayRef<uint8_t>`
instead of a `const std::vector<uint8_t> &`. Feel free to leave it as is or to
update it in an NFC throughout this test afterwards, if you would prefer to
leave it to another patch.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61064/new/
https://reviews.llvm.org/D61064
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
