From d25baf7bd3c53a89ddfe0b9a2992cf1967e18187 Mon Sep 17 00:00:00 2001 From: Ted John Date: Sat, 16 Jul 2016 14:12:16 +0100 Subject: [PATCH 1/4] improve assertion messages --- src/core/Console.cpp | 8 ++++++-- src/core/Console.hpp | 1 + src/core/Guard.cpp | 49 ++++++++++++++++++++++++++++++++++++++++---- src/core/Guard.hpp | 24 ++++++++++++++++------ 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/src/core/Console.cpp b/src/core/Console.cpp index d82064022d..5c75d249a0 100644 --- a/src/core/Console.cpp +++ b/src/core/Console.cpp @@ -94,11 +94,15 @@ namespace Console void WriteLine(const utf8 * format, ...) { va_list args; - va_start(args, format); + WriteLine_VA(format, args); + va_end(args); + } + + void WriteLine_VA(const utf8 * format, va_list args) + { vfprintf(stdout, format, args); puts(""); - va_end(args); } } } \ No newline at end of file diff --git a/src/core/Console.hpp b/src/core/Console.hpp index 4f8171235b..9b1d82f310 100644 --- a/src/core/Console.hpp +++ b/src/core/Console.hpp @@ -37,5 +37,6 @@ namespace Console void WriteFormat(const utf8 * format, ...); void WriteLine(); void WriteLine(const utf8 * format, ...); + void WriteLine_VA(const utf8 * format, va_list args); } } diff --git a/src/core/Guard.cpp b/src/core/Guard.cpp index f88d871846..1e2e336dfc 100644 --- a/src/core/Guard.cpp +++ b/src/core/Guard.cpp @@ -15,34 +15,75 @@ #pragma endregion #include +#include #include +#include #include "Console.hpp" #include "Diagnostics.hpp" #include "Guard.hpp" +extern "C" +{ + #include "../openrct2.h" +} + namespace Guard { - void Assert(bool expression, const char * message) + void Assert(bool expression, const char * message, ...) + { + va_list args; + va_start(args, message); + Assert_VA(expression, message, args); + va_end(args); + } + + void Assert_VA(bool expression, const char * message, va_list args) { if (expression) return; if (message != nullptr) { - Console::Error::WriteLine(message); + Console::Error::WriteLine("Assertion failed:"); + Console::Error::WriteLine_VA(message, args); } #if DEBUG Debug::Break(); #endif +#if __WINDOWS__ + char version[128]; + openrct2_write_full_version_info(version, sizeof(version)); + + char buffer[512]; + strcpy(buffer, "An assertion failed, please report this to the OpenRCT2 developers.\r\n\r\nVersion: "); + strcat(buffer, version); + strcat(buffer, "\r\n"); + vsprintf((char *)strchr(buffer, 0), message, args); + int result = MessageBox(nullptr, buffer, OPENRCT2_NAME, MB_ABORTRETRYIGNORE | MB_ICONEXCLAMATION); + if (result == IDABORT) + { + // Force a crash that breakpad will handle allowing us to get a dump + *((void**)0) = 0; + } +#else assert(false); +#endif } - void Fail(const char * message) + void Fail(const char * message, ...) + { + va_list args; + va_start(args, message); + Fail_VA(message, args); + va_end(args); + } + + void Fail_VA(const char * message, va_list args) { if (message != nullptr) { - Console::Error::WriteLine(message); + Console::Error::WriteLine_VA(message, args); } #if DEBUG diff --git a/src/core/Guard.hpp b/src/core/Guard.hpp index c4ba8cdefa..683514e4dd 100644 --- a/src/core/Guard.hpp +++ b/src/core/Guard.hpp @@ -16,23 +16,35 @@ #pragma once +#include + /** * Utility methods for asserting function parameters. */ namespace Guard { - void Assert(bool expression, const char * message = nullptr); - void Fail(const char * message = nullptr); + void Assert(bool expression, const char * message = nullptr, ...); + void Assert_VA(bool expression, const char * message, va_list args); + void Fail(const char * message = nullptr, ...); + void Fail_VA(const char * message, va_list args); template - void ArgumentNotNull(T * argument, const char * message = nullptr) + void ArgumentNotNull(T * argument, const char * message = nullptr, ...) { - Assert(argument != nullptr, message); + va_list args; + va_start(args, message); + Assert_VA(argument != nullptr, message, args); + va_end(args); } template - void ArgumentInRange(T argument, T min, T max, const char * message = nullptr) + void ArgumentInRange(T argument, T min, T max, const char * message = nullptr, ...) { - Assert(argument >= min && argument <= max, message); + va_list args; + va_start(args, message); + Assert(argument >= min && argument <= max, message, args); + va_end(args); } }; + +#define GUARD_LINE "Location: %s:%d", __func__, __LINE__ From 5f41e3a0eb481f399e33a7fddc3ed178c8a0eafc Mon Sep 17 00:00:00 2001 From: Ted John Date: Sat, 16 Jul 2016 14:17:36 +0100 Subject: [PATCH 2/4] add function and line info to guards --- src/core/List.hpp | 8 ++++---- src/drawing/Image.cpp | 8 ++++---- src/interface/Theme.cpp | 2 +- src/object/ObjectFactory.cpp | 4 ++-- src/object/ObjectManager.cpp | 2 +- src/object/ObjectRepository.cpp | 6 +++--- src/object/SceneryGroupObject.cpp | 2 +- src/rct1/tables.cpp | 4 ++-- 8 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/core/List.hpp b/src/core/List.hpp index c3ef781b27..70c1897144 100644 --- a/src/core/List.hpp +++ b/src/core/List.hpp @@ -72,7 +72,7 @@ public: void Insert(T item, size_t index) { - Guard::ArgumentInRange(index, (size_t)0, this->size()); + Guard::ArgumentInRange(index, (size_t)0, this->size(), GUARD_LINE); this->insert(this->begin() + index, item); } @@ -91,7 +91,7 @@ public: void RemoveAt(size_t index) { - Guard::ArgumentInRange(index, (size_t)0, this->size() - 1); + Guard::ArgumentInRange(index, (size_t)0, this->size() - 1, GUARD_LINE); this->erase(this->begin() + index); } @@ -102,13 +102,13 @@ public: const_reference operator[](size_t index) const { - Guard::ArgumentInRange(index, (size_t)0, this->size() - 1); + Guard::ArgumentInRange(index, (size_t)0, this->size() - 1, GUARD_LINE); return std::vector::operator[](index); } reference operator[](size_t index) { - Guard::ArgumentInRange(index, (size_t)0, this->size() - 1); + Guard::ArgumentInRange(index, (size_t)0, this->size() - 1, GUARD_LINE); return std::vector::operator[](index); } diff --git a/src/drawing/Image.cpp b/src/drawing/Image.cpp index 0fab47c9b9..734509821a 100644 --- a/src/drawing/Image.cpp +++ b/src/drawing/Image.cpp @@ -56,7 +56,7 @@ static bool AllocatedListContains(uint32 baseImageId, uint32 count) static uint32 AllocateImageList(uint32 count) { - Guard::Assert(count != 0); + Guard::Assert(count == 0, GUARD_LINE); if (!_initialised) { @@ -92,12 +92,12 @@ static uint32 AllocateImageList(uint32 count) static void FreeImageList(uint32 baseImageId, uint32 count) { - Guard::Assert(_initialised); - Guard::Assert(baseImageId >= BASE_IMAGE_ID); + Guard::Assert(_initialised, GUARD_LINE); + Guard::Assert(baseImageId >= BASE_IMAGE_ID, GUARD_LINE); #ifdef DEBUG bool contains = AllocatedListContains(baseImageId, count); - Guard::Assert(contains); + Guard::Assert(contains, GUARD_LINE); #endif // TODO validate that this was an allocated list diff --git a/src/interface/Theme.cpp b/src/interface/Theme.cpp index 99949916d8..77bedbad39 100644 --- a/src/interface/Theme.cpp +++ b/src/interface/Theme.cpp @@ -510,7 +510,7 @@ namespace ThemeManager static void GetAvailableThemes(List * outThemes) { - Guard::ArgumentNotNull(outThemes); + Guard::ArgumentNotNull(outThemes, GUARD_LINE); outThemes->Clear(); diff --git a/src/object/ObjectFactory.cpp b/src/object/ObjectFactory.cpp index d4486d4b70..9ab31adf6f 100644 --- a/src/object/ObjectFactory.cpp +++ b/src/object/ObjectFactory.cpp @@ -166,8 +166,8 @@ namespace ObjectFactory Object * CreateObjectFromLegacyData(const rct_object_entry * entry, const void * data, size_t dataSize) { - Guard::ArgumentNotNull(entry); - Guard::ArgumentNotNull(data); + Guard::ArgumentNotNull(entry, GUARD_LINE); + Guard::ArgumentNotNull(data, GUARD_LINE); Object * result = CreateObject(*entry); if (result != nullptr) diff --git a/src/object/ObjectManager.cpp b/src/object/ObjectManager.cpp index 70b18484a9..a0f6c8dfaa 100644 --- a/src/object/ObjectManager.cpp +++ b/src/object/ObjectManager.cpp @@ -225,7 +225,7 @@ private: size_t GetLoadedObjectIndex(const Object * object) { - Guard::ArgumentNotNull(object); + Guard::ArgumentNotNull(object, GUARD_LINE); size_t result = SIZE_MAX; if (_loadedObjects != nullptr) diff --git a/src/object/ObjectRepository.cpp b/src/object/ObjectRepository.cpp index 1aff117211..c183709cdc 100644 --- a/src/object/ObjectRepository.cpp +++ b/src/object/ObjectRepository.cpp @@ -167,7 +167,7 @@ public: Object * LoadObject(const ObjectRepositoryItem * ori) override { - Guard::ArgumentNotNull(ori); + Guard::ArgumentNotNull(ori, GUARD_LINE); Object * object = ObjectFactory::CreateObjectFromLegacyFile(ori->Path); return object; @@ -177,7 +177,7 @@ public: { ObjectRepositoryItem * item = &_items[ori->Id]; - Guard::Assert(item->LoadedObject == nullptr); + Guard::Assert(item->LoadedObject == nullptr, GUARD_LINE); item->LoadedObject = object; } @@ -518,7 +518,7 @@ private: int newRealChecksum = object_calculate_checksum(entry, newData, newDataSize); if (newRealChecksum != entry->checksum) { - Guard::Fail("CalculateExtraBytesToFixChecksum failed to fix checksum."); + Guard::Fail("CalculateExtraBytesToFixChecksum failed to fix checksum.", GUARD_LINE); // Save old data form SaveObject(path, entry, data, dataSize, false); diff --git a/src/object/SceneryGroupObject.cpp b/src/object/SceneryGroupObject.cpp index a0120220dc..c7831e7a0d 100644 --- a/src/object/SceneryGroupObject.cpp +++ b/src/object/SceneryGroupObject.cpp @@ -94,7 +94,7 @@ void SceneryGroupObject::UpdateEntryIndexes() if (ori->LoadedObject == nullptr) continue; uint16 sceneryEntry = objectManager->GetLoadedObjectEntryIndex(ori->LoadedObject); - Guard::Assert(sceneryEntry != UINT8_MAX); + Guard::Assert(sceneryEntry != UINT8_MAX, GUARD_LINE); uint8 objectType = objectEntry->flags & 0x0F; switch (objectType) { diff --git a/src/rct1/tables.cpp b/src/rct1/tables.cpp index 6fb2b865e8..7534d34716 100644 --- a/src/rct1/tables.cpp +++ b/src/rct1/tables.cpp @@ -360,7 +360,7 @@ namespace RCT1 "LEMST ", // RCT1_RIDE_TYPE_LEMONADE_STALL }; - Guard::ArgumentInRange(rideType, 0, Util::CountOf(map), ""); + Guard::ArgumentInRange(rideType, 0, Util::CountOf(map), "Unsupported RCT1 ride type."); return map[rideType]; } @@ -459,7 +459,7 @@ namespace RCT1 "ENTERP ", // RCT1_VEHICLE_TYPE_ENTERPRISE_WHEEL }; - Guard::ArgumentInRange(vehicleType, 0, Util::CountOf(map), ""); + Guard::ArgumentInRange(vehicleType, 0, Util::CountOf(map), "Unsupported RCT1 vehicle type."); return map[vehicleType]; } From bd3331df4f0d3dc89214d92693018d15e759d2b5 Mon Sep 17 00:00:00 2001 From: Ted John Date: Sat, 16 Jul 2016 14:34:10 +0100 Subject: [PATCH 3/4] call assert() if not using breakpad --- src/core/Guard.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/Guard.cpp b/src/core/Guard.cpp index 1e2e336dfc..0179caf63d 100644 --- a/src/core/Guard.cpp +++ b/src/core/Guard.cpp @@ -48,10 +48,10 @@ namespace Guard Console::Error::WriteLine_VA(message, args); } -#if DEBUG +#ifdef DEBUG Debug::Break(); #endif -#if __WINDOWS__ +#ifdef __WINDOWS__ char version[128]; openrct2_write_full_version_info(version, sizeof(version)); @@ -63,8 +63,12 @@ namespace Guard int result = MessageBox(nullptr, buffer, OPENRCT2_NAME, MB_ABORTRETRYIGNORE | MB_ICONEXCLAMATION); if (result == IDABORT) { +#ifdef USE_BREAKPAD // Force a crash that breakpad will handle allowing us to get a dump *((void**)0) = 0; +#else + assert(false); +#endif } #else assert(false); From c413c43123cd652e896270f4b974e623592f20f1 Mon Sep 17 00:00:00 2001 From: Ted John Date: Sat, 16 Jul 2016 14:49:41 +0100 Subject: [PATCH 4/4] only include windows.h on windows --- src/core/Guard.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/core/Guard.cpp b/src/core/Guard.cpp index 0179caf63d..4a775b0c2f 100644 --- a/src/core/Guard.cpp +++ b/src/core/Guard.cpp @@ -17,7 +17,11 @@ #include #include #include + +#ifdef __WINDOWS__ +#define WIN32_LEAN_AND_MEAN #include +#endif #include "Console.hpp" #include "Diagnostics.hpp"