Increased TM stack robustness

1. More nullptr check
2. returnvalue for inititalize function which can fail

src/fsfw/tmtcpacket/SpacePacket.h

@@ -19,7 +19,8 @@ public:
 	/**
 	 * The constructor initializes the packet and sets all header information
 	 * according to the passed parameters.
-	 * @param packetDataLength Sets the packet data length field and therefore specifies the size of the packet.
+	 * @param packetDataLength Sets the packet data length field and therefore specifies
+	 * the size of the packet.
 	 * @param isTelecommand Sets the packet type field to either TC (true) or TM (false).
 	 * @param apid Sets the packet's APID field. The default value describes an idle packet.
 	 * @param sequenceCount ets the packet's Source Sequence Count field.

src/fsfw/tmtcpacket/SpacePacketBase.cpp

@@ -3,8 +3,8 @@
 
 #include <cstring>
 
-SpacePacketBase::SpacePacketBase( const uint8_t* set_address ) {
+SpacePacketBase::SpacePacketBase(const uint8_t* setAddress) {
-	this->data = (SpacePacketPointer*) set_address;
+    this->data = reinterpret_cast<SpacePacketPointer*>(const_cast<uint8_t*>(setAddress));
 }
 
 SpacePacketBase::~SpacePacketBase() {
@@ -12,94 +12,112 @@ SpacePacketBase::~SpacePacketBase() {
 
 //CCSDS Methods:
 uint8_t SpacePacketBase::getPacketVersionNumber( void ) {
-	return (this->data->header.packet_id_h & 0b11100000) >> 5;
+    return (this->data->header.packet_id_h & 0b11100000) >> 5;
 }
 
-void SpacePacketBase::initSpacePacketHeader(bool isTelecommand,
+ReturnValue_t SpacePacketBase::initSpacePacketHeader(bool isTelecommand,
-		bool hasSecondaryHeader, uint16_t apid, uint16_t sequenceCount) {
+        bool hasSecondaryHeader, uint16_t apid, uint16_t sequenceCount) {
-	//reset header to zero:
+    if(data == nullptr) {
-	memset(data,0, sizeof(this->data->header) );
+#if FSFW_VERBOSE_LEVEL >= 1
-	//Set TC/TM bit.
+#if FSFW_CPP_OSTREAM_ENABLED == 1
-	data->header.packet_id_h = ((isTelecommand? 1 : 0)) << 4;
+        sif::warning << "SpacePacketBase::initSpacePacketHeader: Data pointer is invalid"
-	//Set secondaryHeader bit
+                << std::endl;
-	data->header.packet_id_h |= ((hasSecondaryHeader? 1 : 0)) << 3;
+#else
-	this->setAPID( apid );
+        sif::printWarning("SpacePacketBase::initSpacePacketHeader: Data pointer is invalid!\n");
-	//Always initialize as standalone packets.
+#endif
-	data->header.sequence_control_h = 0b11000000;
+#endif
-	setPacketSequenceCount(sequenceCount);
+        return HasReturnvaluesIF::RETURN_FAILED;
-
+    }
+    //reset header to zero:
+    memset(data, 0, sizeof(this->data->header) );
+    //Set TC/TM bit.
+    data->header.packet_id_h = ((isTelecommand? 1 : 0)) << 4;
+    //Set secondaryHeader bit
+    data->header.packet_id_h |= ((hasSecondaryHeader? 1 : 0)) << 3;
+    this->setAPID( apid );
+    //Always initialize as standalone packets.
+    data->header.sequence_control_h = 0b11000000;
+    setPacketSequenceCount(sequenceCount);
+    return HasReturnvaluesIF::RETURN_OK;
 }
 
 bool SpacePacketBase::isTelecommand( void ) {
-	return (this->data->header.packet_id_h & 0b00010000) >> 4;
+    return (this->data->header.packet_id_h & 0b00010000) >> 4;
 }
 
 bool SpacePacketBase::hasSecondaryHeader( void ) {
-	return (this->data->header.packet_id_h & 0b00001000) >> 3;
+    return (this->data->header.packet_id_h & 0b00001000) >> 3;
 }
 
 uint16_t SpacePacketBase::getPacketId() {
-	return ( (this->data->header.packet_id_h) << 8 ) +
+    return ( (this->data->header.packet_id_h) << 8 ) +
-			this->data->header.packet_id_l;
+            this->data->header.packet_id_l;
 }
 
 uint16_t SpacePacketBase::getAPID( void ) const {
-	return ( (this->data->header.packet_id_h & 0b00000111) << 8 ) +
+    return ( (this->data->header.packet_id_h & 0b00000111) << 8 ) +
-			this->data->header.packet_id_l;
+            this->data->header.packet_id_l;
 }
 
 void SpacePacketBase::setAPID( uint16_t new_apid ) {
-	//Use first three bits of new APID, but keep rest of packet id as it was (see specification).
+    // Use first three bits of new APID, but keep rest of packet id as it was (see specification).
-	this->data->header.packet_id_h = (this->data->header.packet_id_h & 0b11111000) | ( ( new_apid & 0x0700 ) >> 8 );
+    this->data->header.packet_id_h = (this->data->header.packet_id_h & 0b11111000) |
-	this->data->header.packet_id_l = ( new_apid & 0x00FF );
+            ( ( new_apid & 0x0700 ) >> 8 );
+    this->data->header.packet_id_l = ( new_apid & 0x00FF );
+}
+
+void SpacePacketBase::setSequenceFlags( uint8_t sequenceflags ) {
+    this->data->header.sequence_control_h &= 0x3F;
+    this->data->header.sequence_control_h |= sequenceflags << 6;
 }
 
 uint16_t SpacePacketBase::getPacketSequenceControl( void ) {
-	return ( (this->data->header.sequence_control_h) << 8 )
+    return ( (this->data->header.sequence_control_h) << 8 )
-		+ this->data->header.sequence_control_l;
+            + this->data->header.sequence_control_l;
 }
 
 uint8_t SpacePacketBase::getSequenceFlags( void ) {
-	return (this->data->header.sequence_control_h & 0b11000000) >> 6 ;
+    return (this->data->header.sequence_control_h & 0b11000000) >> 6 ;
 }
 
 uint16_t SpacePacketBase::getPacketSequenceCount( void ) const {
-	return ( (this->data->header.sequence_control_h & 0b00111111) << 8 )
+    return ( (this->data->header.sequence_control_h & 0b00111111) << 8 )
-		+ this->data->header.sequence_control_l;
+            + this->data->header.sequence_control_l;
 }
 
 void SpacePacketBase::setPacketSequenceCount( uint16_t new_count) {
-	this->data->header.sequence_control_h = ( this->data->header.sequence_control_h & 0b11000000 ) | ( ( (new_count%LIMIT_SEQUENCE_COUNT) & 0x3F00 ) >> 8 );
+    this->data->header.sequence_control_h = ( this->data->header.sequence_control_h & 0b11000000 ) |
-	this->data->header.sequence_control_l = ( (new_count%LIMIT_SEQUENCE_COUNT) & 0x00FF );
+            ( ( (new_count%LIMIT_SEQUENCE_COUNT) & 0x3F00 ) >> 8 );
+    this->data->header.sequence_control_l = ( (new_count%LIMIT_SEQUENCE_COUNT) & 0x00FF );
 }
 
 uint16_t SpacePacketBase::getPacketDataLength() const {
-	return ( (this->data->header.packet_length_h) << 8 )
+    return ( (this->data->header.packet_length_h) << 8 )
-		+ this->data->header.packet_length_l;
+            + this->data->header.packet_length_l;
 }
 
 void SpacePacketBase::setPacketDataLength( uint16_t new_length) {
-	this->data->header.packet_length_h = ( ( new_length & 0xFF00 ) >> 8 );
+    this->data->header.packet_length_h = ( ( new_length & 0xFF00 ) >> 8 );
-	this->data->header.packet_length_l = ( new_length & 0x00FF );
+    this->data->header.packet_length_l = ( new_length & 0x00FF );
 }
 
 size_t SpacePacketBase::getFullSize() {
-	//+1 is done because size in packet data length field is: size of data field -1
+    // +1 is done because size in packet data length field is: size of data field -1
-	return this->getPacketDataLength() + sizeof(this->data->header) + 1;
+    return this->getPacketDataLength() + sizeof(this->data->header) + 1;
 }
 
 uint8_t* SpacePacketBase::getWholeData() {
-	return (uint8_t*)this->data;
+    return (uint8_t*)this->data;
 }
 
 void SpacePacketBase::setData( const uint8_t* p_Data ) {
-	this->data = (SpacePacketPointer*)p_Data;
+    this->data = (SpacePacketPointer*)p_Data;
 }
 
 uint32_t SpacePacketBase::getApidAndSequenceCount() const {
-	return (getAPID() << 16) + getPacketSequenceCount();
+    return (getAPID() << 16) + getPacketSequenceCount();
 }
 
 uint8_t* SpacePacketBase::getPacketData() {
-	return &(data->packet_data);
+    return &(data->packet_data);
 }

src/fsfw/tmtcpacket/SpacePacketBase.h

@@ -2,6 +2,8 @@
 #define FSFW_TMTCPACKET_SPACEPACKETBASE_H_
 
 #include "ccsds_header.h"
+#include "fsfw/returnvalues/HasReturnvaluesIF.h"
+
 #include <cstddef>
 
 /**
@@ -20,8 +22,8 @@
  * @ingroup tmtcpackets
  */
 struct SpacePacketPointer {
-	CCSDSPrimaryHeader header;
+    CCSDSPrimaryHeader header;
-	uint8_t packet_data;
+    uint8_t packet_data;
 };
 
 /**
@@ -37,143 +39,151 @@ struct SpacePacketPointer {
  */
 class SpacePacketBase {
 protected:
-	/**
+    /**
-	 * A pointer to a structure which defines the data structure of
+     * A pointer to a structure which defines the data structure of
-	 * the packet header.
+     * the packet header.
-	 * To be hardware-safe, all elements are of byte size.
+     * To be hardware-safe, all elements are of byte size.
-	 */
+     */
-	SpacePacketPointer* data;
+    SpacePacketPointer* data;
 public:
-	static const uint16_t LIMIT_APID = 2048; //2^1
+    static const uint16_t LIMIT_APID = 2048; //2^1
-	static const uint16_t LIMIT_SEQUENCE_COUNT = 16384; // 2^14
+    static const uint16_t LIMIT_SEQUENCE_COUNT = 16384; // 2^14
-	static const uint16_t APID_IDLE_PACKET = 0x7FF;
+    static const uint16_t APID_IDLE_PACKET = 0x7FF;
-	static const uint8_t TELECOMMAND_PACKET = 1;
+    static const uint8_t TELECOMMAND_PACKET = 1;
-	static const uint8_t TELEMETRY_PACKET = 0;
+    static const uint8_t TELEMETRY_PACKET = 0;
-	/**
+    /**
-	 * This definition defines the CRC size in byte.
+     * This definition defines the CRC size in byte.
-	 */
+     */
-	static const uint8_t CRC_SIZE = 2;
+    static const uint8_t CRC_SIZE = 2;
-	/**
+    /**
-	 * This is the minimum size of a SpacePacket.
+     * This is the minimum size of a SpacePacket.
-	 */
+     */
-	static const uint16_t MINIMUM_SIZE = sizeof(CCSDSPrimaryHeader) + CRC_SIZE;
+    static const uint16_t MINIMUM_SIZE = sizeof(CCSDSPrimaryHeader) + CRC_SIZE;
-	/**
+    /**
-	 * This is the default constructor.
+     * This is the default constructor.
-	 * It sets its internal data pointer to the address passed.
+     * It sets its internal data pointer to the address passed.
-	 * @param set_address	The position where the packet data lies.
+     * @param set_address	The position where the packet data lies.
-	 */
+     */
-	SpacePacketBase( const uint8_t* set_address );
+    SpacePacketBase( const uint8_t* set_address );
-	/**
+    /**
-	 * No data is allocated, so the destructor is empty.
+     * No data is allocated, so the destructor is empty.
-	 */
+     */
-	virtual ~SpacePacketBase();
+    virtual ~SpacePacketBase();
 
-	//CCSDS Methods:
+    //CCSDS Methods
-	/**
-	 * Getter for the packet version number field.
-	 * @return Returns the highest three bit of the packet in one byte.
-	 */
-	uint8_t getPacketVersionNumber( void );
-	/**
-	 * This method checks the type field in the header.
-	 * This bit specifies, if the command is interpreted as Telecommand of
-	 * as Telemetry. For a Telecommand, the bit is set.
-	 * @return Returns true if the bit is set and false if not.
-	 */
-	bool isTelecommand( void );
 
-	void initSpacePacketHeader(bool isTelecommand, bool hasSecondaryHeader,
+    /**
-	        uint16_t apid, uint16_t sequenceCount = 0);
+     * Getter for the packet version number field.
-	/**
+     * @return Returns the highest three bit of the packet in one byte.
-	 * The CCSDS header provides a secondary header flag (the fifth-highest bit),
+     */
-	 *  which is checked with this method.
+    uint8_t getPacketVersionNumber( void );
-	 * @return	Returns true if the bit is set and false if not.
+    /**
-	 */
+     * This method checks the type field in the header.
-	bool hasSecondaryHeader( void );
+     * This bit specifies, if the command is interpreted as Telecommand of
-	/**
+     * as Telemetry. For a Telecommand, the bit is set.
-	 * Returns the complete first two bytes of the packet, which together form
+     * @return Returns true if the bit is set and false if not.
-	 * the CCSDS packet id.
+     */
-	 * @return	The CCSDS packet id.
+    bool isTelecommand( void );
-	 */
-	uint16_t getPacketId( void );
-	/**
-	 * Returns the APID of a packet, which are the lowest 11 bit of the packet
-	 * id.
-	 * @return The CCSDS APID.
-	 */
-	uint16_t getAPID( void ) const;
-	/**
-	 * Sets the APID of a packet, which are the lowest 11 bit of the packet
-	 * id.
-	 * @param 	The APID to set. The highest five bits of the parameter are
-	 * 			ignored.
-	 */
-	void setAPID( uint16_t setAPID );
-	/**
-	 * Returns the CCSDS packet sequence control field, which are the third and
-	 * the fourth byte of the CCSDS primary header.
-	 * @return The CCSDS packet sequence control field.
-	 */
-	uint16_t getPacketSequenceControl( void );
-	/**
-	 * Returns the SequenceFlags, which are the highest two bit of the packet
-	 * sequence control field.
-	 * @return	The CCSDS sequence flags.
-	 */
-	uint8_t getSequenceFlags( void );
-	/**
-	 * Returns the packet sequence count, which are the lowest 14 bit of the
-	 * packet sequence control field.
-	 * @return The CCSDS sequence count.
-	 */
-	uint16_t getPacketSequenceCount( void ) const;
-	/**
-	 * Sets the packet sequence count, which are the lowest 14 bit of the
-	 * packet sequence control field.
-	 * setCount is modulo-divided by \c LIMIT_SEQUENCE_COUNT to avoid overflows.
-	 * @param setCount	The value to set the count to.
-	 */
-	void setPacketSequenceCount( uint16_t setCount );
-	/**
-	 * Returns the packet data length, which is the fifth and sixth byte of the
-	 * CCSDS Primary Header. The packet data length is the size of every kind
-	 * of data \b after the CCSDS Primary Header \b -1.
-	 * @return
-	 * The CCSDS packet data length. uint16_t is sufficient,
-	 * because this is limit in CCSDS standard
-	 */
-	uint16_t getPacketDataLength(void) const;
-	/**
-	 * Sets the packet data length, which is the fifth and sixth byte of the
-	 * CCSDS Primary Header.
-	 * @param setLength The value of the length to set. It must fit the true
-	 * 					CCSDS packet data length . The packet data length is
-	 * 					the size of every kind of data \b after the CCSDS
-	 * 					Primary Header \b -1.
-	 */
-	void setPacketDataLength( uint16_t setLength );
 
-	//Helper methods:
+    ReturnValue_t initSpacePacketHeader(bool isTelecommand, bool hasSecondaryHeader,
-	/**
+            uint16_t apid, uint16_t sequenceCount = 0);
-	 * This method returns a raw uint8_t pointer to the packet.
+    /**
-	 * @return	A \c uint8_t pointer to the first byte of the CCSDS primary header.
+     * The CCSDS header provides a secondary header flag (the fifth-highest bit),
-	 */
+     *  which is checked with this method.
-	virtual uint8_t* getWholeData( void );
+     * @return	Returns true if the bit is set and false if not.
+     */
+    bool hasSecondaryHeader( void );
+    /**
+     * Returns the complete first two bytes of the packet, which together form
+     * the CCSDS packet id.
+     * @return	The CCSDS packet id.
+     */
+    uint16_t getPacketId( void );
+    /**
+     * Returns the APID of a packet, which are the lowest 11 bit of the packet
+     * id.
+     * @return The CCSDS APID.
+     */
+    uint16_t getAPID( void ) const;
+    /**
+     * Sets the APID of a packet, which are the lowest 11 bit of the packet
+     * id.
+     * @param 	The APID to set. The highest five bits of the parameter are
+     * 			ignored.
+     */
+    void setAPID( uint16_t setAPID );
 
-	uint8_t* getPacketData();
+    /**
-	/**
+     * Sets the sequence flags of a packet, which are bit 17 and 18 in the space packet header.
-	 * With this method, the packet data pointer can be redirected to another
+     * @param   The sequence flags to set
-	 * location.
+     */
-	 * @param p_Data A pointer to another raw Space Packet.
+    void setSequenceFlags( uint8_t sequenceflags );
-	 */
-	virtual void setData( const uint8_t* p_Data );
-	/**
-	 * This method returns the full raw packet size.
-	 * @return	The full size of the packet in bytes.
-	 */
-	size_t getFullSize();
 
-	uint32_t getApidAndSequenceCount() const;
+    /**
+     * Returns the CCSDS packet sequence control field, which are the third and
+     * the fourth byte of the CCSDS primary header.
+     * @return The CCSDS packet sequence control field.
+     */
+    uint16_t getPacketSequenceControl( void );
+    /**
+     * Returns the SequenceFlags, which are the highest two bit of the packet
+     * sequence control field.
+     * @return	The CCSDS sequence flags.
+     */
+    uint8_t getSequenceFlags( void );
+    /**
+     * Returns the packet sequence count, which are the lowest 14 bit of the
+     * packet sequence control field.
+     * @return The CCSDS sequence count.
+     */
+    uint16_t getPacketSequenceCount( void ) const;
+    /**
+     * Sets the packet sequence count, which are the lowest 14 bit of the
+     * packet sequence control field.
+     * setCount is modulo-divided by \c LIMIT_SEQUENCE_COUNT to avoid overflows.
+     * @param setCount	The value to set the count to.
+     */
+    void setPacketSequenceCount( uint16_t setCount );
+    /**
+     * Returns the packet data length, which is the fifth and sixth byte of the
+     * CCSDS Primary Header. The packet data length is the size of every kind
+     * of data \b after the CCSDS Primary Header \b -1.
+     * @return
+     * The CCSDS packet data length. uint16_t is sufficient,
+     * because this is limit in CCSDS standard
+     */
+    uint16_t getPacketDataLength(void) const;
+    /**
+     * Sets the packet data length, which is the fifth and sixth byte of the
+     * CCSDS Primary Header.
+     * @param setLength The value of the length to set. It must fit the true
+     * 					CCSDS packet data length . The packet data length is
+     * 					the size of every kind of data \b after the CCSDS
+     * 					Primary Header \b -1.
+     */
+    void setPacketDataLength( uint16_t setLength );
+
+    // Helper methods
+    /**
+     * This method returns a raw uint8_t pointer to the packet.
+     * @return	A \c uint8_t pointer to the first byte of the CCSDS primary header.
+     */
+    virtual uint8_t* getWholeData( void );
+
+    uint8_t* getPacketData();
+    /**
+     * With this method, the packet data pointer can be redirected to another
+     * location.
+     * @param p_Data A pointer to another raw Space Packet.
+     */
+    virtual void setData( const uint8_t* p_Data );
+    /**
+     * This method returns the full raw packet size.
+     * @return	The full size of the packet in bytes.
+     */
+    size_t getFullSize();
+
+    uint32_t getApidAndSequenceCount() const;
 
 };
 

src/fsfw/tmtcpacket/pus/tm/TmPacketPusC.cpp

@@ -53,11 +53,14 @@ uint8_t* TmPacketPusC::getPacketTimeRaw() const{
 
 }
 
-void TmPacketPusC::initializeTmPacket(uint16_t apid, uint8_t service,
+ReturnValue_t TmPacketPusC::initializeTmPacket(uint16_t apid, uint8_t service,
         uint8_t subservice, uint16_t packetSubcounter, uint16_t destinationId,
         uint8_t timeRefField) {
     //Set primary header:
-    initSpacePacketHeader(false, true, apid);
+    ReturnValue_t result = initSpacePacketHeader(false, true, apid);
+    if(result != HasReturnvaluesIF::RETURN_OK) {
+        return result;
+    }
     //Set data Field Header:
     //First, set to zero.
     memset(&tmData->dataField, 0, sizeof(tmData->dataField));
@@ -76,6 +79,7 @@ void TmPacketPusC::initializeTmPacket(uint16_t apid, uint8_t service,
         timeStamper->addTimeStamp(tmData->dataField.time,
                 sizeof(tmData->dataField.time));
     }
+    return HasReturnvaluesIF::RETURN_OK;
 }
 
 void TmPacketPusC::setSourceDataSize(uint16_t size) {

src/fsfw/tmtcpacket/pus/tm/TmPacketPusC.h

@@ -100,7 +100,7 @@ protected:
      * @param subservice PUS Subservice
      * @param packetSubcounter Additional subcounter used.
      */
-    void initializeTmPacket(uint16_t apid, uint8_t service, uint8_t subservice,
+    ReturnValue_t initializeTmPacket(uint16_t apid, uint8_t service, uint8_t subservice,
             uint16_t packetSubcounter, uint16_t destinationId = 0, uint8_t timeRefField = 0);
 
     /**

src/fsfw/tmtcpacket/pus/tm/TmPacketStoredPusC.cpp

@@ -43,27 +43,55 @@ TmPacketStoredPusC::TmPacketStoredPusC(uint16_t apid, uint8_t service,
         return;
     }
     size_t sourceDataSize = 0;
-    if (content != NULL) {
+    if (content != nullptr) {
         sourceDataSize += content->getSerializedSize();
     }
-    if (header != NULL) {
+    if (header != nullptr) {
         sourceDataSize += header->getSerializedSize();
     }
-    uint8_t *p_data = NULL;
+    uint8_t *pData = nullptr;
-    ReturnValue_t returnValue = store->getFreeElement(&storeAddress,
+    size_t sizeToReserve = getPacketMinimumSize() + sourceDataSize;
-            (getPacketMinimumSize() + sourceDataSize), &p_data);
+    ReturnValue_t returnValue = store->getFreeElement(&storeAddress, sizeToReserve, &pData);
     if (returnValue != store->RETURN_OK) {
+#if FSFW_VERBOSE_LEVEL >= 1
+        switch(returnValue) {
+#if FSFW_CPP_OSTREAM_ENABLED == 1
+        case(StorageManagerIF::DATA_STORAGE_FULL): {
+            sif::warning << "TmPacketStoredPusC::TmPacketStoredPusC: Store full for packet with "
+                    "size " << sizeToReserve << std::endl;
+            break;
+        }
+        case(StorageManagerIF::DATA_TOO_LARGE): {
+            sif::warning << "TmPacketStoredPusC::TmPacketStoredPusC: Data with size " <<
+                    sizeToReserve << " too large" <<  std::endl;
+            break;
+        }
+#else
+        case(StorageManagerIF::DATA_STORAGE_FULL): {
+            sif::printWarning("TmPacketStoredPusC::TmPacketStoredPusC: Store full for packet with "
+                    "size %d\n", sizeToReserve);
+            break;
+        }
+        case(StorageManagerIF::DATA_TOO_LARGE): {
+            sif::printWarning("TmPacketStoredPusC::TmPacketStoredPusC: Data with size "
+                    "%d too large\n", sizeToReserve);
+            break;
+        }
+#endif
+#endif
+        }
         TmPacketStoredBase::checkAndReportLostTm();
+        return;
     }
-    setData(p_data);
+    setData(pData);
     initializeTmPacket(apid, service, subservice, packetSubcounter, destinationId, timeRefField);
     uint8_t *putDataHere = getSourceData();
     size_t size = 0;
-    if (header != NULL) {
+    if (header != nullptr) {
         header->serialize(&putDataHere, &size, sourceDataSize,
                 SerializeIF::Endianness::BIG);
     }
-    if (content != NULL) {
+    if (content != nullptr) {
         content->serialize(&putDataHere, &size, sourceDataSize,
                 SerializeIF::Endianness::BIG);
     }