jhenderson added inline comments.
================
Comment at: lib/Object/Minidump.cpp:58
+MinidumpFile::getMemoryInfoList() const {
+ auto OptionalStream = getRawStream(StreamType::MemoryInfoList);
+ if (!OptionalStream)
----------------
labath wrote:
> jhenderson wrote:
> > I probably should have picked up on this in previous reviews, but this is
> > too much `auto` for my liking, as it's not obvious from the call site what
> > `getRawStream` returns.
> Done. I've also changed the other calls to getRawStream.
Thanks!
================
Comment at: unittests/Object/MinidumpTest.cpp:620
+ };
+ EXPECT_THAT_EXPECTED(cantFail(create(HeaderTooBig))->getMemoryInfoList(),
+ Failed<BinaryError>());
----------------
Here and in the similar places, I'm not convinced that `cantFail` is
appropriate (if the creation code is broken, this will assert and therefore
possibly hide the actual testing failures that show where it went wrong more
precisely). It should probably be a two phase thing:
```
Expected<std::unique_ptr<MinidumpFile>> Minidump = HeaderTooBig);
ASSERT_THAT_EXPECTED(Minidump, Succeeded());
EXPECTE_THAT_EXPECTED(Minidump->getMemoryInfoList(), Failed<BinaryError>());
```
================
Comment at: unittests/Object/MinidumpTest.cpp:624
+ // Header fits into the stream, but it is too small to contain the required
+ // entries).
+ std::vector<uint8_t> HeaderTooSmall{
----------------
Nit: delete the ')'
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68210/new/
https://reviews.llvm.org/D68210
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
