From 1d3f3cf8b35bb433efb0c526111279e8bcd6b5d8 Mon Sep 17 00:00:00 2001 From: Jarod42 Date: Fri, 6 Oct 2023 11:32:23 +0200 Subject: [PATCH 1/2] Use `std::vector Names` in `GetAVolumePath` to avoid manual memory handle. And clean up `getVolumes()`. --- src/include/stratagus.h | 5 ++ src/stratagus/script.cpp | 153 +++++++++++++++------------------------ 2 files changed, 63 insertions(+), 95 deletions(-) diff --git a/src/include/stratagus.h b/src/include/stratagus.h index 77755a9601..863425e4e9 100644 --- a/src/include/stratagus.h +++ b/src/include/stratagus.h @@ -157,6 +157,11 @@ inline bool starts_with(std::string_view s, std::string_view prefix) return s.substr(0, prefix.size()) == prefix; } +inline bool starts_with(std::wstring_view s, std::wstring_view prefix) +{ + return s.substr(0, prefix.size()) == prefix; +} + /*---------------------------------------------------------------------------- -- General ----------------------------------------------------------------------------*/ diff --git a/src/stratagus/script.cpp b/src/stratagus/script.cpp index b44662d0f4..0776dbb97f 100644 --- a/src/stratagus/script.cpp +++ b/src/stratagus/script.cpp @@ -2346,122 +2346,85 @@ void LoadCcl(const fs::path &filename, const std::string &luaArgStr) } #ifdef WIN32 -// copied from msdn fs::path GetAVolumePath(__in PWCHAR VolumeName) { - DWORD CharCount = MAX_PATH + 1; - PWCHAR Names = nullptr; - PWCHAR NameIdx = nullptr; - BOOL Success = FALSE; + DWORD CharCount = MAX_PATH + 1; + std::vector Names(CharCount); - for (;;) { - Names = (PWCHAR) new BYTE [CharCount * sizeof(WCHAR)]; - - if (!Names) { - ExitFatal(1); - } - - Success = GetVolumePathNamesForVolumeNameW(VolumeName, Names, CharCount, &CharCount); - if (Success) { - break; - } - - if (GetLastError() != ERROR_MORE_DATA) { - break; - } - - delete [] Names; - Names = nullptr; - } - - for (NameIdx = Names; NameIdx[0] != L'\0'; NameIdx += wcslen(NameIdx) + 1) { - fs::path result(NameIdx); - delete [] Names; - Names = nullptr; - return result; + for (;;) { + const BOOL Success = + GetVolumePathNamesForVolumeNameW(VolumeName, Names.data(), Names.size(), &CharCount); + Names.resize(CharCount); + if (Success || GetLastError() != ERROR_MORE_DATA) { + break; + } } - - return fs::path(); + return Names.data(); } std::vector getVolumes() { - DWORD CharCount = 0; - WCHAR DeviceName[MAX_PATH] = L""; - DWORD Error = ERROR_SUCCESS; - HANDLE FindHandle = INVALID_HANDLE_VALUE; - BOOL Found = FALSE; - size_t Index = 0; - BOOL Success = FALSE; - WCHAR VolumeName[MAX_PATH] = L""; + WCHAR VolumeName[MAX_PATH] = L""; + + // Enumerate all volumes in the system. + HANDLE FindHandle = FindFirstVolumeW(VolumeName, std::size(VolumeName)); + + if (FindHandle == INVALID_HANDLE_VALUE) { + const DWORD Error = GetLastError(); + wprintf(L"FindFirstVolumeW failed with error code %d\n", Error); + ExitFatal(1); + } - int curIndex = 0; std::vector result; + for (;;) { + size_t Index = wcslen(VolumeName) - 1; + + wprintf(L"Volume name: %s\n", VolumeName); + if (!starts_with(VolumeName, LR"(\\?\)") || VolumeName[Index] != L'\\') { + wprintf(L"FindFirstVolumeW/FindNextVolumeW returned a bad path: %s\n", VolumeName); + ExitFatal(1); + } + + // Skip the \\?\ prefix and remove the trailing backslash. + // QueryDosDeviceW does not allow a trailing backslash, + // so temporarily remove it. + VolumeName[Index] = L'\0'; + WCHAR DeviceName[MAX_PATH] = L""; + DWORD CharCount = QueryDosDeviceW(&VolumeName[4], DeviceName, std::size(DeviceName)); + VolumeName[Index] = L'\\'; + + if (CharCount == 0) { + const DWORD Error = GetLastError(); + wprintf(L"QueryDosDeviceW failed with error code %d\n", Error); + ExitFatal(1); + } - // Enumerate all volumes in the system. - FindHandle = FindFirstVolumeW(VolumeName, ARRAYSIZE(VolumeName)); - - if (FindHandle == INVALID_HANDLE_VALUE) { - Error = GetLastError(); - wprintf(L"FindFirstVolumeW failed with error code %d\n", Error); - ExitFatal(1); - } - - for (;;) { - // Skip the \\?\ prefix and remove the trailing backslash. - Index = wcslen(VolumeName) - 1; - - if (VolumeName[0] != L'\\' || - VolumeName[1] != L'\\' || - VolumeName[2] != L'?' || - VolumeName[3] != L'\\' || - VolumeName[Index] != L'\\') { - Error = ERROR_BAD_PATHNAME; - wprintf(L"FindFirstVolumeW/FindNextVolumeW returned a bad path: %s\n", VolumeName); - ExitFatal(1); - } - - // QueryDosDeviceW does not allow a trailing backslash, - // so temporarily remove it. - VolumeName[Index] = L'\0'; - CharCount = QueryDosDeviceW(&VolumeName[4], DeviceName, ARRAYSIZE(DeviceName)); - VolumeName[Index] = L'\\'; - - if (CharCount == 0) { - Error = GetLastError(); - wprintf(L"QueryDosDeviceW failed with error code %d\n", Error); - ExitFatal(1); - } - - wprintf(L"\nFound a device:\n %s", DeviceName); - wprintf(L"\nVolume name: %s", VolumeName); + wprintf(L" Found a device: %s\n", DeviceName); fs::path r = GetAVolumePath(VolumeName); if (!r.empty()) { result.push_back(r); - wprintf(L"\nPath: %s", r.wstring().c_str()); + wprintf(L" Path: %s\n", r.wstring().c_str()); } - // Move on to the next volume. - Success = FindNextVolumeW(FindHandle, VolumeName, ARRAYSIZE(VolumeName)); + // Move on to the next volume. + const BOOL Success = FindNextVolumeW(FindHandle, VolumeName, std::size(VolumeName)); - if (!Success) { - Error = GetLastError(); + if (!Success) { + const DWORD Error = GetLastError(); - if (Error != ERROR_NO_MORE_FILES) { - wprintf(L"FindNextVolumeW failed with error code %d\n", Error); - ExitFatal(1); - } + if (Error != ERROR_NO_MORE_FILES) { + wprintf(L"FindNextVolumeW failed with error code %d\n", Error); + ExitFatal(1); + } - // Finished iterating through all the volumes. - Error = ERROR_SUCCESS; - break; - } - } + // Finished iterating through all the volumes. + break; + } + } - FindVolumeClose(FindHandle); - FindHandle = INVALID_HANDLE_VALUE; + FindVolumeClose(FindHandle); - return result; + return result; } #endif From dd7275e5276ca5a045eda7cd14a581742f927b3c Mon Sep 17 00:00:00 2001 From: Jarod42 Date: Fri, 6 Oct 2023 12:14:01 +0200 Subject: [PATCH 2/2] Turn `CUnitStats::Variables` into `std::vector`. --- src/action/action_die.cpp | 2 +- src/action/action_upgradeto.cpp | 2 +- src/include/unit.h | 2 +- src/include/upgrade_structs.h | 4 ++-- src/unit/script_unittype.cpp | 4 ++-- src/unit/unit.cpp | 10 +++++----- src/unit/unittype.cpp | 6 ++---- src/unit/upgrade.cpp | 14 ++------------ 8 files changed, 16 insertions(+), 28 deletions(-) diff --git a/src/action/action_die.cpp b/src/action/action_die.cpp index 93092b8372..2141162e74 100644 --- a/src/action/action_die.cpp +++ b/src/action/action_die.cpp @@ -125,7 +125,7 @@ void COrder_Die::Execute(CUnit &unit) /* override */ unit.Remove(nullptr); unit.Type = &corpseType; - unit.Stats = &corpseType.Stats[unit.Player->Index]; + unit.Stats = const_cast(&corpseType.Stats[unit.Player->Index]); UpdateUnitSightRange(unit); unit.Place(unit.tilePos); diff --git a/src/action/action_upgradeto.cpp b/src/action/action_upgradeto.cpp index bb348771d8..2416bc1b1f 100644 --- a/src/action/action_upgradeto.cpp +++ b/src/action/action_upgradeto.cpp @@ -156,7 +156,7 @@ static int TransformUnitIntoType(CUnit &unit, const CUnitType &newtype) } unit.Type = const_cast(&newtype); - unit.Stats = &unit.Type->Stats[player.Index]; + unit.Stats = const_cast(&unit.Type->Stats[player.Index]); if (!newtype.CanCastSpell.empty() && !unit.AutoCastSpell) { unit.AutoCastSpell = new char[SpellTypeTable.size()]; diff --git a/src/include/unit.h b/src/include/unit.h index 6592b68931..923079627d 100644 --- a/src/include/unit.h +++ b/src/include/unit.h @@ -342,7 +342,7 @@ class CUnit const CUnitType *Type = nullptr; /// Pointer to unit-type (peon,...) CPlayer *Player = nullptr; /// Owner of this unit - const CUnitStats *Stats = nullptr; /// Current unit stats + CUnitStats *Stats = nullptr; /// Current unit stats int CurrentSightRange; /// Unit's Current Sight Range // Pathfinding stuff: diff --git a/src/include/upgrade_structs.h b/src/include/upgrade_structs.h index 47edfc8392..a245441014 100644 --- a/src/include/upgrade_structs.h +++ b/src/include/upgrade_structs.h @@ -110,14 +110,14 @@ class CUnitStats { public: CUnitStats() = default; - ~CUnitStats(); + ~CUnitStats() = default; CUnitStats &operator=(const CUnitStats &rhs); bool operator == (const CUnitStats &rhs) const; bool operator != (const CUnitStats &rhs) const; public: - CVariable *Variables = nullptr; /// user defined variable. + std::vector Variables; /// user defined variable. int Costs[MaxCosts]{}; /// current costs of the unit int Storing[MaxCosts]{}; /// storage increasing int ImproveIncomes[MaxCosts]{}; /// Gives player an improved income diff --git a/src/unit/script_unittype.cpp b/src/unit/script_unittype.cpp index e9c2c5651d..e96af2eb60 100644 --- a/src/unit/script_unittype.cpp +++ b/src/unit/script_unittype.cpp @@ -1405,8 +1405,8 @@ static int CclDefineUnitStats(lua_State *l) Assert(playerId < PlayerMax); CUnitStats *stats = &type.Stats[playerId]; - if (!stats->Variables) { - stats->Variables = new CVariable[UnitTypeVar.GetNumberVariable()]; + if (stats->Variables.empty()) { + stats->Variables.resize(UnitTypeVar.GetNumberVariable()); } // Parse the list: (still everything could be changed!) diff --git a/src/unit/unit.cpp b/src/unit/unit.cpp index cab6758e58..aa71356fce 100644 --- a/src/unit/unit.cpp +++ b/src/unit/unit.cpp @@ -621,7 +621,7 @@ void CUnit::Init(const CUnitType &type) Assert(!Variable); const unsigned int size = UnitTypeVar.GetNumberVariable(); Variable = new CVariable[size]; - std::copy(type.MapDefaultStat.Variables, type.MapDefaultStat.Variables + size, Variable); + std::copy(type.MapDefaultStat.Variables.begin(), type.MapDefaultStat.Variables.end(), Variable); } else { Variable = nullptr; } @@ -746,12 +746,12 @@ void CUnit::AssignToPlayer(CPlayer &player) } } Player = &player; - Stats = &type.Stats[Player->Index]; + Stats = const_cast(&type.Stats[Player->Index]); if (!SaveGameLoading) { if (UnitTypeVar.GetNumberVariable()) { Assert(Variable); - Assert(Stats->Variables); - memcpy(Variable, Stats->Variables, UnitTypeVar.GetNumberVariable() * sizeof(*Variable)); + Assert(Stats->Variables.size() == UnitTypeVar.GetNumberVariable()); + std::copy(Stats->Variables.begin(), Stats->Variables.end(), Variable); } } } @@ -1870,7 +1870,7 @@ void CUnit::ChangeOwner(CPlayer &newplayer) MapUnmarkUnitSight(*this); newplayer.AddUnit(*this); - Stats = &Type->Stats[newplayer.Index]; + Stats = const_cast(&Type->Stats[newplayer.Index]); UpdateUnitSightRange(*this); MapMarkUnitSight(*this); diff --git a/src/unit/unittype.cpp b/src/unit/unittype.cpp index f622060452..3cf9a3b535 100644 --- a/src/unit/unittype.cpp +++ b/src/unit/unittype.cpp @@ -824,10 +824,8 @@ std::pair NewUnitTypeSlot(std::string_view ident) type->Ident = ident; type->BoolFlag.resize(new_bool_size); - type->DefaultStat.Variables = new CVariable[UnitTypeVar.GetNumberVariable()]; - for (unsigned int i = 0; i < UnitTypeVar.GetNumberVariable(); ++i) { - type->DefaultStat.Variables[i] = UnitTypeVar.Variable[i]; - } + type->DefaultStat.Variables = UnitTypeVar.Variable; + UnitTypes.push_back(type); UnitTypeMap[type->Ident] = type; return {type, false}; diff --git a/src/unit/upgrade.cpp b/src/unit/upgrade.cpp index 878ef9f178..6d17a9981c 100644 --- a/src/unit/upgrade.cpp +++ b/src/unit/upgrade.cpp @@ -78,12 +78,6 @@ static std::map> Upgrades; -- Functions ----------------------------------------------------------------------------*/ - -CUnitStats::~CUnitStats() -{ - delete [] this->Variables; -} - CUnitStats &CUnitStats::operator = (const CUnitStats &rhs) { for (unsigned int i = 0; i < MaxCosts; ++i) { @@ -91,11 +85,7 @@ CUnitStats &CUnitStats::operator = (const CUnitStats &rhs) this->Storing[i] = rhs.Storing[i]; this->ImproveIncomes[i] = rhs.ImproveIncomes[i]; } - delete [] this->Variables; - const unsigned int size = UnitTypeVar.GetNumberVariable(); - this->Variables = new CVariable[size]; - - std::copy(rhs.Variables, rhs.Variables + size, this->Variables); + this->Variables = rhs.Variables; return *this; } @@ -248,7 +238,7 @@ static int CclDefineModifier(lua_State *l) memset(um->ChangeUpgrades, '?', sizeof(um->ChangeUpgrades)); memset(um->ApplyTo, '?', sizeof(um->ApplyTo)); - um->Modifier.Variables = new CVariable[UnitTypeVar.GetNumberVariable()]; + um->Modifier.Variables.resize(UnitTypeVar.GetNumberVariable()); um->ModifyPercent.resize(UnitTypeVar.GetNumberVariable()); std::string_view upgrade_ident = LuaToString(l, 1);