[PATCH] D18216: [Support] Creation of minidump after compiler crash on Windows
Aaron Ballman via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 28 06:47:58 PDT 2016
aaron.ballman added inline comments.
================
Comment at: include/llvm/Support/Process.h:73
@@ +72,3 @@
+ /// \brief true if PreventCoreFiles has been called, false otherwise.
+ static bool IsCoreFilesPrevented();
+
----------------
Perhaps: `AreCoreFilesPrevented()` to be a bit more grammatically correct?
================
Comment at: lib/Support/Windows/Signals.inc:124
@@ -119,1 +123,3 @@
+typedef BOOL(WINAPI *fpMiniDumpWriteDump)(HANDLE, DWORD, HANDLE, MINIDUMP_TYPE,
+ PMINIDUMP_EXCEPTION_INFORMATION,
----------------
Are we still supporting 32-bit MinGW so that we need to keep doing this dance? :-(
================
Comment at: lib/Support/Windows/Signals.inc:585
@@ +584,3 @@
+static HKEY FindWERKey(const llvm::Twine &RegistryLocation) {
+ HKEY Key = NULL;
+ if (ERROR_SUCCESS != ::RegOpenKeyEx(HKEY_LOCAL_MACHINE,
----------------
No need to initialize this.
================
Comment at: lib/Support/Windows/Signals.inc:611
@@ +610,3 @@
+
+ SmallVector<wchar_t, MAX_PATH> Buffer(BufferLengthBytes);
+
----------------
This doesn't qualify as a "small" vector; might as well just use std::vector.
================
Comment at: lib/Support/Windows/Signals.inc:622
@@ +621,3 @@
+
+ SmallVector<wchar_t, MAX_PATH> ExpandBuffer(ExpandBufferSize);
+
----------------
Same here.
================
Comment at: lib/Support/Windows/Signals.inc:652
@@ +651,3 @@
+
+ DWORD DumpType = 1;
+ DWORD TypeSize = sizeof(DumpType);
----------------
No need to initialize this.
================
Comment at: lib/Support/Windows/Signals.inc:696
@@ +695,3 @@
+ if (MainExecutableName.empty())
+ ProgramName = "clang.EXE";
+ else
----------------
I don't think this is reasonable to assume since the crash dump functionality could be called from any executable in the LLVM project. I would say that if we can't get the executable filename, things are in worse shape than we realize and we should just bail out.
================
Comment at: lib/Support/Windows/WindowsSupport.h:175
@@ +174,3 @@
+ static handle_type GetInvalid() {
+ return 0;
+ }
----------------
s/0/NULL
Repository:
rL LLVM
http://reviews.llvm.org/D18216
More information about the llvm-commits
mailing list