amccarth added a comment.
Are we putting this code in the right place? I wouldn't expect minidump
parsing to fall under Plugins/Process.
I assume the eventual intent is to turn the Windows-specific code into a user
of your new code? I look forward to seeing that happen.
================
Comment at: source/Plugins/Process/minidump/MinidumpTypes.cpp:13
@@ +12,3 @@
+#include "MinidumpParser.h"
+#include "MinidumpTypes.h"
+
----------------
Include MinidumpTypes.h first in MinidumpTypes.cpp. This helps ensure that
headers can stand alone.
================
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:14
@@ +13,3 @@
+// C includes
+#include <cstring>
+
----------------
Maybe I'm missing it, but it doesn't seem like this header is using anything
from `<cstring>` (which should be under C++ includes).
================
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:40
@@ +39,3 @@
+
+enum MinidumpStreamType
+{
----------------
// Reference:
https://msdn.microsoft.com/en-us/library/windows/desktop/ms680394.aspx
================
Comment at: source/Plugins/Process/minidump/MinidumpTypes.h:257
@@ +256,3 @@
+};
+const int MINIDUMP_SYSTEM_INFO_SIZE = 3 * 2 + 2 * 1 + 4 * 4 + sizeof(RVA) + 2
* 2 + MINIDUMP_CPU_INFO_SIZE;
+static_assert(sizeof(MinidumpSystemInfo) == MINIDUMP_SYSTEM_INFO_SIZE, "sizeof
MinidumpSystemInfo is not correct!");
----------------
Why do the arithmetic and static_assert? Why not use
`sizeof(MinidumpSystemInfo)`?
https://reviews.llvm.org/D23545
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
