From 2a28114b49fc48d01e95bb350d8c52d299bcab5c Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Mon, 7 Sep 2020 15:43:48 +0200 Subject: [PATCH 1/4] Fixes for #190 and #189 --- container/DynamicFIFO.h | 16 ++++++++++++++-- container/FIFO.h | 12 ++++++++++++ container/FixedMap.h | 3 +++ tmtcservices/CommandingServiceBase.h | 17 +++++++++++++++-- 4 files changed, 44 insertions(+), 4 deletions(-) diff --git a/container/DynamicFIFO.h b/container/DynamicFIFO.h index abb533308..8f5c5e40d 100644 --- a/container/DynamicFIFO.h +++ b/container/DynamicFIFO.h @@ -27,14 +27,26 @@ public: /** * @brief Custom copy constructor which prevents setting the - * underlying pointer wrong. + * underlying pointer wrong. This function allocates memory! + * @details This is a very heavy operation so try to avoid this! + * */ DynamicFIFO(const DynamicFIFO& other): FIFOBase(other), fifoVector(other.maxCapacity) { this->setContainer(fifoVector.data()); } - + /** + * @brief Custom assignment operator + * @details This is a very heavy operation so try to avoid this! + * @param other DyamicFIFO to copy from + */ + DynamicFIFO& operator=(const DynamicFIFO& other){ + FIFOBase::operator=(other); + this->fifoVector = other.fifoVector; + this->setContainer(fifoVector.data()); + return *this; + } private: std::vector fifoVector; }; diff --git a/container/FIFO.h b/container/FIFO.h index 19f763fc8..701138520 100644 --- a/container/FIFO.h +++ b/container/FIFO.h @@ -25,9 +25,21 @@ public: * @param other */ FIFO(const FIFO& other): FIFOBase(other) { + this->fifoArray = other.fifoArray; this->setContainer(fifoArray.data()); } + /** + * @brief Custom assignment operator + * @param other + */ + FIFO& operator=(const FIFO& other){ + FIFOBase::operator=(other); + this->fifoArray = other.fifoArray; + this->setContainer(fifoArray.data()); + return *this; + } + private: std::array fifoArray; }; diff --git a/container/FixedMap.h b/container/FixedMap.h index dc19ce539..65459631f 100644 --- a/container/FixedMap.h +++ b/container/FixedMap.h @@ -4,12 +4,15 @@ #include "ArrayList.h" #include "../returnvalues/HasReturnvaluesIF.h" #include +#include /** * \ingroup container */ template class FixedMap: public SerializeIF { + static_assert (std::is_trivially_copyable::value or std::is_base_of::value, + "Types used in FixedMap must either be trivial copy-able or a derived Class from SerializeIF to be serialize-able"); public: static const uint8_t INTERFACE_ID = CLASS_ID::FIXED_MAP; static const ReturnValue_t KEY_ALREADY_EXISTS = MAKE_RETURN_CODE(0x01); diff --git a/tmtcservices/CommandingServiceBase.h b/tmtcservices/CommandingServiceBase.h index 864a0614b..23b08acf1 100644 --- a/tmtcservices/CommandingServiceBase.h +++ b/tmtcservices/CommandingServiceBase.h @@ -211,8 +211,7 @@ protected: virtual void doPeriodicOperation(); - - struct CommandInfo { + struct CommandInfo: public SerializeIF{ struct tcInfo { uint8_t ackFlags; uint16_t tcPacketId; @@ -225,6 +224,20 @@ protected: Command_t command; object_id_t objectId; FIFO fifo; + + virtual ReturnValue_t serialize(uint8_t **buffer, size_t *size, + size_t maxSize, Endianness streamEndianness) const override{ + return HasReturnvaluesIF::RETURN_FAILED; + }; + + virtual size_t getSerializedSize() const override { + return 0; + }; + + virtual ReturnValue_t deSerialize(const uint8_t **buffer, size_t *size, + Endianness streamEndianness) override{ + return HasReturnvaluesIF::RETURN_FAILED; + }; }; using CommandMapIter = FixedMap Date: Mon, 7 Sep 2020 15:49:28 +0200 Subject: [PATCH 2/4] Another fix in DynamicFIFO --- container/DynamicFIFO.h | 1 + 1 file changed, 1 insertion(+) diff --git a/container/DynamicFIFO.h b/container/DynamicFIFO.h index 8f5c5e40d..86d43f7d7 100644 --- a/container/DynamicFIFO.h +++ b/container/DynamicFIFO.h @@ -33,6 +33,7 @@ public: */ DynamicFIFO(const DynamicFIFO& other): FIFOBase(other), fifoVector(other.maxCapacity) { + this->fifoVector = other.fifoVector; this->setContainer(fifoVector.data()); } From b65789824cc103d8fbc46479e6eff0f09c6346d6 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Mon, 7 Sep 2020 18:06:08 +0200 Subject: [PATCH 3/4] Removed old hacks and replaced by standard c++ Added overflow check in SerializeAdapter --- container/IsDerivedFrom.h | 41 ------------------------------------ serialize/SerializeAdapter.h | 32 ++++++++++++++++------------ 2 files changed, 18 insertions(+), 55 deletions(-) delete mode 100644 container/IsDerivedFrom.h diff --git a/container/IsDerivedFrom.h b/container/IsDerivedFrom.h deleted file mode 100644 index 520033dbd..000000000 --- a/container/IsDerivedFrom.h +++ /dev/null @@ -1,41 +0,0 @@ -#ifndef ISDERIVEDFROM_H_ -#define ISDERIVEDFROM_H_ - -template -class IsDerivedFrom { - class No { - }; - class Yes { - No no[3]; - }; - - static Yes Test(B*); // declared, but not defined - static No Test(... ); // declared, but not defined - -public: - enum { - Is = sizeof(Test(static_cast(0))) == sizeof(Yes) - }; -}; - -template -struct is_same { - static bool const value = false; -}; - -template -struct is_same { - static bool const value = true; -}; - - -template -struct enable_if { - typedef T type; -}; - -template -struct enable_if { }; - - -#endif /* ISDERIVEDFROM_H_ */ diff --git a/serialize/SerializeAdapter.h b/serialize/SerializeAdapter.h index af5ec116a..cd6b6b064 100644 --- a/serialize/SerializeAdapter.h +++ b/serialize/SerializeAdapter.h @@ -1,11 +1,11 @@ #ifndef SERIALIZEADAPTER_H_ #define SERIALIZEADAPTER_H_ -#include "../container/IsDerivedFrom.h" #include "../returnvalues/HasReturnvaluesIF.h" #include "EndianConverter.h" #include "SerializeIF.h" #include +#include /** * \ingroup serialize @@ -15,34 +15,39 @@ class SerializeAdapter { public: template static ReturnValue_t serialize(const T *object, uint8_t **buffer, - size_t *size, size_t maxSize, SerializeIF::Endianness streamEndianness) { - InternalSerializeAdapter::Is> adapter; + size_t *size, size_t maxSize, + SerializeIF::Endianness streamEndianness) { + InternalSerializeAdapter::value> adapter; return adapter.serialize(object, buffer, size, maxSize, streamEndianness); } template static uint32_t getSerializedSize(const T *object) { - InternalSerializeAdapter::Is> adapter; + InternalSerializeAdapter::value> adapter; return adapter.getSerializedSize(object); } template static ReturnValue_t deSerialize(T *object, const uint8_t **buffer, size_t *size, SerializeIF::Endianness streamEndianness) { - InternalSerializeAdapter::Is> adapter; + InternalSerializeAdapter::value> adapter; return adapter.deSerialize(object, buffer, size, streamEndianness); } private: - template - class InternalSerializeAdapter { + template class InternalSerializeAdapter; + + template + class InternalSerializeAdapter { public: static ReturnValue_t serialize(const T *object, uint8_t **buffer, - size_t *size, size_t max_size, SerializeIF::Endianness streamEndianness) { + size_t *size, size_t max_size, + SerializeIF::Endianness streamEndianness) { size_t ignoredSize = 0; if (size == NULL) { size = &ignoredSize; } - //TODO check integer overflow of *size - if (sizeof(T) + *size <= max_size) { + //Check remaining size is large enough and check integer overflow of *size + size_t newSize = sizeof(T) + *size; + if ((newSize <= max_size) and (newSize > *size)) { T tmp; switch (streamEndianness) { case SerializeIF::Endianness::BIG: @@ -94,14 +99,13 @@ private: uint32_t getSerializedSize(const T *object) { return sizeof(T); } - }; template - class InternalSerializeAdapter { + class InternalSerializeAdapter { public: - ReturnValue_t serialize(const T *object, uint8_t **buffer, - size_t *size, size_t max_size, + ReturnValue_t serialize(const T *object, uint8_t **buffer, size_t *size, + size_t max_size, SerializeIF::Endianness streamEndianness) const { size_t ignoredSize = 0; if (size == NULL) { From 0258ce62f0ccb64be8f87e17321627171601f348 Mon Sep 17 00:00:00 2001 From: Steffen Gaisser Date: Mon, 7 Sep 2020 18:35:25 +0200 Subject: [PATCH 4/4] Added documentation --- serialize/SerializeAdapter.h | 71 ++++++++++++++++++++++++++++++++---- serialize/SerializeIF.h | 2 +- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/serialize/SerializeAdapter.h b/serialize/SerializeAdapter.h index cd6b6b064..a05b17916 100644 --- a/serialize/SerializeAdapter.h +++ b/serialize/SerializeAdapter.h @@ -1,10 +1,10 @@ -#ifndef SERIALIZEADAPTER_H_ -#define SERIALIZEADAPTER_H_ +#ifndef _FSFW_SERIALIZE_SERIALIZEADAPTER_H_ +#define _FSFW_SERIALIZE_SERIALIZEADAPTER_H_ #include "../returnvalues/HasReturnvaluesIF.h" #include "EndianConverter.h" #include "SerializeIF.h" -#include +#include #include /** @@ -13,6 +13,22 @@ class SerializeAdapter { public: + /*** + * This function can be used to serialize a trivial copy-able type or a child of SerializeIF. + * The right template to be called is determined in the function itself. + * For objects of non trivial copy-able type this function is almost never called by the user directly. + * Instead helpers for specific types like SerialArrayListAdapter or SerialLinkedListAdapter is the right choice here. + * + * @param[in] object Object to serialize, the used type is deduced from this pointer + * @param[in/out] buffer Buffer to serialize into. Will be moved by the function. + * @param[in/out] size Size of current written buffer. Will be incremented by the function. + * @param[in] maxSize Max size of Buffer + * @param[in] streamEndianness Endianness of serialized element as in according to SerializeIF::Endianness + * @return + * - @c BUFFER_TOO_SHORT The given buffer in is too short + * - @c RETURN_FAILED Generic Error + * - @c RETURN_OK Successful serialization + */ template static ReturnValue_t serialize(const T *object, uint8_t **buffer, size_t *size, size_t maxSize, @@ -21,11 +37,35 @@ public: return adapter.serialize(object, buffer, size, maxSize, streamEndianness); } + /** + * Function to return the serialized size of the object in the pointer. + * May be a trivially copy-able object or a Child of SerializeIF + * + * @param object Pointer to Object + * @return Serialized size of object + */ template - static uint32_t getSerializedSize(const T *object) { + static size_t getSerializedSize(const T *object){ InternalSerializeAdapter::value> adapter; return adapter.getSerializedSize(object); } + /** + * @brief + * Deserializes a object from a given buffer of given size. + * Object Must be trivially copy-able or a child of SerializeIF. + * + * @details + * Buffer will be moved to the current read location. Size will be decreased by the function. + * + * @param[in/out] buffer Buffer to deSerialize from. Will be moved by the function. + * @param[in/out] size Remaining size of the buffer to read from. Will be decreased by function. + * @param[in] streamEndianness Endianness as in according to SerializeIF::Endianness + * @return + * - @c STREAM_TOO_SHORT The input stream is too short to deSerialize the object + * - @c TOO_MANY_ELEMENTS The buffer has more inputs than expected + * - @c RETURN_FAILED Generic Error + * - @c RETURN_OK Successful deserialization + */ template static ReturnValue_t deSerialize(T *object, const uint8_t **buffer, size_t *size, SerializeIF::Endianness streamEndianness) { @@ -33,16 +73,26 @@ public: return adapter.deSerialize(object, buffer, size, streamEndianness); } private: + /** + * Internal template to deduce the right function calls at compile time + */ template class InternalSerializeAdapter; + /** + * Template to be used if T is not a child of SerializeIF + * + * @tparam T T must be trivially_copyable + */ template class InternalSerializeAdapter { + static_assert (std::is_trivially_copyable::value, + "If a type needs to be serialized it must be a child of SerializeIF or trivially copy-able"); public: static ReturnValue_t serialize(const T *object, uint8_t **buffer, size_t *size, size_t max_size, SerializeIF::Endianness streamEndianness) { size_t ignoredSize = 0; - if (size == NULL) { + if (size == nullptr) { size = &ignoredSize; } //Check remaining size is large enough and check integer overflow of *size @@ -101,6 +151,11 @@ private: } }; + /** + * Template for objects that inherit from SerializeIF + * + * @tparam T A child of SerializeIF + */ template class InternalSerializeAdapter { public: @@ -108,12 +163,12 @@ private: size_t max_size, SerializeIF::Endianness streamEndianness) const { size_t ignoredSize = 0; - if (size == NULL) { + if (size == nullptr) { size = &ignoredSize; } return object->serialize(buffer, size, max_size, streamEndianness); } - uint32_t getSerializedSize(const T *object) const { + size_t getSerializedSize(const T *object) const { return object->getSerializedSize(); } @@ -124,4 +179,4 @@ private: }; }; -#endif /* SERIALIZEADAPTER_H_ */ +#endif /* _FSFW_SERIALIZE_SERIALIZEADAPTER_H_ */ diff --git a/serialize/SerializeIF.h b/serialize/SerializeIF.h index 7f9ea9dfd..a701fe53f 100644 --- a/serialize/SerializeIF.h +++ b/serialize/SerializeIF.h @@ -43,7 +43,7 @@ public: * @param[in] maxSize The size of the buffer that is allowed to be used for serialize. * @param[in] streamEndianness Endianness of the serialized data according to SerializeIF::Endianness * @return - * - @ยข BUFFER_TOO_SHORT The given buffer in is too short + * - @c BUFFER_TOO_SHORT The given buffer in is too short * - @c RETURN_FAILED Generic error * - @c RETURN_OK Successful serialization */