Skip to content

Commit

Permalink
Revise DpContainer and unit tests
Browse files Browse the repository at this point in the history
Don't fail an assertion on bad serial input
  • Loading branch information
bocchino committed Feb 2, 2024
1 parent 13ee8b8 commit 0fc6eb2
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 24 deletions.
60 changes: 38 additions & 22 deletions Fw/Dp/DpContainer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,39 +35,55 @@ DpContainer::DpContainer()
// Public member functions
// ----------------------------------------------------------------------

void DpContainer::deserializeHeader() {
Fw::SerializeStatus DpContainer::deserializeHeader() {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
FW_ASSERT(this->m_buffer.isValid());
Fw::SerializeBufferBase& serializeRepr = this->m_buffer.getSerializeRepr();
// Reset deserialization
serializeRepr.setBuffLen(this->m_buffer.getSize());

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
setBuffLen
is not checked.
Fw::SerializeStatus status = serializeRepr.moveDeserToOffset(Header::ID_OFFSET);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(status));
Fw::SerializeStatus status = serializeRepr.moveDeserToOffset(Header::PACKET_DESCRIPTOR_OFFSET);
// Deserialize the packet type
if (status == Fw::FW_SERIALIZE_OK) {
FwPacketDescriptorType packetDescriptor;
status = serializeRepr.deserialize(packetDescriptor);
if (packetDescriptor != Fw::ComPacket::FW_PACKET_DP) {
status = Fw::FW_SERIALIZE_FORMAT_ERROR;
}
}
// Deserialize the container id
status = serializeRepr.deserialize(this->m_id);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(status));
if (status == Fw::FW_SERIALIZE_OK) {
status = serializeRepr.deserialize(this->m_id);
}
// Deserialize the priority
status = serializeRepr.deserialize(this->m_priority);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(status));
if (status == Fw::FW_SERIALIZE_OK) {
status = serializeRepr.deserialize(this->m_priority);
}
// Deserialize the time tag
status = serializeRepr.deserialize(this->m_timeTag);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(status));
if (status == Fw::FW_SERIALIZE_OK) {
status = serializeRepr.deserialize(this->m_timeTag);
}
// Deserialize the processing types
status = serializeRepr.deserialize(this->m_procTypes);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(status));
if (status == Fw::FW_SERIALIZE_OK) {
status = serializeRepr.deserialize(this->m_procTypes);
}
// Deserialize the user data
const bool omitLength = true;
const FwSizeType requestedSize = sizeof this->m_userData;
FwSizeType receivedSize = requestedSize;
status = serializeRepr.deserialize(this->m_userData, receivedSize, omitLength);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(status));
FW_ASSERT(receivedSize == requestedSize, static_cast<FwAssertArgType>(requestedSize),
static_cast<FwAssertArgType>(receivedSize));
if (status == Fw::FW_SERIALIZE_OK) {
const bool omitLength = true;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

omitLength uses the basic integral type bool rather than a typedef with size and signedness.
const FwSizeType requestedSize = sizeof this->m_userData;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

requestedSize uses the basic integral type unsigned long rather than a typedef with size and signedness.
FwSizeType receivedSize = requestedSize;

Check notice

Code scanning / CodeQL

Use of basic integral type Note

receivedSize uses the basic integral type unsigned long rather than a typedef with size and signedness.
status = serializeRepr.deserialize(this->m_userData, receivedSize, omitLength);
if (receivedSize != requestedSize) {
status = Fw::FW_DESERIALIZE_SIZE_MISMATCH;
}
}
// Deserialize the data product state
status = serializeRepr.deserialize(this->m_dpState);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(status));
if (status == Fw::FW_SERIALIZE_OK) {
status = serializeRepr.deserialize(this->m_dpState);
}
// Deserialize the data size
status = serializeRepr.deserialize(this->m_dataSize);
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, static_cast<FwAssertArgType>(status));
if (status == Fw::FW_SERIALIZE_OK) {
status = serializeRepr.deserialize(this->m_dataSize);
}
return status;
}

void DpContainer::serializeHeader() {

Check notice

Code scanning / CodeQL

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
Expand Down
3 changes: 2 additions & 1 deletion Fw/Dp/DpContainer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ class DpContainer {

//! Deserialize the header from the packet buffer
//! Buffer must be valid and large enough to hold a DP container packet
void deserializeHeader();
//! \return The serialize status
Fw::SerializeStatus deserializeHeader();

//! Serialize the header into the packet buffer and update the header hash
//! Buffer must be valid and large enough to hold a DP container packet
Expand Down
21 changes: 20 additions & 1 deletion Fw/Dp/test/ut/TestMain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ void checkHeader(FwDpIdType id, Fw::Buffer& buffer, DpContainer& container) {
// Test deserialization: Deserialize the header into a new container
DpContainer deserContainer;
deserContainer.setBuffer(container.getBuffer());
deserContainer.deserializeHeader();
const Fw::SerializeStatus serialStatus = deserContainer.deserializeHeader();
ASSERT_EQ(serialStatus, Fw::FW_SERIALIZE_OK);
// Clear out the header in the buffer
FW_ASSERT(buffer.isValid());
::memset(buffer.getData(), 0, DpContainer::Header::SIZE);
Expand Down Expand Up @@ -121,6 +122,24 @@ TEST(Header, BufferSet) {
checkBuffers(container, sizeof bufferData);
}

TEST(Header, BadPacketDescriptor) {
COMMENT("Test header serialization with bad packet descriptor");
// Create a buffer
Fw::Buffer buffer(bufferData, sizeof bufferData);
// Set the packet descriptor to a bad value
Fw::SerializeBufferBase& serialRepr = buffer.getSerializeRepr();
const FwPacketDescriptorType badPacketDescriptor = Fw::ComPacket::FW_PACKET_DP + 1;
Fw::SerializeStatus status = serialRepr.serialize(badPacketDescriptor);
ASSERT_EQ(status, Fw::FW_SERIALIZE_OK);
// Use the buffer to create a container
DpContainer container;
container.setBuffer(buffer);
// Deserialize the header
const Fw::SerializeStatus serialStatus = container.deserializeHeader();
// Check the error
ASSERT_EQ(serialStatus, Fw::FW_SERIALIZE_FORMAT_ERROR);
}

int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
STest::Random::seed();
Expand Down

0 comments on commit 0fc6eb2

Please sign in to comment.