From 9040d7813d25926e98163647846105d5fe3fa535 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Fri, 8 Sep 2023 20:34:20 +0100 Subject: [PATCH] Codechange: Use std::array and std::unique_ptr for PersistentStorageArrays. This (mostly) avoids the need for manual memory management and copying. --- src/newgrf_storage.h | 36 +++++++++--------------------------- src/saveload/industry_sl.cpp | 2 +- src/saveload/station_sl.cpp | 2 +- 3 files changed, 11 insertions(+), 29 deletions(-) diff --git a/src/newgrf_storage.h b/src/newgrf_storage.h index 1461581fd2..7ea531443b 100644 --- a/src/newgrf_storage.h +++ b/src/newgrf_storage.h @@ -65,26 +65,10 @@ private: */ template struct PersistentStorageArray : BasePersistentStorageArray { - TYPE storage[SIZE]; ///< Memory to for the storage array - TYPE *prev_storage; ///< Memory to store "old" states so we can revert them on the performance of test cases for commands etc. + using StorageType = std::array; - /** Simply construct the array */ - PersistentStorageArray() : prev_storage(nullptr) - { - memset(this->storage, 0, sizeof(this->storage)); - } - - /** And free all data related to it */ - ~PersistentStorageArray() - { - free(this->prev_storage); - } - - /** Resets all values to zero. */ - void ResetToZero() - { - memset(this->storage, 0, sizeof(this->storage)); - } + StorageType storage{}; ///< Memory for the storage array + std::unique_ptr prev_storage{}; ///< Temporary memory to store previous state so it can be reverted, e.g. for command tests. /** * Stores some value at a given position. @@ -104,10 +88,9 @@ struct PersistentStorageArray : BasePersistentStorageArray { /* We do not have made a backup; lets do so */ if (AreChangesPersistent()) { - assert(this->prev_storage == nullptr); - } else if (this->prev_storage == nullptr) { - this->prev_storage = MallocT(SIZE); - memcpy(this->prev_storage, this->storage, sizeof(this->storage)); + assert(!this->prev_storage); + } else if (!this->prev_storage) { + this->prev_storage = std::make_unique(this->storage); /* We only need to register ourselves when we made the backup * as that is the only time something will have changed */ @@ -132,10 +115,9 @@ struct PersistentStorageArray : BasePersistentStorageArray { void ClearChanges() { - if (this->prev_storage != nullptr) { - memcpy(this->storage, this->prev_storage, sizeof(this->storage)); - free(this->prev_storage); - this->prev_storage = nullptr; + if (this->prev_storage) { + this->storage = *this->prev_storage; + this->prev_storage.reset(); } } }; diff --git a/src/saveload/industry_sl.cpp b/src/saveload/industry_sl.cpp index 017ab89bec..1c0b79f5b3 100644 --- a/src/saveload/industry_sl.cpp +++ b/src/saveload/industry_sl.cpp @@ -246,7 +246,7 @@ struct INDYChunkHandler : ChunkHandler { /* Store the old persistent storage. The GRFID will be added later. */ assert(PersistentStorage::CanAllocateItem()); i->psa = new PersistentStorage(0, 0, 0); - memcpy(i->psa->storage, _old_ind_persistent_storage.storage, sizeof(_old_ind_persistent_storage.storage)); + std::copy(std::begin(_old_ind_persistent_storage.storage), std::end(_old_ind_persistent_storage.storage), std::begin(i->psa->storage)); } if (IsSavegameVersionBefore(SLV_INDUSTRY_CARGO_REORGANISE)) LoadMoveAcceptsProduced(i); Industry::IncIndustryTypeCount(i->type); diff --git a/src/saveload/station_sl.cpp b/src/saveload/station_sl.cpp index 76ef03dee4..3ae0dabc5f 100644 --- a/src/saveload/station_sl.cpp +++ b/src/saveload/station_sl.cpp @@ -426,7 +426,7 @@ public: /* Store the old persistent storage. The GRFID will be added later. */ assert(PersistentStorage::CanAllocateItem()); st->airport.psa = new PersistentStorage(0, 0, 0); - memcpy(st->airport.psa->storage, _old_st_persistent_storage.storage, sizeof(_old_st_persistent_storage.storage)); + std::copy(std::begin(_old_st_persistent_storage.storage), std::end(_old_st_persistent_storage.storage), std::begin(st->airport.psa->storage)); } size_t num_cargo = this->GetNumCargo();