Address review feedback

Author: slurmlord

Core/GameEngine/CMakeLists.txt

@@ -1173,6 +1173,7 @@ target_include_directories(corei_gameengine_private INTERFACE
 
 target_link_libraries(corei_gameengine_private INTERFACE
     corei_always
+    $<$<AND:$<BOOL:${IS_VS6_BUILD}>,$<BOOL:${RTS_CRASHDUMP_ENABLE}>>:shlwapi.lib>
 )
 
 target_compile_definitions(corei_gameengine_private INTERFACE

Core/GameEngine/Include/Common/GameMemory.h

@@ -503,14 +503,17 @@ enum { MAX_SPECIAL_USED = 256 };
 #endif
 
 #ifdef RTS_ENABLE_CRASHDUMP
-class AllocationRangeIterator {
+class AllocationRangeIterator
+{
 	typedef const MemoryPoolAllocatedRange value_type;
 	typedef const MemoryPoolAllocatedRange* pointer;
 	typedef const MemoryPoolAllocatedRange& reference;
+
 public:
 
 	AllocationRangeIterator(const MemoryPoolFactory* factory);
-	AllocationRangeIterator(MemoryPool& pool, MemoryPoolBlob& blob) {
+	AllocationRangeIterator(MemoryPool& pool, MemoryPoolBlob& blob)
+	{
 		m_currentPool = &pool;
 		m_currentBlobInPool = &blob;
 		m_factory = NULL;
@@ -542,8 +545,15 @@ class AllocationRangeIterator {
 	// Postfix increment
 	AllocationRangeIterator operator++(int) { AllocationRangeIterator tmp = *this; ++(*this); return tmp; }
 
-	friend bool operator== (const AllocationRangeIterator& a, const AllocationRangeIterator& b) { return a.m_currentBlobInPool == b.m_currentBlobInPool; };
-	friend bool operator!= (const AllocationRangeIterator& a, const AllocationRangeIterator& b) { return a.m_currentBlobInPool != b.m_currentBlobInPool; };
+	friend const bool operator== (const AllocationRangeIterator& a, const AllocationRangeIterator& b)
+	{
+		return a.m_currentBlobInPool == b.m_currentBlobInPool;
+	};
+
+	friend const bool operator!= (const AllocationRangeIterator& a, const AllocationRangeIterator& b)
+	{
+		return a.m_currentBlobInPool != b.m_currentBlobInPool;
+	};
 
 private:
 	const MemoryPoolFactory* m_factory;
@@ -648,11 +658,13 @@ class MemoryPoolFactory
 
 	#endif
 #ifdef RTS_ENABLE_CRASHDUMP
-		AllocationRangeIterator cbegin() const {
+		AllocationRangeIterator cbegin() const
+		{
 			return AllocationRangeIterator(this);
 		}
 
-		AllocationRangeIterator cend() const {
+		AllocationRangeIterator cend() const
+		{
 			return AllocationRangeIterator(NULL, NULL);
 		}
 

Core/GameEngine/Include/Common/MiniDumper.h

@@ -22,38 +22,42 @@
 #include <imagehlp.h>
 #include "Common/MiniDumper_compat.h"
 
+enum DumpType CPP_11(: Int)
+{
+	// Smallest dump type with call stacks and some supporting variables
+	DUMP_TYPE_MINIMAL,
+	// Large dump including all memory regions allocated by the GameMemory implementaion
+	DUMP_TYPE_GAMEMEMORY,
+	// Largest dump size including complete memory contents of the process
+	DUMP_TYPE_FULL,
+};
+
+enum MiniDumperExitCode CPP_11(: Int)
+{
+	DUMPER_EXIT_SUCCESS = 0x0,
+	DUMPER_EXIT_FAILURE_WAIT = 0x37DA1040,
+	DUMPER_EXIT_FAILURE_PARAM = 0x4EA527BB,
+	DUMPER_EXIT_FORCED_TERMINATE = 0x158B1154,
+};
+
 class MiniDumper
 {
 public:
-	MiniDumper()
-	{
-		m_miniDumpInitialized = false;
-		m_extendedInfoRequested = false;
-		m_dbgHlp = NULL;
-		m_pMiniDumpWriteDump = NULL;
-		m_dumpRequested = NULL;
-		m_dumpComplete = NULL;
-		m_quitting = NULL;
-		m_dumpThread = NULL;
-		m_dumpThreadId = 0;
-		m_dumpObjectsState = 0;
-		m_dumpObjectsSubState = 0;
-		m_dmaRawBlockIndex = 0;
-		memset(m_dumpDir, 0, ARRAY_SIZE(m_dumpDir));
-		memset(m_dumpFile, 0, ARRAY_SIZE(m_dumpFile));
-		memset(m_sysDbgHelpPath, 0, ARRAY_SIZE(m_sysDbgHelpPath));
-	};
-
-	void Initialize(const AsciiString& userDirPath);
+	MiniDumper();
 	Bool IsInitialized() const;
-	void TriggerMiniDump(Bool extendedInfo = false);
-	void TriggerMiniDumpForException(struct _EXCEPTION_POINTERS* e_info, Bool extendedInfo = false);
-	void ShutDown();
+	void TriggerMiniDump(DumpType dumpType);
+	void TriggerMiniDumpForException(struct _EXCEPTION_POINTERS* e_info, DumpType dumpType);
+	static void initMiniDumper(const AsciiString& userDirPath);
+	static void shutdownMiniDumper();
 	static LONG WINAPI DumpingExceptionFilter(struct _EXCEPTION_POINTERS* e_info);
+
 private:
-	void CreateMiniDump(Bool extendedInfo);
+	void Initialize(const AsciiString& userDirPath);
+	void ShutDown();
+	void CreateMiniDump(DumpType dumpType);
 	BOOL DumpMemoryObjects(ULONG64& memoryBase, ULONG& memorySize);
 	void CleanupResources();
+	Bool IsDumpThreadStillRunning() const;
 
 	// Callbacks from dbghelp
 	static BOOL CALLBACK MiniDumpCallback(PVOID CallbackParam, PMINIDUMP_CALLBACK_INPUT CallbackInput, PMINIDUMP_CALLBACK_OUTPUT CallbackOutput);
@@ -68,7 +72,8 @@ class MiniDumper
 	static void KeepNewestFiles(const std::string& directory, const std::string& fileWildcard, const Int keepCount);
 
 	// Struct to hold file information
-	struct FileInfo {
+	struct FileInfo
+	{
 		std::string name;
 		FILETIME lastWriteTime;
 	};
@@ -77,12 +82,13 @@ class MiniDumper
 
 private:
 	Bool m_miniDumpInitialized;
-	Bool m_extendedInfoRequested;
+	DumpType m_requestedDumpType;
 
 	// Path buffers
 	Char m_dumpDir[MAX_PATH];
 	Char m_dumpFile[MAX_PATH];
 	Char m_sysDbgHelpPath[MAX_PATH];
+	WideChar m_executablePath[MAX_PATH];
 
 	// Module handles
 	HMODULE m_dbgHlp;
@@ -101,8 +107,7 @@ class MiniDumper
 	int m_dumpObjectsSubState;
 	int m_dmaRawBlockIndex;
 
-	AllocationRangeIterator m_RangeIter;
-	AllocationRangeIterator m_endRangeIter;
+	AllocationRangeIterator m_rangeIter;
 
 	// Function pointer to MiniDumpWriteDump in dbghelp.dll
 	typedef BOOL(WINAPI* MiniDumpWriteDump_t)(
@@ -117,4 +122,6 @@ class MiniDumper
 
 	MiniDumpWriteDump_t m_pMiniDumpWriteDump;
 };
+
+extern MiniDumper* TheMiniDumper;
 #endif

Core/GameEngine/Source/Common/System/Debug.cpp

@@ -72,8 +72,6 @@
 #endif
 #ifdef RTS_ENABLE_CRASHDUMP
 #include "Common/MiniDumper.h"
-
-MiniDumper TheMiniDumper = MiniDumper();
 #endif
 
 // Horrible reference, but we really, really need to know if we are windowed.
@@ -735,12 +733,12 @@ void ReleaseCrash(const char *reason)
 	}
 
 #ifdef RTS_ENABLE_CRASHDUMP
-	if (TheMiniDumper.IsInitialized())
+	if (TheMiniDumper && TheMiniDumper->IsInitialized())
 	{
 		// Do dumps both with and without extended info
-		TheMiniDumper.TriggerMiniDump(false);
-		TheMiniDumper.TriggerMiniDump(true);
-		TheMiniDumper.ShutDown();
+		TheMiniDumper->TriggerMiniDump(DUMP_TYPE_MINIMAL);
+		TheMiniDumper->TriggerMiniDump(DUMP_TYPE_GAMEMEMORY);
+		MiniDumper::shutdownMiniDumper();
 	}
 #endif
 
@@ -810,12 +808,12 @@ void ReleaseCrashLocalized(const AsciiString& p, const AsciiString& m)
 	}
 
 #ifdef RTS_ENABLE_CRASHDUMP
-	if (TheMiniDumper.IsInitialized())
+	if (TheMiniDumper && TheMiniDumper->IsInitialized())
 	{
 		// Do dumps both with and without extended info
-		TheMiniDumper.TriggerMiniDump(false);
-		TheMiniDumper.TriggerMiniDump(true);
-		TheMiniDumper.ShutDown();
+		TheMiniDumper->TriggerMiniDump(DUMP_TYPE_MINIMAL);
+		TheMiniDumper->TriggerMiniDump(DUMP_TYPE_GAMEMEMORY);
+		MiniDumper::shutdownMiniDumper();
 	}
 #endif
 

Core/GameEngine/Source/Common/System/GameMemory.cpp

@@ -2603,7 +2603,6 @@ Int DynamicMemoryAllocator::getRawBlockCount() const
 }
 void DynamicMemoryAllocator::fillAllocationRangeForRawBlockN(const Int n, MemoryPoolAllocatedRange& allocationRange) const
 {
-
 	MemoryPoolSingleBlock* block = m_rawBlocks;
 	for (int i = 0; i < n; ++i)
 	{