From c4fde6211e808acb0e4825db7c51f156169a041f Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Thu, 17 Oct 2024 19:17:21 -0300 Subject: [PATCH 1/3] improve: change filestream/fileloader to std::ranges::copy --- src/io/fileloader.hpp | 31 +++++++++++++------- src/io/filestream.cpp | 26 ++++++++++------ src/io/functions/iologindata_load_player.cpp | 12 ++------ src/io/functions/iologindata_save_player.cpp | 4 +-- src/io/ioprey.cpp | 5 ++-- 5 files changed, 44 insertions(+), 34 deletions(-) diff --git a/src/io/fileloader.hpp b/src/io/fileloader.hpp index 2069487202f..fc4f71023e1 100644 --- a/src/io/fileloader.hpp +++ b/src/io/fileloader.hpp @@ -22,9 +22,9 @@ namespace OTB { Node &operator=(const Node &) = delete; std::list children; - mio::mmap_source::const_iterator propsBegin; - mio::mmap_source::const_iterator propsEnd; - uint8_t type; + mio::mmap_source::const_iterator propsBegin {}; + mio::mmap_source::const_iterator propsEnd {}; + uint8_t type {}; enum NodeChar : uint8_t { ESCAPE = 0xFD, START = 0xFE, @@ -67,12 +67,19 @@ class PropStream { template bool read(T &ret) { + static_assert(std::is_trivially_copyable_v, "Type T must be trivially copyable"); + if (size() < sizeof(T)) { return false; } - memcpy(&ret, p, sizeof(T)); + std::span sourceSpan(reinterpret_cast(p), sizeof(T)); + std::array tempBuffer; + std::ranges::copy(sourceSpan, tempBuffer.begin()); + ret = std::bit_cast(tempBuffer); + p += sizeof(T); + return true; } @@ -86,12 +93,13 @@ class PropStream { return false; } - char* str = new char[strLen + 1]; - memcpy(str, p, strLen); - str[strLen] = 0; - ret.assign(str, strLen); - delete[] str; + std::vector tempBuffer(strLen); + std::span sourceSpan(reinterpret_cast(p), strLen); + std::ranges::copy(sourceSpan, tempBuffer.begin()); + ret.assign(reinterpret_cast(tempBuffer.data()), strLen); + p += strLen; + return true; } @@ -129,7 +137,8 @@ class PropWriteStream { template void write(T add) { char* addr = reinterpret_cast(&add); - std::copy(addr, addr + sizeof(T), std::back_inserter(buffer)); + std::span sourceSpan(addr, sizeof(T)); + std::ranges::copy(sourceSpan, std::back_inserter(buffer)); } void writeString(const std::string &str) { @@ -140,7 +149,7 @@ class PropWriteStream { } write(static_cast(strLength)); - std::copy(str.begin(), str.end(), std::back_inserter(buffer)); + std::ranges::copy(str, std::back_inserter(buffer)); } private: diff --git a/src/io/filestream.cpp b/src/io/filestream.cpp index 71b2dd4aa2f..687c2501df9 100644 --- a/src/io/filestream.cpp +++ b/src/io/filestream.cpp @@ -17,7 +17,8 @@ uint32_t FileStream::tell() const { void FileStream::seek(uint32_t pos) { if (pos > m_data.size()) { - throw std::ios_base::failure("Seek failed"); + g_logger().error("Seek failed"); + return; } m_pos = pos; } @@ -29,7 +30,8 @@ void FileStream::skip(uint32_t len) { uint32_t FileStream::size() const { std::size_t size = m_data.size(); if (size > std::numeric_limits::max()) { - throw std::overflow_error("File size exceeds uint32_t range"); + g_logger().error("File size exceeds uint32_t range"); + return {}; } return static_cast(size); @@ -37,27 +39,31 @@ uint32_t FileStream::size() const { template bool FileStream::read(T &ret, bool escape) { + static_assert(std::is_trivially_copyable_v, "Type T must be trivially copyable"); + const auto size = sizeof(T); if (m_pos + size > m_data.size()) { - throw std::ios_base::failure("Read failed"); + g_logger().error("Read failed"); + return false; } std::array array; if (escape) { - for (int_fast8_t i = -1; ++i < size;) { + for (int_fast8_t i = 0; i < size; ++i) { if (m_data[m_pos] == OTB::Node::ESCAPE) { ++m_pos; } array[i] = m_data[m_pos]; ++m_pos; } - memcpy(&ret, array.data(), size); } else { - memcpy(&ret, &m_data[m_pos], size); + std::span sourceSpan(m_data.data() + m_pos, size); + std::ranges::copy(sourceSpan, array.begin()); m_pos += size; } + ret = std::bit_cast(array); return true; } @@ -65,7 +71,8 @@ uint8_t FileStream::getU8() { uint8_t v = 0; if (m_pos + 1 > m_data.size()) { - throw std::ios_base::failure("Failed to getU8"); + g_logger().error("Failed to getU8"); + return {}; } // Fast Escape Val @@ -101,13 +108,14 @@ std::string FileStream::getString() { std::string str; if (const uint16_t len = getU16(); len > 0 && len < 8192) { if (m_pos + len > m_data.size()) { - throw std::ios_base::failure("[FileStream::getString] - Read failed"); + g_logger().error("[FileStream::getString] - Read failed"); + return {}; } str = { (char*)&m_data[m_pos], len }; m_pos += len; } else if (len != 0) { - throw std::ios_base::failure("[FileStream::getString] - Read failed because string is too big"); + g_logger().error("[FileStream::getString] - Read failed because string is too big"); } return str; } diff --git a/src/io/functions/iologindata_load_player.cpp b/src/io/functions/iologindata_load_player.cpp index cb24c092ebb..5b2621c1cbf 100644 --- a/src/io/functions/iologindata_load_player.cpp +++ b/src/io/functions/iologindata_load_player.cpp @@ -734,10 +734,6 @@ void IOLoginDataLoad::loadPlayerPreyClass(std::shared_ptr player, DBResu query << "SELECT * FROM `player_prey` WHERE `player_id` = " << player->getGUID(); if ((result = db.storeQuery(query.str()))) { do { - auto selectedRaceId = result->getNumber("raceid"); - if (selectedRaceId == 0) { - continue; - } auto slot = std::make_unique(static_cast(result->getNumber("slot"))); auto state = static_cast(result->getNumber("state")); if (slot->id == PreySlot_Two && state == PreyDataState_Locked) { @@ -749,7 +745,7 @@ void IOLoginDataLoad::loadPlayerPreyClass(std::shared_ptr player, DBResu } else { slot->state = state; } - slot->selectedRaceId = selectedRaceId; + slot->selectedRaceId = result->getNumber("raceid"); slot->option = static_cast(result->getNumber("option")); slot->bonus = static_cast(result->getNumber("bonus_type")); slot->bonusRarity = static_cast(result->getNumber("bonus_rarity")); @@ -785,10 +781,6 @@ void IOLoginDataLoad::loadPlayerTaskHuntingClass(std::shared_ptr player, query << "SELECT * FROM `player_taskhunt` WHERE `player_id` = " << player->getGUID(); if ((result = db.storeQuery(query.str()))) { do { - auto selectedRaceId = result->getNumber("raceid"); - if (selectedRaceId == 0) { - continue; - } auto slot = std::make_unique(static_cast(result->getNumber("slot"))); auto state = static_cast(result->getNumber("state")); if (slot->id == PreySlot_Two && state == PreyTaskDataState_Locked) { @@ -800,7 +792,7 @@ void IOLoginDataLoad::loadPlayerTaskHuntingClass(std::shared_ptr player, } else { slot->state = state; } - slot->selectedRaceId = selectedRaceId; + slot->selectedRaceId = result->getNumber("raceid"); slot->upgrade = result->getNumber("upgrade"); slot->rarity = static_cast(result->getNumber("rarity")); slot->currentKills = result->getNumber("kills"); diff --git a/src/io/functions/iologindata_save_player.cpp b/src/io/functions/iologindata_save_player.cpp index f320a67ca22..ec8ab71fd07 100644 --- a/src/io/functions/iologindata_save_player.cpp +++ b/src/io/functions/iologindata_save_player.cpp @@ -606,7 +606,7 @@ bool IOLoginDataSave::savePlayerPreyClass(std::shared_ptr player) { << slot->freeRerollTimeStamp << ", "; PropWriteStream propPreyStream; - std::ranges::for_each(slot->raceIdList.begin(), slot->raceIdList.end(), [&propPreyStream](uint16_t raceId) { + std::ranges::for_each(slot->raceIdList, [&propPreyStream](uint16_t raceId) { propPreyStream.write(raceId); }); @@ -659,7 +659,7 @@ bool IOLoginDataSave::savePlayerTaskHuntingClass(std::shared_ptr player) query << slot->freeRerollTimeStamp << ", "; PropWriteStream propTaskHuntingStream; - std::ranges::for_each(slot->raceIdList.begin(), slot->raceIdList.end(), [&propTaskHuntingStream](uint16_t raceId) { + std::ranges::for_each(slot->raceIdList, [&propTaskHuntingStream](uint16_t raceId) { propTaskHuntingStream.write(raceId); }); diff --git a/src/io/ioprey.cpp b/src/io/ioprey.cpp index 8dcbfe10e46..b21f442bf67 100644 --- a/src/io/ioprey.cpp +++ b/src/io/ioprey.cpp @@ -65,8 +65,9 @@ void PreySlot::reloadMonsterGrid(std::vector blackList, uint32_t level // Disabling prey system if the server have less then 36 registered monsters on bestiary because: // - Impossible to generate random lists without duplications on slots. // - Stress the server with unnecessary loops. - std::map bestiary = g_game().getBestiaryList(); + const std::map &bestiary = g_game().getBestiaryList(); if (bestiary.size() < 36) { + g_logger().error("[PreySlot::reloadMonsterGrid] - Bestiary size is less than 36, disabling prey system."); return; } @@ -338,7 +339,7 @@ void IOPrey::parsePreyAction(std::shared_ptr player, PreySlot_t slotId, } else if (player->getPreyWithMonster(raceId)) { player->sendMessageDialog("This creature is already selected on another slot."); return; - } else if (!mtype->info.isPreyable) { + } else if (mtype && !mtype->info.isPreyable) { player->sendMessageDialog("This creature can't be select on prey. Please choose another one."); return; } From afa875bdfe4d9700688647475bf2f178fd5b182e Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Thu, 17 Oct 2024 19:43:43 -0300 Subject: [PATCH 2/3] fix: remove use of reinterpret cast --- src/io/fileloader.hpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/io/fileloader.hpp b/src/io/fileloader.hpp index fc4f71023e1..93c02502717 100644 --- a/src/io/fileloader.hpp +++ b/src/io/fileloader.hpp @@ -73,9 +73,12 @@ class PropStream { return false; } - std::span sourceSpan(reinterpret_cast(p), sizeof(T)); - std::array tempBuffer; - std::ranges::copy(sourceSpan, tempBuffer.begin()); + std::span charSpan { p, sizeof(T) }; + auto byteSpan = std::as_bytes(charSpan); + + std::array tempBuffer; + std::ranges::copy(byteSpan, tempBuffer.begin()); + ret = std::bit_cast(tempBuffer); p += sizeof(T); @@ -93,10 +96,11 @@ class PropStream { return false; } - std::vector tempBuffer(strLen); - std::span sourceSpan(reinterpret_cast(p), strLen); + std::vector tempBuffer(strLen); + std::span sourceSpan(p, strLen); std::ranges::copy(sourceSpan, tempBuffer.begin()); - ret.assign(reinterpret_cast(tempBuffer.data()), strLen); + + ret.assign(tempBuffer.begin(), tempBuffer.end()); p += strLen; From 5fa04cfa60ca8b2b99c685f13393ea8fc7ee8c25 Mon Sep 17 00:00:00 2001 From: Eduardo Dantas Date: Thu, 17 Oct 2024 21:43:32 -0300 Subject: [PATCH 3/3] improve: add static_assert and remove reinterpret cast Uses std::bit_cast intead of reinterpret cast --- src/io/fileloader.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/io/fileloader.hpp b/src/io/fileloader.hpp index 93c02502717..863837464ae 100644 --- a/src/io/fileloader.hpp +++ b/src/io/fileloader.hpp @@ -140,9 +140,11 @@ class PropWriteStream { template void write(T add) { - char* addr = reinterpret_cast(&add); - std::span sourceSpan(addr, sizeof(T)); - std::ranges::copy(sourceSpan, std::back_inserter(buffer)); + static_assert(std::is_trivially_copyable_v, "Type T must be trivially copyable"); + + auto byteArray = std::bit_cast>(add); + std::span charSpan(byteArray); + std::ranges::copy(charSpan, std::back_inserter(buffer)); } void writeString(const std::string &str) {