Storage Manager Update #685

Closed
muellerr wants to merge 11 commits from mueller/local-pool-update into development
8 changed files with 103 additions and 76 deletions
Showing only changes of commit ab7c3480f5 - Show all commits

View File

@ -19,7 +19,7 @@ ConstStorageAccessor::~ConstStorageAccessor() {
}
}
ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other)
ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other) noexcept
: constDataPointer(other.constDataPointer),
storeId(other.storeId),
size_(other.size_),
@ -30,7 +30,7 @@ ConstStorageAccessor::ConstStorageAccessor(ConstStorageAccessor&& other)
other.store = nullptr;
}
ConstStorageAccessor& ConstStorageAccessor::operator=(ConstStorageAccessor&& other) {
ConstStorageAccessor& ConstStorageAccessor::operator=(ConstStorageAccessor&& other) noexcept {
constDataPointer = other.constDataPointer;
storeId = other.storeId;
store = other.store;
@ -84,7 +84,7 @@ void ConstStorageAccessor::print() const {
arrayprinter::print(constDataPointer, size_);
}
void ConstStorageAccessor::assignStore(StorageManagerIF* store) {
void ConstStorageAccessor::assignStore(StorageManagerIF* store_) {
internalState = AccessState::ASSIGNED;
this->store = store;
store = store_;
}

View File

@ -23,6 +23,7 @@ class ConstStorageAccessor {
//! StorageManager classes have exclusive access to private variables.
friend class PoolManager;
friend class LocalPool;
friend class StorageManagerIF;
public:
/**
@ -30,7 +31,7 @@ class ConstStorageAccessor {
* entry to access.
* @param storeId
*/
ConstStorageAccessor(store_address_t storeId);
explicit ConstStorageAccessor(store_address_t storeId);
ConstStorageAccessor(store_address_t storeId, StorageManagerIF* store);
/**
@ -43,7 +44,7 @@ class ConstStorageAccessor {
* @brief Returns a pointer to the read-only data
* @return
*/
const uint8_t* data() const;
[[nodiscard]] const uint8_t* data() const;
/**
* @brief Copies the read-only data to the supplied pointer
@ -61,13 +62,13 @@ class ConstStorageAccessor {
* Get the size of the data
* @return
*/
size_t size() const;
[[nodiscard]] size_t size() const;
/**
* Get the storage ID.
* @return
*/
store_address_t getId() const;
[[nodiscard]] store_address_t getId() const;
void print() const;
@ -79,8 +80,8 @@ class ConstStorageAccessor {
* @param
* @return
*/
ConstStorageAccessor& operator=(ConstStorageAccessor&&);
ConstStorageAccessor(ConstStorageAccessor&&);
ConstStorageAccessor& operator=(ConstStorageAccessor&&) noexcept;
ConstStorageAccessor(ConstStorageAccessor&&) noexcept;
//! The copy ctor and copy assignemnt should be deleted implicitely
//! according to https://foonathan.net/2019/02/special-member-functions/

View File

@ -29,7 +29,7 @@ LocalPool::LocalPool(object_id_t setObjectId, const LocalPoolConfig& poolConfig,
}
}
LocalPool::~LocalPool(void) {}
LocalPool::~LocalPool() = default;
ReturnValue_t LocalPool::addData(store_address_t* storageId, const uint8_t* data, size_t size,
bool ignoreFault) {
@ -48,22 +48,6 @@ ReturnValue_t LocalPool::getData(store_address_t packetId, const uint8_t** packe
return status;
}
ReturnValue_t LocalPool::getData(store_address_t storeId, ConstStorageAccessor& storeAccessor) {
uint8_t* tempData = nullptr;
ReturnValue_t status = modifyData(storeId, &tempData, &storeAccessor.size_);
storeAccessor.assignStore(this);
storeAccessor.constDataPointer = tempData;
return status;
}
ConstAccessorPair LocalPool::getData(store_address_t storeId) {
uint8_t* tempData = nullptr;
ConstStorageAccessor constAccessor(storeId, this);
ReturnValue_t status = modifyData(storeId, &tempData, &constAccessor.size_);
constAccessor.constDataPointer = tempData;
return ConstAccessorPair(status, std::move(constAccessor));
}
ReturnValue_t LocalPool::getFreeElement(store_address_t* storageId, const size_t size,
uint8_t** pData, bool ignoreFault) {
ReturnValue_t status = reserveSpace(size, storageId, ignoreFault);
@ -75,20 +59,6 @@ ReturnValue_t LocalPool::getFreeElement(store_address_t* storageId, const size_t
return status;
}
AccessorPair LocalPool::modifyData(store_address_t storeId) {
StorageAccessor accessor(storeId, this);
ReturnValue_t status = modifyData(storeId, &accessor.dataPointer, &accessor.size_);
accessor.assignConstPointer();
return AccessorPair(status, std::move(accessor));
}
ReturnValue_t LocalPool::modifyData(store_address_t storeId, StorageAccessor& storeAccessor) {
storeAccessor.assignStore(this);
ReturnValue_t status = modifyData(storeId, &storeAccessor.dataPointer, &storeAccessor.size_);
storeAccessor.assignConstPointer();
return status;
}
ReturnValue_t LocalPool::modifyData(store_address_t storeId, uint8_t** packetPtr, size_t* size) {
ReturnValue_t status = returnvalue::FAILED;
if (storeId.poolIndex >= NUMBER_OF_SUBPOOLS) {
@ -197,8 +167,7 @@ void LocalPool::clearStore() {
}
}
ReturnValue_t LocalPool::reserveSpace(const size_t size, store_address_t* storeId,
bool ignoreFault) {
ReturnValue_t LocalPool::reserveSpace(size_t size, store_address_t* storeId, bool ignoreFault) {
ReturnValue_t status = getSubPoolIndex(size, &storeId->poolIndex);
if (status != returnvalue::OK) {
#if FSFW_CPP_OSTREAM_ENABLED == 1
@ -349,3 +318,27 @@ bool LocalPool::hasDataAtId(store_address_t storeId) const {
}
return false;
}
mohr marked this conversation as resolved Outdated
Outdated
Review

Why are these functions overwritten, calling the base class implementation which is already available per inheritance?

Why are these functions overwritten, calling the base class implementation which is already available per inheritance?

These need to "made available", either by re-exposing the IF calls like this or using
using StorageManagerIF::<functionName> in the header. I think the second way is better, will update the code to do it like that. Not doing any of those two will require the caller code to use a fully scoped function call, e.g. they must write myPool.StorageManagerIF::<functionCall>

These need to "made available", either by re-exposing the IF calls like this or using `using StorageManagerIF::<functionName>` in the header. I think the second way is better, will update the code to do it like that. Not doing any of those two will require the caller code to use a fully scoped function call, e.g. they must write `myPool.StorageManagerIF::<functionCall>`
ReturnValue_t LocalPool::getFreeElement(store_address_t* storeId, size_t size, uint8_t** pData) {
return StorageManagerIF::getFreeElement(storeId, size, pData);
}
ConstAccessorPair LocalPool::getData(store_address_t storeId) {
return StorageManagerIF::getData(storeId);
}
ReturnValue_t LocalPool::addData(store_address_t* storeId, const uint8_t* data, size_t size) {
return StorageManagerIF::addData(storeId, data, size);
}
ReturnValue_t LocalPool::getData(store_address_t storeId, ConstStorageAccessor& accessor) {
return StorageManagerIF::getData(storeId, accessor);
}
ReturnValue_t LocalPool::modifyData(store_address_t storeId, StorageAccessor& accessor) {
return StorageManagerIF::modifyData(storeId, accessor);
}
AccessorPair LocalPool::modifyData(store_address_t storeId) {
return StorageManagerIF::modifyData(storeId);
}

View File

@ -87,21 +87,23 @@ class LocalPool : public SystemObject, public StorageManagerIF {
* Documentation: See StorageManagerIF.h
*/
ReturnValue_t addData(store_address_t* storeId, const uint8_t* data, size_t size,
bool ignoreFault = false) override;
ReturnValue_t getFreeElement(store_address_t* storeId, const size_t size, uint8_t** pData,
bool ignoreFault = false) override;
bool ignoreFault) override;
ReturnValue_t addData(store_address_t* storeId, const uint8_t* data, size_t size) override;
ReturnValue_t getFreeElement(store_address_t* storeId, size_t size, uint8_t** pData) override;
ReturnValue_t getFreeElement(store_address_t* storeId, size_t size, uint8_t** pData,
bool ignoreFault) override;
ConstAccessorPair getData(store_address_t storeId) override;
ReturnValue_t getData(store_address_t storeId, ConstStorageAccessor& constAccessor) override;
ReturnValue_t getData(store_address_t storeId, ConstStorageAccessor& accessor) override;
ReturnValue_t getData(store_address_t storeId, const uint8_t** packet_ptr, size_t* size) override;
AccessorPair modifyData(store_address_t storeId) override;
ReturnValue_t modifyData(store_address_t storeId, StorageAccessor& storeAccessor) override;
ReturnValue_t modifyData(store_address_t storeId, uint8_t** packet_ptr, size_t* size) override;
ReturnValue_t modifyData(store_address_t storeId, StorageAccessor& accessor) override;
virtual ReturnValue_t deleteData(store_address_t storeId) override;
virtual ReturnValue_t deleteData(uint8_t* ptr, size_t size,
store_address_t* storeId = nullptr) override;
ReturnValue_t deleteData(store_address_t storeId) override;
ReturnValue_t deleteData(uint8_t* ptr, size_t size, store_address_t* storeId) override;
/**
* Get the total size of allocated memory for pool data.
@ -131,8 +133,8 @@ class LocalPool : public SystemObject, public StorageManagerIF {
* Get number sub pools. Each pool has pages with a specific bucket size.
* @return
*/
max_subpools_t getNumberOfSubPools() const override;
bool hasDataAtId(store_address_t storeId) const override;
[[nodiscard]] max_subpools_t getNumberOfSubPools() const override;
[[nodiscard]] bool hasDataAtId(store_address_t storeId) const override;
protected:
/**
@ -142,7 +144,7 @@ class LocalPool : public SystemObject, public StorageManagerIF {
* @return - returnvalue::OK on success,
* - the return codes of #getPoolIndex or #findEmpty otherwise.
*/
virtual ReturnValue_t reserveSpace(const size_t size, store_address_t* address, bool ignoreFault);
virtual ReturnValue_t reserveSpace(size_t size, store_address_t* address, bool ignoreFault);
private:
/**

View File

@ -27,7 +27,7 @@ class PoolManager : public LocalPool {
* @brief In the PoolManager's destructor all allocated memory
* is freed.
*/
virtual ~PoolManager();
~PoolManager() override;
/**
* Set the default mutex timeout for internal calls.
@ -40,8 +40,7 @@ class PoolManager : public LocalPool {
* which wraps LocalPool calls with a mutex protection.
*/
ReturnValue_t deleteData(store_address_t) override;
ReturnValue_t deleteData(uint8_t* buffer, size_t size,
store_address_t* storeId = nullptr) override;
ReturnValue_t deleteData(uint8_t* buffer, size_t size, store_address_t* storeId) override;
/**
* The developer is allowed to lock the mutex in case the lock needs
@ -58,8 +57,7 @@ class PoolManager : public LocalPool {
//! Default mutex timeout value to prevent permanent blocking.
uint32_t mutexTimeoutMs = 20;
ReturnValue_t reserveSpace(const size_t size, store_address_t* address,
bool ignoreFault) override;
ReturnValue_t reserveSpace(size_t size, store_address_t* address, bool ignoreFault) override;
/**
* @brief The mutex is created in the constructor and makes

View File

@ -10,7 +10,7 @@ StorageAccessor::StorageAccessor(store_address_t storeId) : ConstStorageAccessor
StorageAccessor::StorageAccessor(store_address_t storeId, StorageManagerIF* store)
: ConstStorageAccessor(storeId, store) {}
StorageAccessor& StorageAccessor::operator=(StorageAccessor&& other) {
StorageAccessor& StorageAccessor::operator=(StorageAccessor&& other) noexcept {
// Call the parent move assignment and also assign own member.
dataPointer = other.dataPointer;
ConstStorageAccessor::operator=(std::move(other));
@ -18,7 +18,7 @@ StorageAccessor& StorageAccessor::operator=(StorageAccessor&& other) {
}
// Call the parent move ctor and also transfer own member.
StorageAccessor::StorageAccessor(StorageAccessor&& other)
StorageAccessor::StorageAccessor(StorageAccessor&& other) noexcept
: ConstStorageAccessor(std::move(other)), dataPointer(other.dataPointer) {}
ReturnValue_t StorageAccessor::getDataCopy(uint8_t* pointer, size_t maxSize) {

View File

@ -12,9 +12,10 @@ class StorageAccessor : public ConstStorageAccessor {
//! StorageManager classes have exclusive access to private variables.
friend class PoolManager;
friend class LocalPool;
friend class StorageManagerIF;
public:
StorageAccessor(store_address_t storeId);
explicit StorageAccessor(store_address_t storeId);
StorageAccessor(store_address_t storeId, StorageManagerIF* store);
/**
@ -25,8 +26,8 @@ class StorageAccessor : public ConstStorageAccessor {
* @param
* @return
*/
StorageAccessor& operator=(StorageAccessor&&);
StorageAccessor(StorageAccessor&&);
StorageAccessor& operator=(StorageAccessor&&) noexcept;
StorageAccessor(StorageAccessor&&) noexcept;
ReturnValue_t write(uint8_t* data, size_t size, uint16_t offset = 0);
uint8_t* data();

View File

@ -67,7 +67,12 @@ class StorageManagerIF {
* @returnvalue::FAILED if data could not be added, storageId is unchanged then.
*/
virtual ReturnValue_t addData(store_address_t* storageId, const uint8_t* data, size_t size,
bool ignoreFault = false) = 0;
bool ignoreFault) = 0;
virtual ReturnValue_t addData(store_address_t* storageId, const uint8_t* data, size_t size) {
return addData(storageId, data, size, false);
}
/**
* @brief With deleteData, the storageManager frees the memory region
* identified by packet_id.
@ -86,9 +91,10 @@ class StorageManagerIF {
* @return @li returnvalue::OK on success.
* @li failure code if deletion did not work
*/
virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size,
store_address_t* storeId = nullptr) = 0;
virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size, store_address_t* storeId) = 0;
virtual ReturnValue_t deleteData(uint8_t* buffer, size_t size) {
return deleteData(buffer, size, nullptr);
}
/**
* @brief Access the data by supplying a store ID.
* @details
@ -97,7 +103,13 @@ class StorageManagerIF {
* @param storeId
* @return Pair of return value and a ConstStorageAccessor instance
*/
virtual ConstAccessorPair getData(store_address_t storeId) = 0;
virtual ConstAccessorPair getData(store_address_t storeId) {
uint8_t* tempData = nullptr;
ConstStorageAccessor constAccessor(storeId, this);
ReturnValue_t status = modifyData(storeId, &tempData, &constAccessor.size_);
constAccessor.constDataPointer = tempData;
return {status, std::move(constAccessor)};
}
/**
* @brief Access the data by supplying a store ID and a helper
@ -106,7 +118,13 @@ class StorageManagerIF {
* @param constAccessor Wrapper function to access store data.
* @return
*/
virtual ReturnValue_t getData(store_address_t storeId, ConstStorageAccessor& constAccessor) = 0;
virtual ReturnValue_t getData(store_address_t storeId, ConstStorageAccessor& accessor) {
uint8_t* tempData = nullptr;
ReturnValue_t status = modifyData(storeId, &tempData, &accessor.size_);
accessor.assignStore(this);
accessor.constDataPointer = tempData;
return status;
}
/**
* @brief getData returns an address to data and the size of the data
@ -127,7 +145,12 @@ class StorageManagerIF {
* @param storeId
* @return Pair of return value and StorageAccessor helper
*/
virtual AccessorPair modifyData(store_address_t storeId) = 0;
virtual AccessorPair modifyData(store_address_t storeId) {
StorageAccessor accessor(storeId, this);
ReturnValue_t status = modifyData(storeId, &accessor.dataPointer, &accessor.size_);
accessor.assignConstPointer();
return {status, std::move(accessor)};
}
/**
* Modify data by supplying a store ID and a StorageAccessor helper instance.
@ -135,7 +158,12 @@ class StorageManagerIF {
* @param accessor Helper class to access the modifiable data.
* @return
*/
virtual ReturnValue_t modifyData(store_address_t storeId, StorageAccessor& accessor) = 0;
virtual ReturnValue_t modifyData(store_address_t storeId, StorageAccessor& accessor) {
accessor.assignStore(this);
ReturnValue_t status = modifyData(storeId, &accessor.dataPointer, &accessor.size_);
accessor.assignConstPointer();
return status;
}
/**
* Get pointer and size of modifiable data by supplying the storeId
@ -154,12 +182,16 @@ class StorageManagerIF {
* written to p_data!
* @param storageId A pointer to the storageId to retrieve.
* @param size The size of the space to be reserved.
* @param p_data A pointer to the element data is returned here.
* @return Returns @li returnvalue::OK if data was added.
* @param dataPtr A pointer to the element data is returned here.
* @return Returns @returnvalue::OK if data was added.
* @returnvalue::FAILED if data could not be added, storageId is unchanged then.
*/
virtual ReturnValue_t getFreeElement(store_address_t* storageId, size_t size, uint8_t** p_data,
bool ignoreFault = false) = 0;
virtual ReturnValue_t getFreeElement(store_address_t* storageId, size_t size, uint8_t** dataPtr,
bool ignoreFault) = 0;
virtual ReturnValue_t getFreeElement(store_address_t* storageId, size_t size, uint8_t** dataPtr) {
return getFreeElement(storageId, size, dataPtr, false);
}
[[nodiscard]] virtual bool hasDataAtId(store_address_t storeId) const = 0;