From d3ec94393a8410edfdfec77977c5b6ac45a2bbba Mon Sep 17 00:00:00 2001 From: Jarod42 Date: Mon, 13 May 2024 18:56:15 +0200 Subject: [PATCH 1/3] Replace `memcpy` by `ranges::copy`. So fix copy of `ResInfo` in `CclCopyUnitType` --- src/ui/botpanel.cpp | 15 ++++++--------- src/unit/script_unittype.cpp | 14 ++++++++------ 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/ui/botpanel.cpp b/src/ui/botpanel.cpp index bbf30c67eb..a2ac818a8a 100644 --- a/src/ui/botpanel.cpp +++ b/src/ui/botpanel.cpp @@ -528,24 +528,20 @@ void DrawPopup(const ButtonAction &button, const CUIButton &uibutton, int x, int LastDrawnButtonPopup = const_cast(&button); } - int popupWidth, popupHeight; - int Costs[ManaResCost + 1]; - memset(Costs, 0, sizeof(Costs)); - + int Costs[ManaResCost + 1]{}; switch (button.Action) { case ButtonCmd::Research: - memcpy(Costs, AllUpgrades[button.Value]->Costs, sizeof(AllUpgrades[button.Value]->Costs)); + ranges::copy(AllUpgrades[button.Value]->Costs, std::begin(Costs)); break; case ButtonCmd::SpellCast: - memcpy(Costs, SpellTypeTable[button.Value]->Costs, sizeof(SpellTypeTable[button.Value]->Costs)); + ranges::copy(SpellTypeTable[button.Value]->Costs, std::begin(Costs)); Costs[ManaResCost] = SpellTypeTable[button.Value]->ManaCost; break; case ButtonCmd::Build: case ButtonCmd::Train: case ButtonCmd::UpgradeTo: - memcpy(Costs, - getUnitTypes()[button.Value]->Stats[ThisPlayer->Index].Costs, - sizeof(getUnitTypes()[button.Value]->Stats[ThisPlayer->Index].Costs)); + ranges::copy(getUnitTypes()[button.Value]->Stats[ThisPlayer->Index].Costs, + std::begin(Costs)); Costs[FoodCost] = getUnitTypes()[button.Value] ->Stats[ThisPlayer->Index] .Variables[DEMAND_INDEX] @@ -555,6 +551,7 @@ void DrawPopup(const ButtonAction &button, const CUIButton &uibutton, int x, int break; } + int popupWidth, popupHeight; if (useCache) { popupWidth = popupCache.popupWidth; popupHeight = popupCache.popupHeight; diff --git a/src/unit/script_unittype.cpp b/src/unit/script_unittype.cpp index 0b447dbe01..a77c42c75e 100644 --- a/src/unit/script_unittype.cpp +++ b/src/unit/script_unittype.cpp @@ -1239,9 +1239,9 @@ static int CclCopyUnitType(lua_State *l) to->Portrait.Files = from.Portrait.Files; to->Portrait.Mngs.resize(to->Portrait.Files.size()); #endif - memcpy(to->DefaultStat.Costs, from.DefaultStat.Costs, sizeof(from.DefaultStat.Costs)); - memcpy(to->DefaultStat.Storing, from.DefaultStat.Storing, sizeof(from.DefaultStat.Storing)); - memcpy(to->DefaultStat.ImproveIncomes, from.DefaultStat.ImproveIncomes, sizeof(from.DefaultStat.ImproveIncomes)); + ranges::copy(from.DefaultStat.Costs, std::begin(to->DefaultStat.Costs)); + ranges::copy(from.DefaultStat.Storing, std::begin(to->DefaultStat.Storing)); + ranges::copy(from.DefaultStat.ImproveIncomes, std::begin(to->DefaultStat.ImproveIncomes)); to->Construction = from.Construction; to->DrawLevel = from.DrawLevel; to->MaxOnBoard = from.MaxOnBoard; @@ -1310,7 +1310,7 @@ static int CclCopyUnitType(lua_State *l) to->CanAttack = from.CanAttack; to->RepairRange = from.RepairRange; to->RepairHP = from.RepairHP; - memcpy(to->RepairCosts, from.RepairCosts, sizeof(from.RepairCosts)); + ranges::copy(from.RepairCosts, std::begin(to->RepairCosts)); to->CanTarget = from.CanTarget; to->Building = from.Building; to->BuildingRules.clear(); @@ -1343,9 +1343,11 @@ static int CclCopyUnitType(lua_State *l) to->BoolFlag[i].CanTargetFlag = from.BoolFlag[i].CanTargetFlag; to->BoolFlag[i].AiPriorityTarget = from.BoolFlag[i].AiPriorityTarget; } - memcpy(to->ResInfo, from.ResInfo, sizeof(from.ResInfo)); + for (std::size_t i = 0; i != std::size(to->ResInfo); ++i) { + from.ResInfo[i] = to->ResInfo[i] ? std::make_unique(*to->ResInfo[i]) : nullptr; + } to->GivesResource = from.GivesResource; - memcpy(to->CanStore, from.CanStore, sizeof(from.CanStore)); + ranges::copy(from.CanStore, std::begin(to->CanStore)); to->CanCastSpell = from.CanCastSpell; to->AutoCastActive = from.AutoCastActive; to->Sound.Selected.Name = from.Sound.Selected.Name; From fe7c05105d40e9e583dc99b6e9d8247fade1d759 Mon Sep 17 00:00:00 2001 From: Jarod42 Date: Mon, 13 May 2024 19:41:05 +0200 Subject: [PATCH 2/3] Replace `memset` by `ranges::fill` (or by value initialization). --- src/ai/ai_resource.cpp | 4 +--- src/include/ui/statusline.h | 14 ++++++-------- src/include/upgrade_structs.h | 7 ++++--- src/include/util.h | 1 + src/map/map.cpp | 4 ++-- src/map/tileset.cpp | 8 ++++---- src/network/mdns_wrapper.cpp | 5 ++--- src/network/net_lowlevel.cpp | 9 +++------ src/network/net_message.cpp | 2 +- src/network/netsockets.cpp | 3 +-- src/network/network.cpp | 10 +++++----- src/stratagus/player.cpp | 22 +++++++++++----------- src/stratagus/util.cpp | 3 +-- src/ui/interface.cpp | 2 +- src/ui/statusline.cpp | 2 +- src/unit/unit.cpp | 4 ++-- src/unit/upgrade.cpp | 4 ++-- src/video/shaders.cpp | 2 +- tests/network/test_net_lowlevel.cpp | 4 ++-- tests/network/test_udpsocket.cpp | 4 ++-- 20 files changed, 53 insertions(+), 61 deletions(-) diff --git a/src/ai/ai_resource.cpp b/src/ai/ai_resource.cpp index 08fa23fe28..d8f58726da 100644 --- a/src/ai/ai_resource.cpp +++ b/src/ai/ai_resource.cpp @@ -490,9 +490,7 @@ static bool AiRequestSupply() // Count the already made build requests. const auto counter = AiGetBuildRequestsCount(*AiPlayer); - struct cnode cache[16]; - - memset(cache, 0, sizeof(cache)); + struct cnode cache[16]{}; // // Check if we can build this? diff --git a/src/include/ui/statusline.h b/src/include/ui/statusline.h index 42c1a033a4..04b1b2fb9d 100644 --- a/src/include/ui/statusline.h +++ b/src/include/ui/statusline.h @@ -41,9 +41,7 @@ class CFont; class CStatusLine { public: - CStatusLine() : Width(0), TextX(0), TextY(0), Font(0) { - memset(Costs, 0, (ManaResCost + 1) * sizeof(int)); - } + CStatusLine() = default; void Draw(); void DrawCosts(); @@ -54,11 +52,11 @@ class CStatusLine void ClearCosts(); public: - int Width; - int TextX; - int TextY; - CFont *Font; - int Costs[ManaResCost + 1]; + int Width = 0; + int TextX = 0; + int TextY = 0; + CFont *Font = nullptr; + int Costs[ManaResCost + 1]{}; private: std::string StatusLine; diff --git a/src/include/upgrade_structs.h b/src/include/upgrade_structs.h index 020d0be34f..048ebc851d 100644 --- a/src/include/upgrade_structs.h +++ b/src/include/upgrade_structs.h @@ -38,6 +38,7 @@ ----------------------------------------------------------------------------*/ #include "settings.h" +#include "util.h" #include #include @@ -198,8 +199,8 @@ class CAllow void Clear() { - memset(Units, 0, sizeof(Units)); - memset(Upgrades, 0, sizeof(Upgrades)); + ranges::fill(Units, 0); + ranges::fill(Upgrades, '\0'); } int Units[UnitTypeMax]{}; /// maximum amount of units allowed @@ -215,7 +216,7 @@ class CUpgradeTimers public: CUpgradeTimers() = default; - void Clear() { memset(Upgrades, 0, sizeof(Upgrades)); } + void Clear() { ranges::fill(Upgrades, 0); } /** ** all 0 at the beginning, all upgrade actions do increment values in diff --git a/src/include/util.h b/src/include/util.h index c01bf0cfdc..b2404c7c20 100644 --- a/src/include/util.h +++ b/src/include/util.h @@ -33,6 +33,7 @@ //@{ #include "filesystem.h" +#include "stratagus.h" #include #include diff --git a/src/map/map.cpp b/src/map/map.cpp index e8e9304aad..3770955108 100644 --- a/src/map/map.cpp +++ b/src/map/map.cpp @@ -313,8 +313,8 @@ void CMapInfo::Clear() this->Description.clear(); this->Filename.clear(); this->MapWidth = this->MapHeight = 0; - memset(this->PlayerSide, 0, sizeof(this->PlayerSide)); - memset(this->PlayerType, 0, sizeof(this->PlayerType)); + ranges::fill(this->PlayerSide, 0); + ranges::fill(this->PlayerType, PlayerTypes::PlayerUnset); this->MapUID = 0; this->HighgroundsEnabled = false; diff --git a/src/map/tileset.cpp b/src/map/tileset.cpp index d6b4c166f8..c5bc1e88de 100644 --- a/src/map/tileset.cpp +++ b/src/map/tileset.cpp @@ -214,15 +214,15 @@ void CTileset::clear() midOneTreeTile = 0; botOneTreeTile = 0; removedTreeTile = 0; - memset(woodTable, 0, sizeof(woodTable)); + ranges::fill(woodTable, 0); mixedLookupTable.clear(); topOneRockTile = 0; midOneRockTile = 0; botOneRockTile = 0; removedRockTile = 0; - memset(rockTable, 0, sizeof(rockTable)); - memset(humanWallTable, 0, sizeof(humanWallTable)); - memset(orcWallTable, 0, sizeof(orcWallTable)); + ranges::fill(rockTable, 0); + ranges::fill(humanWallTable, 0); + ranges::fill(orcWallTable, 0); } bool CTileset::setTileCount(const size_t newCount) diff --git a/src/network/mdns_wrapper.cpp b/src/network/mdns_wrapper.cpp index 17745818a3..e89fa83afd 100644 --- a/src/network/mdns_wrapper.cpp +++ b/src/network/mdns_wrapper.cpp @@ -64,9 +64,8 @@ static int service_callback(int sock, const struct sockaddr* from, size_t addrle void MDNS::AnswerServerQueries() { if (serviceSocket == -1) { - // When recieving, a socket can recieve data from all network interfaces - struct sockaddr_in sock_addr; - memset(&sock_addr, 0, sizeof(struct sockaddr_in)); + // When receiving, a socket can receive data from all network interfaces + struct sockaddr_in sock_addr{}; sock_addr.sin_family = AF_INET; #ifdef _WIN32 sock_addr.sin_addr = in4addr_any; diff --git a/src/network/net_lowlevel.cpp b/src/network/net_lowlevel.cpp index 06a5936aac..3ea5233265 100644 --- a/src/network/net_lowlevel.cpp +++ b/src/network/net_lowlevel.cpp @@ -307,9 +307,8 @@ Socket NetOpenUDP(unsigned long ip, int port) } // bind local port if (port) { - struct sockaddr_in sock_addr; + struct sockaddr_in sock_addr{}; - memset(&sock_addr, 0, sizeof(sock_addr)); sock_addr.sin_family = AF_INET; sock_addr.sin_addr.s_addr = ip; sock_addr.sin_port = htons(port); @@ -339,9 +338,8 @@ Socket NetOpenTCP(const char *addr, int port) } // bind local port if (port) { - struct sockaddr_in sock_addr; + struct sockaddr_in sock_addr{}; - memset(&sock_addr, 0, sizeof(sock_addr)); sock_addr.sin_family = AF_INET; if (addr) { sock_addr.sin_addr.s_addr = inet_addr(addr); @@ -377,7 +375,6 @@ Socket NetOpenTCP(const char *addr, int port) */ int NetConnectTCP(Socket sockfd, unsigned long addr, int port) { - struct sockaddr_in sa; #ifndef __BEOS__ int opt = 1; setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, (setsockopttype)&opt, sizeof(opt)); @@ -389,7 +386,7 @@ int NetConnectTCP(Socket sockfd, unsigned long addr, int port) return -1; } - memset(&sa, 0, sizeof(sa)); + struct sockaddr_in sa{}; memcpy(&sa.sin_addr, &addr, sizeof(addr)); sa.sin_family = AF_INET; sa.sin_port = htons(port); diff --git a/src/network/net_message.cpp b/src/network/net_message.cpp index 36b4dd8ebb..6441199baf 100644 --- a/src/network/net_message.cpp +++ b/src/network/net_message.cpp @@ -233,7 +233,7 @@ void CNetworkHost::Clear() this->Host = 0; this->Port = 0; this->PlyNr = 0; - memset(this->PlyName, 0, sizeof(this->PlyName)); + ranges::fill(this->PlyName, '\0'); } void CNetworkHost::SetName(const char *name) diff --git a/src/network/netsockets.cpp b/src/network/netsockets.cpp index 8566b1ec2e..fd89c651ba 100644 --- a/src/network/netsockets.cpp +++ b/src/network/netsockets.cpp @@ -184,8 +184,7 @@ class CTCPSocket_Impl bool CTCPSocket_Impl::Open(const CHost &host) { - char ip[24]; // 127.255.255.255:65555 - memset(&ip, 0, sizeof(ip)); + char ip[24]{}; // 127.255.255.255:65555 sprintf(ip, "%d.%d.%d.%d", NIPQUAD(ntohl(host.getIp()))); this->socket = NetOpenTCP(ip, host.getPort()); return this->socket != INVALID_SOCKET; diff --git a/src/network/network.cpp b/src/network/network.cpp index 86e502968b..78904b841b 100644 --- a/src/network/network.cpp +++ b/src/network/network.cpp @@ -512,11 +512,11 @@ void NetworkOnStartGame() ncqs[1].Type = MessageNone; } } - memset(NetworkSyncSeeds, 0, sizeof(NetworkSyncSeeds)); - memset(NetworkSyncHashs, 0, sizeof(NetworkSyncHashs)); - memset(PlayerQuit, 0, sizeof(PlayerQuit)); - memset(NetworkLastFrame, 0, sizeof(NetworkLastFrame)); - memset(NetworkLastCycle, 0, sizeof(NetworkLastCycle)); + ranges::fill(NetworkSyncSeeds, 0); + ranges::fill(NetworkSyncHashs, 0); + ranges::fill(PlayerQuit, 0); + ranges::fill(NetworkLastFrame, 0); + ranges::fill(NetworkLastCycle, 0); } //---------------------------------------------------------------------------- diff --git a/src/stratagus/player.cpp b/src/stratagus/player.cpp index 8b463d0b7b..9b778a87cb 100644 --- a/src/stratagus/player.cpp +++ b/src/stratagus/player.cpp @@ -751,8 +751,8 @@ void CPlayer::Init(PlayerTypes type) this->MaxResources[i] = DefaultResourceMaxAmounts[i]; } - memset(this->UnitTypesCount, 0, sizeof(this->UnitTypesCount)); - memset(this->UnitTypesAiActiveCount, 0, sizeof(this->UnitTypesAiActiveCount)); + ranges::fill(this->UnitTypesCount, 0); + ranges::fill(this->UnitTypesAiActiveCount, 0); this->Supply = 0; this->Demand = 0; @@ -801,14 +801,14 @@ void CPlayer::Clear() GaveVisionTo.clear(); StartPos.x = 0; StartPos.y = 0; - memset(Resources, 0, sizeof(Resources)); - memset(StoredResources, 0, sizeof(StoredResources)); - memset(MaxResources, 0, sizeof(MaxResources)); - memset(LastResources, 0, sizeof(LastResources)); - memset(Incomes, 0, sizeof(Incomes)); - memset(Revenue, 0, sizeof(Revenue)); - memset(UnitTypesCount, 0, sizeof(UnitTypesCount)); - memset(UnitTypesAiActiveCount, 0, sizeof(UnitTypesAiActiveCount)); + ranges::fill(Resources, 0); + ranges::fill(StoredResources, 0); + ranges::fill(MaxResources, 0); + ranges::fill(LastResources, 0); + ranges::fill(Incomes, 0); + ranges::fill(Revenue, 0); + ranges::fill(UnitTypesCount, 0); + ranges::fill(UnitTypesAiActiveCount, 0); AiEnabled = false; Ai = nullptr; this->Units.resize(0); @@ -823,7 +823,7 @@ void CPlayer::Clear() Score = 0; TotalUnits = 0; TotalBuildings = 0; - memset(TotalResources, 0, sizeof(TotalResources)); + ranges::fill(TotalResources, 0); TotalRazings = 0; TotalKills = 0; this->LostMainFacilityTimer = 0; diff --git a/src/stratagus/util.cpp b/src/stratagus/util.cpp index 0e429dd0ee..623bbfab09 100644 --- a/src/stratagus/util.cpp +++ b/src/stratagus/util.cpp @@ -676,8 +676,7 @@ void aligned_free(void *block) fs::path GetExecutablePath() { #ifdef WIN32 - TCHAR executable_path[MAX_PATH]; - memset(executable_path, 0, sizeof(executable_path)); + TCHAR executable_path[MAX_PATH]{}; GetModuleFileName(nullptr, executable_path, sizeof(executable_path)-1); #else const auto& executable_path = OriginalArgv[0]; diff --git a/src/ui/interface.cpp b/src/ui/interface.cpp index 164a838673..3213635c00 100644 --- a/src/ui/interface.cpp +++ b/src/ui/interface.cpp @@ -135,7 +135,7 @@ static void ShowInput() static void UiBeginInput() { KeyState = EKeyState::Input; - memset(Input, 0, sizeof(Input)); + ranges::fill(Input, '\0'); InputIndex = 0; addCursorToInput(); UI.StatusLine.ClearCosts(); diff --git a/src/ui/statusline.cpp b/src/ui/statusline.cpp index 558c93d6b7..5ad1a431d3 100644 --- a/src/ui/statusline.cpp +++ b/src/ui/statusline.cpp @@ -80,7 +80,7 @@ void CStatusLine::SetCosts(int mana, int food, const int *costs) if (costs) { memcpy(Costs, costs, MaxCosts * sizeof(*costs)); } else { - memset(Costs, 0, sizeof(Costs)); + ranges::fill(Costs, 0); } Costs[ManaResCost] = mana; Costs[FoodCost] = food; diff --git a/src/unit/unit.cpp b/src/unit/unit.cpp index 50cd442a0a..7b240a3096 100644 --- a/src/unit/unit.cpp +++ b/src/unit/unit.cpp @@ -426,7 +426,7 @@ void CUnit::Init() Frame = 0; Colors = -1; - memset(IndividualUpgrades, 0, sizeof(IndividualUpgrades)); + ranges::fill(IndividualUpgrades, false); IX = 0; IY = 0; Direction = 0; @@ -453,7 +453,7 @@ void CUnit::Init() JustMoved = 0; TeamSelected = 0; RescuedFrom = nullptr; - memset(VisCount, 0, sizeof(VisCount)); + ranges::fill(VisCount, 0); memset(&Seen, 0, sizeof(Seen)); Variable.clear(); TTL = 0; diff --git a/src/unit/upgrade.cpp b/src/unit/upgrade.cpp index dad5111b55..ce32456793 100644 --- a/src/unit/upgrade.cpp +++ b/src/unit/upgrade.cpp @@ -218,8 +218,8 @@ static int CclDefineModifier(lua_State *l) auto um = std::make_unique(); - memset(um->ChangeUpgrades, '?', sizeof(um->ChangeUpgrades)); - memset(um->ApplyTo, '?', sizeof(um->ApplyTo)); + ranges::fill(um->ChangeUpgrades, '?'); + ranges::fill(um->ApplyTo, '?'); um->Modifier.Variables.resize(UnitTypeVar.GetNumberVariable()); um->ModifyPercent.resize(UnitTypeVar.GetNumberVariable()); diff --git a/src/video/shaders.cpp b/src/video/shaders.cpp index d08384c44a..bf95058953 100644 --- a/src/video/shaders.cpp +++ b/src/video/shaders.cpp @@ -358,7 +358,7 @@ static bool RenderWithShaderInternal(SDL_Renderer *renderer, SDL_Window* win, SD lazyGlGetFloatv(GL_MODELVIEW_MATRIX, modelview); lazyGlGetFloatv(GL_PROJECTION_MATRIX, projection); - memset(matrix, 0, sizeof(matrix)); + ranges::fill(matrix, 0); for (int i = 0; i < 4; i++) { for(int j = 0; j < 4; j++) { for (int k = 0; k < 4; k++) { diff --git a/tests/network/test_net_lowlevel.cpp b/tests/network/test_net_lowlevel.cpp index d286d1cd83..6ef8d6c2cd 100644 --- a/tests/network/test_net_lowlevel.cpp +++ b/tests/network/test_net_lowlevel.cpp @@ -134,7 +134,7 @@ class ClientTCP class Foo { public: - Foo() { memset(&data, 0, sizeof(data)); } + Foo() = default; void Fill() { @@ -153,7 +153,7 @@ class Foo return true; } public: - char data[42]; + char data[42]{}; }; class ReceiverTCPJob : public Job diff --git a/tests/network/test_udpsocket.cpp b/tests/network/test_udpsocket.cpp index d53dcf0537..1751182966 100644 --- a/tests/network/test_udpsocket.cpp +++ b/tests/network/test_udpsocket.cpp @@ -55,7 +55,7 @@ TEST_CASE_FIXTURE(AutoNetwork, "CHost") class Foo { public: - Foo() { memset(&data, 0, sizeof(data)); } + Foo() = default; void Fill() { @@ -74,7 +74,7 @@ class Foo return true; } public: - char data[42]; + char data[42]{}; }; TEST_CASE_FIXTURE(AutoNetwork, "CUDPSocket") From 65390577ccb283760aa883d45387f4d6d91787a9 Mon Sep 17 00:00:00 2001 From: Jarod42 Date: Mon, 20 May 2024 09:24:55 +0200 Subject: [PATCH 3/3] Replace `memcmp` by `std::equal`. --- src/include/util.h | 7 +++++++ src/network/net_message.cpp | 5 ++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/include/util.h b/src/include/util.h index b2404c7c20..73293e896d 100644 --- a/src/include/util.h +++ b/src/include/util.h @@ -197,6 +197,13 @@ namespace ranges return std::none_of(std::begin(range), std::end(range), std::forward(predicate)); } + template + bool equal(const Range &range1, const Range &range2) + { + return std::equal(std::begin(range1), std::end(range1), std::begin(range2), std::end(range2)); + } + + template std::size_t count_if(const Range &range, Predicate &&predicate) { diff --git a/src/network/net_message.cpp b/src/network/net_message.cpp index 6441199baf..02593c9c4f 100644 --- a/src/network/net_message.cpp +++ b/src/network/net_message.cpp @@ -348,9 +348,8 @@ void CServerSetup::Save(const std::function & f) { bool CServerSetup::operator == (const CServerSetup &rhs) const { - return (ServerGameSettings == rhs.ServerGameSettings - && memcmp(CompOpt, rhs.CompOpt, sizeof(CompOpt)) == 0 - && memcmp(Ready, rhs.Ready, sizeof(Ready)) == 0); + return (ServerGameSettings == rhs.ServerGameSettings && ranges::equal(CompOpt, rhs.CompOpt) + && ranges::equal(Ready, rhs.Ready)); } //