grimar added inline comments.
================ Comment at: llvm/include/llvm/ObjectYAML/MinidumpYAML.h:113-114 + ExceptionStream() + : Stream(StreamKind::Exception, minidump::StreamType::Exception) { + memset(&MDExceptionStream, 0, sizeof(minidump::ExceptionStream)); + } ---------------- I'd avoid memset: ``` ExceptionStream() : Stream(StreamKind::Exception, minidump::StreamType::Exception), MDExceptionStream({}) { ``` ================ Comment at: llvm/lib/ObjectYAML/MinidumpYAML.cpp:525 + case StreamKind::Exception: { + auto ExpectedExceptionStream = File.getExceptionStream(); + if (!ExpectedExceptionStream) ---------------- We often avoid using `auto` when return type is not obvious. ================ Comment at: llvm/unittests/ObjectYAML/MinidumpYAMLTest.cpp:143 + +TEST(MinidumpYAML, ExceptionStream) { + SmallString<0> Storage; ---------------- I'd add a comment for each test to describe what they do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68657/new/ https://reviews.llvm.org/D68657 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits