[Lldb-commits] [PATCH] D68657: Update MinidumpYAML to use minidump::Exception for exception stream

[George Rimar via Phabricator via lldb-commits]

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