From 8c722feafb18f15e5e1db3ddde65ae66a3b09dad Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 11 Aug 2020 16:21:59 +0200 Subject: [PATCH 1/4] Fixes #47 and #36 --- container/ArrayList.h | 8 ++- container/FixedMap.h | 23 ++++----- container/FixedOrderedMultimap.h | 13 +---- subsystem/Subsystem.cpp | 4 +- tmtcservices/CommandingServiceBase.cpp | 70 +++++++++++++------------- 5 files changed, 54 insertions(+), 64 deletions(-) diff --git a/container/ArrayList.h b/container/ArrayList.h index 9c4c4ceb..0a2f460b 100644 --- a/container/ArrayList.h +++ b/container/ArrayList.h @@ -72,11 +72,15 @@ public: return tmp; } - T operator*() { + T& operator*(){ return *value; } - T *operator->() { + const T& operator*() const{ + return *value; + } + + T *operator->(){ return value; } diff --git a/container/FixedMap.h b/container/FixedMap.h index ac170bd2..0823b670 100644 --- a/container/FixedMap.h +++ b/container/FixedMap.h @@ -1,5 +1,5 @@ -#ifndef FIXEDMAP_H_ -#define FIXEDMAP_H_ +#ifndef FRAMEWORK_CONTAINER_FIXEDMAP_H_ +#define FRAMEWORK_CONTAINER_FIXEDMAP_H_ #include #include @@ -7,6 +7,10 @@ /** * \ingroup container + * + * + * \warning Iterators return a non-const key_t in the pair. + * \warning A User is not allowed to change the key, otherwise the map is corrupted. */ template class FixedMap: public SerializeIF { @@ -47,15 +51,6 @@ public: Iterator(std::pair *pair) : ArrayList, uint32_t>::Iterator(pair) { } - - T operator*() { - return ArrayList, uint32_t>::Iterator::value->second; - } - - T *operator->() { - return &ArrayList, uint32_t>::Iterator::value->second; - } - }; Iterator begin() const { @@ -70,7 +65,7 @@ public: return _size; } - ReturnValue_t insert(key_t key, T value, Iterator *storedValue = NULL) { + ReturnValue_t insert(key_t key, T value, Iterator *storedValue = nullptr) { if (exists(key) == HasReturnvaluesIF::RETURN_OK) { return KEY_ALREADY_EXISTS; } @@ -79,7 +74,7 @@ public: } theMap[_size].first = key; theMap[_size].second = value; - if (storedValue != NULL) { + if (storedValue != nullptr) { *storedValue = Iterator(&theMap[_size]); } ++_size; @@ -87,7 +82,7 @@ public: } ReturnValue_t insert(std::pair pair) { - return insert(pair.fist, pair.second); + return insert(pair.first, pair.second); } ReturnValue_t exists(key_t key) const { diff --git a/container/FixedOrderedMultimap.h b/container/FixedOrderedMultimap.h index 21629664..4f81c97c 100644 --- a/container/FixedOrderedMultimap.h +++ b/container/FixedOrderedMultimap.h @@ -68,15 +68,6 @@ public: Iterator(std::pair *pair) : ArrayList, uint32_t>::Iterator(pair) { } - - T operator*() { - return ArrayList, uint32_t>::Iterator::value->second; - } - - T *operator->() { - return &ArrayList, uint32_t>::Iterator::value->second; - } - }; Iterator begin() const { @@ -91,7 +82,7 @@ public: return _size; } - ReturnValue_t insert(key_t key, T value, Iterator *storedValue = NULL) { + ReturnValue_t insert(key_t key, T value, Iterator *storedValue = nullptr) { if (_size == theMap.maxSize()) { return MAP_FULL; } @@ -101,7 +92,7 @@ public: theMap[position].first = key; theMap[position].second = value; ++_size; - if (storedValue != NULL) { + if (storedValue != nullptr) { *storedValue = Iterator(&theMap[position]); } return HasReturnvaluesIF::RETURN_OK; diff --git a/subsystem/Subsystem.cpp b/subsystem/Subsystem.cpp index fcf2e189..094e79b4 100644 --- a/subsystem/Subsystem.cpp +++ b/subsystem/Subsystem.cpp @@ -549,7 +549,7 @@ Mode_t Subsystem::getFallbackSequence(Mode_t sequence) { for (FixedMap::Iterator iter = modeSequences.begin(); iter != modeSequences.end(); ++iter) { if (iter.value->first == sequence) { - return iter->fallbackSequence; + return iter->second.fallbackSequence; } } return -1; @@ -558,7 +558,7 @@ Mode_t Subsystem::getFallbackSequence(Mode_t sequence) { bool Subsystem::isFallbackSequence(Mode_t SequenceId) { for (FixedMap::Iterator iter = modeSequences.begin(); iter != modeSequences.end(); iter++) { - if (iter->fallbackSequence == SequenceId) { + if (iter->second.fallbackSequence == SequenceId) { return true; } } diff --git a/tmtcservices/CommandingServiceBase.cpp b/tmtcservices/CommandingServiceBase.cpp index 331bc7c8..b08e315d 100644 --- a/tmtcservices/CommandingServiceBase.cpp +++ b/tmtcservices/CommandingServiceBase.cpp @@ -104,8 +104,8 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { // Implemented by child class, specifies what to do with reply. - ReturnValue_t result = handleReply(reply, iter->command, &iter->state, - &nextCommand, iter->objectId, &isStep); + ReturnValue_t result = handleReply(reply, iter->second.command, &iter->second.state, + &nextCommand, iter->second.objectId, &isStep); /* If the child implementation does not implement special handling for * rejected replies (RETURN_FAILED is returned), a failure verification @@ -114,7 +114,7 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { if(reply->getCommand() == CommandMessage::REPLY_REJECTED and result == RETURN_FAILED) { result = reply->getReplyRejectedReason(); - failureParameter1 = iter->command; + failureParameter1 = iter->second.command; } switch (result) { @@ -131,14 +131,14 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { default: if (isStep) { verificationReporter.sendFailureReport( - TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, - iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, - result, ++iter->step, failureParameter1, + TC_VERIFY::PROGRESS_FAILURE, iter->second.tcInfo.ackFlags, + iter->second.tcInfo.tcPacketId, iter->second.tcInfo.tcSequenceControl, + result, ++iter->second.step, failureParameter1, failureParameter2); } else { verificationReporter.sendFailureReport( - TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, - iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, + TC_VERIFY::COMPLETION_FAILURE, iter->second.tcInfo.ackFlags, + iter->second.tcInfo.tcPacketId, iter->second.tcInfo.tcSequenceControl, result, 0, failureParameter1, failureParameter2); } failureParameter1 = 0; @@ -152,7 +152,7 @@ void CommandingServiceBase::handleCommandMessage(CommandMessage* reply) { void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, CommandMapIter iter, CommandMessage* nextCommand, CommandMessage* reply, bool& isStep) { - iter->command = nextCommand->getCommand(); + iter->second.command = nextCommand->getCommand(); // In case a new command is to be sent immediately, this is performed here. // If no new command is sent, only analyse reply result by initializing @@ -167,14 +167,14 @@ void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, if (isStep and result != NO_STEP_MESSAGE) { verificationReporter.sendSuccessReport( TC_VERIFY::PROGRESS_SUCCESS, - iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, ++iter->step); + iter->second.tcInfo.ackFlags, iter->second.tcInfo.tcPacketId, + iter->second.tcInfo.tcSequenceControl, ++iter->second.step); } else { verificationReporter.sendSuccessReport( TC_VERIFY::COMPLETION_SUCCESS, - iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, 0); + iter->second.tcInfo.ackFlags, iter->second.tcInfo.tcPacketId, + iter->second.tcInfo.tcSequenceControl, 0); checkAndExecuteFifo(iter); } } @@ -182,16 +182,16 @@ void CommandingServiceBase::handleReplyHandlerResult(ReturnValue_t result, if (isStep) { nextCommand->clearCommandMessage(); verificationReporter.sendFailureReport( - TC_VERIFY::PROGRESS_FAILURE, iter->tcInfo.ackFlags, - iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, sendResult, - ++iter->step, failureParameter1, failureParameter2); + TC_VERIFY::PROGRESS_FAILURE, iter->second.tcInfo.ackFlags, + iter->second.tcInfo.tcPacketId, + iter->second.tcInfo.tcSequenceControl, sendResult, + ++iter->second.step, failureParameter1, failureParameter2); } else { nextCommand->clearCommandMessage(); verificationReporter.sendFailureReport( TC_VERIFY::COMPLETION_FAILURE, - iter->tcInfo.ackFlags, iter->tcInfo.tcPacketId, - iter->tcInfo.tcSequenceControl, sendResult, 0, + iter->second.tcInfo.ackFlags, iter->second.tcInfo.tcPacketId, + iter->second.tcInfo.tcSequenceControl, sendResult, 0, failureParameter1, failureParameter2); } failureParameter1 = 0; @@ -230,7 +230,7 @@ void CommandingServiceBase::handleRequestQueue() { iter = commandMap.find(queue); if (iter != commandMap.end()) { - result = iter->fifo.insert(address); + result = iter->second.fifo.insert(address); if (result != RETURN_OK) { rejectPacket(TC_VERIFY::START_FAILURE, &packet, OBJECT_BUSY); } @@ -298,11 +298,11 @@ void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, CommandMapIter iter) { ReturnValue_t result = RETURN_OK; CommandMessage command; - iter->subservice = storedPacket->getSubService(); - result = prepareCommand(&command, iter->subservice, + iter->second.subservice = storedPacket->getSubService(); + result = prepareCommand(&command, iter->second.subservice, storedPacket->getApplicationData(), - storedPacket->getApplicationDataSize(), &iter->state, - iter->objectId); + storedPacket->getApplicationDataSize(), &iter->second.state, + iter->second.objectId); ReturnValue_t sendResult = RETURN_OK; switch (result) { @@ -312,13 +312,13 @@ void CommandingServiceBase::startExecution(TcPacketStored *storedPacket, &command); } if (sendResult == RETURN_OK) { - Clock::getUptime(&iter->uptimeOfStart); - iter->step = 0; - iter->subservice = storedPacket->getSubService(); - iter->command = command.getCommand(); - iter->tcInfo.ackFlags = storedPacket->getAcknowledgeFlags(); - iter->tcInfo.tcPacketId = storedPacket->getPacketId(); - iter->tcInfo.tcSequenceControl = + Clock::getUptime(&iter->second.uptimeOfStart); + iter->second.step = 0; + iter->second.subservice = storedPacket->getSubService(); + iter->second.command = command.getCommand(); + iter->second.tcInfo.ackFlags = storedPacket->getAcknowledgeFlags(); + iter->second.tcInfo.tcPacketId = storedPacket->getPacketId(); + iter->second.tcInfo.tcSequenceControl = storedPacket->getPacketSequenceControl(); acceptPacket(TC_VERIFY::START_SUCCESS, storedPacket); } else { @@ -368,7 +368,7 @@ void CommandingServiceBase::acceptPacket(uint8_t reportId, void CommandingServiceBase::checkAndExecuteFifo(CommandMapIter iter) { store_address_t address; - if (iter->fifo.retrieve(&address) != RETURN_OK) { + if (iter->second.fifo.retrieve(&address) != RETURN_OK) { commandMap.erase(&iter); } else { TcPacketStored newPacket(address); @@ -394,10 +394,10 @@ void CommandingServiceBase::checkTimeout() { Clock::getUptime(&uptime); CommandMapIter iter; for (iter = commandMap.begin(); iter != commandMap.end(); ++iter) { - if ((iter->uptimeOfStart + (timeoutSeconds * 1000)) < uptime) { + if ((iter->second.uptimeOfStart + (timeoutSeconds * 1000)) < uptime) { verificationReporter.sendFailureReport( - TC_VERIFY::COMPLETION_FAILURE, iter->tcInfo.ackFlags, - iter->tcInfo.tcPacketId, iter->tcInfo.tcSequenceControl, + TC_VERIFY::COMPLETION_FAILURE, iter->second.tcInfo.ackFlags, + iter->second.tcInfo.tcPacketId, iter->second.tcInfo.tcSequenceControl, TIMEOUT); checkAndExecuteFifo(iter); } From e963aca02a343a8afa46e5ef0103307b1b87248d Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Tue, 11 Aug 2020 17:02:19 +0200 Subject: [PATCH 2/4] Fixed an issue with memmove in FixedOrderedMultimap --- container/FixedOrderedMultimap.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/container/FixedOrderedMultimap.h b/container/FixedOrderedMultimap.h index 4f81c97c..4ddc5dcb 100644 --- a/container/FixedOrderedMultimap.h +++ b/container/FixedOrderedMultimap.h @@ -48,7 +48,7 @@ private: if (_size <= position) { return; } - memmove(&theMap[position], &theMap[position + 1], + memmove(static_cast(&theMap[position]), static_cast(&theMap[position + 1]), (_size - position - 1) * sizeof(std::pair)); --_size; } @@ -87,7 +87,7 @@ public: return MAP_FULL; } uint32_t position = findNicePlace(key); - memmove(&theMap[position + 1], &theMap[position], + memmove(static_cast(&theMap[position + 1]),static_cast(&theMap[position]), (_size - position) * sizeof(std::pair)); theMap[position].first = key; theMap[position].second = value; From b484e4d5ad7325597fa42905fdb984ec145a475e Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Thu, 13 Aug 2020 10:27:16 +0200 Subject: [PATCH 3/4] Removed unused function --- container/FixedOrderedMultimap.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/container/FixedOrderedMultimap.h b/container/FixedOrderedMultimap.h index 4ddc5dcb..872425e6 100644 --- a/container/FixedOrderedMultimap.h +++ b/container/FixedOrderedMultimap.h @@ -136,12 +136,6 @@ public: return HasReturnvaluesIF::RETURN_OK; } - //This is potentially unsafe -// T *findValue(key_t key) const { -// return &theMap[findFirstIndex(key)].second; -// } - - Iterator find(key_t key) const { ReturnValue_t result = exists(key); if (result != HasReturnvaluesIF::RETURN_OK) { From e7444912d5eb69dc701b12f7de5f94d730d2992d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robin=20M=C3=BCller?= Date: Thu, 10 Sep 2020 15:29:19 +0200 Subject: [PATCH 4/4] corrected include guard --- container/FixedMap.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/container/FixedMap.h b/container/FixedMap.h index 56a6140a..d1cc31ff 100644 --- a/container/FixedMap.h +++ b/container/FixedMap.h @@ -1,5 +1,5 @@ -#ifndef FRAMEWORK_CONTAINER_FIXEDMAP_H_ -#define FRAMEWORK_CONTAINER_FIXEDMAP_H_ +#ifndef FSFW_CONTAINER_FIXEDMAP_H_ +#define FSFW_CONTAINER_FIXEDMAP_H_ #include "ArrayList.h" #include "../returnvalues/HasReturnvaluesIF.h" @@ -7,11 +7,9 @@ #include /** - * \ingroup container - * - * - * \warning Iterators return a non-const key_t in the pair. - * \warning A User is not allowed to change the key, otherwise the map is corrupted. + * @warning Iterators return a non-const key_t in the pair. + * @warning A User is not allowed to change the key, otherwise the map is corrupted. + * @ingroup container */ template class FixedMap: public SerializeIF { @@ -194,4 +192,4 @@ public: }; -#endif /* FIXEDMAP_H_ */ +#endif /* FSFW_CONTAINER_FIXEDMAP_H_ */