Skip to content

Commit

Permalink
Fix static analysis warnings and add comments to UTs
Browse files Browse the repository at this point in the history
  • Loading branch information
thomas-bc committed Feb 3, 2025
1 parent e2b5426 commit 5c22baf
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 16 deletions.
1 change: 1 addition & 0 deletions Svc/Deframer/test/ut/DeframerTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ void DeframerTester ::testNominalFrame() {
// TODO: make this test multiple times with different random bytes and lengths
// Get random byte of data
U8 randomByte = STest::Random::lowerUpper(0, 255);
// Note: the content of the frame header/footer doesn't actually matter in these tests
// | F´ start word | Length (= 1) | Data | Checksum (4 bytes) |
U8 data[13] = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x00, 0x00, 0x01, randomByte, 0x00, 0x00, 0x00, 0x00};
this->mockReceiveData(data, sizeof(data));
Expand Down
1 change: 1 addition & 0 deletions Svc/Deframer/test/ut/DeframerTester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ class DeframerTester : public DeframerGTestBase {
//! Initialize components
void initComponents();

//! Sends a buffer of supplied data and size on the component input port
void mockReceiveData(U8* data, FwSizeType size);


Expand Down
9 changes: 5 additions & 4 deletions Svc/FrameAccumulator/FrameAccumulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace Svc {
// ----------------------------------------------------------------------

FrameAccumulator ::FrameAccumulator(const char* const compName) : FrameAccumulatorComponentBase(compName),

Check notice

Code scanning / CodeQL

Use of basic integral type Note

compName uses the basic integral type char rather than a typedef with size and signedness.
m_detector(nullptr), m_memoryAllocator(nullptr), m_memory(nullptr), m_allocatorId(-1) {}
m_detector(nullptr), m_memoryAllocator(nullptr), m_memory(nullptr), m_allocatorId(0) {}

Check notice

Code scanning / CodeQL

More than one statement per line Note

This line contains 2 statements; only one is allowed.

FrameAccumulator ::~FrameAccumulator() {
// If configuration happened, we must deallocate
Expand Down Expand Up @@ -129,11 +129,12 @@ void FrameAccumulator ::processBuffer(Fw::Buffer& buffer) {
size_out <= remaining,
static_cast<FwAssertArgType>(size_out),
static_cast<FwAssertArgType>(remaining));
Fw::Buffer buffer = this->frameAllocate_out(0, size_out);
// REVIEW NOTE: size_out needs to be cast down in multiple places below - is this ok?
Fw::Buffer buffer = this->frameAllocate_out(0, static_cast<U32>(size_out));
if (buffer.isValid()) {
// Copy out data and rotate
FW_ASSERT(this->m_inRing.peek(buffer.getData(), size_out) == Fw::SerializeStatus::FW_SERIALIZE_OK);
buffer.setSize(size_out);
FW_ASSERT(this->m_inRing.peek(buffer.getData(), static_cast<NATIVE_UINT_TYPE>(size_out)) == Fw::SerializeStatus::FW_SERIALIZE_OK);
buffer.setSize(static_cast<U32>(size_out));
m_inRing.rotate(static_cast<U32>(size_out));

Check warning

Code scanning / CodeQL

Unchecked return value Warning

The return value of non-void function
rotate
is not checked.
FW_ASSERT(
m_inRing.get_allocated_size() == static_cast<U32>(remaining - size_out),
Expand Down
6 changes: 3 additions & 3 deletions Svc/FrameAccumulator/test/ut/FrameAccumulatorTester.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
#ifndef Svc_FrameAccumulatorTester_HPP
#define Svc_FrameAccumulatorTester_HPP

#include "Fw/Types/MallocAllocator.hpp"
#include "Svc/FrameAccumulator/FrameAccumulator.hpp"
#include "Svc/FrameAccumulator/FrameAccumulatorGTestBase.hpp"
#include "Svc/FrameAccumulator/FrameDetector/FprimeFrameDetector.hpp"
#include "Fw/Types/MallocAllocator.hpp"

namespace Svc {

Expand Down Expand Up @@ -70,7 +70,7 @@ class FrameAccumulatorTester : public FrameAccumulatorGTestBase {

//! Send a series of random-size buffers, terminated by a buffer that
//! will be detected as a full frame by the MockDetector
//! frame_size and buffer_count are updated with the size of the frame and the number of buffers sent
//! (output) frame_size and buffer_count are updated with the size of the frame and the number of buffers sent
void mockAccumulateFullFrame(U32& frame_size, U32& buffer_count);

//! Connect ports
Expand Down Expand Up @@ -117,7 +117,7 @@ class FrameAccumulatorTester : public FrameAccumulatorGTestBase {
MockDetector mockDetector;
Fw::MallocAllocator mallocator;

Fw::Buffer m_buffer; // buffer to be returned by mocked frameAllocate call
Fw::Buffer m_buffer; // buffer to be returned by mocked frameAllocate call
U8 m_buffer_slot[2048];
};

Expand Down
18 changes: 9 additions & 9 deletions Svc/Router/test/ut/RouterTester.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,23 @@ RouterTester ::~RouterTester() {}

void RouterTester ::testRouteComInterface() {
this->mockReceivePacketType(Fw::ComPacket::FW_PACKET_COMMAND);
ASSERT_from_commandOut_SIZE(1);
ASSERT_from_fileOut_SIZE(0);
ASSERT_from_bufferDeallocate_SIZE(1);
ASSERT_from_commandOut_SIZE(1); // one command packet emitted
ASSERT_from_fileOut_SIZE(0); // no file packet emitted
ASSERT_from_bufferDeallocate_SIZE(1); // command packets are deallocated by the router
}

void RouterTester ::testRouteFileInterface() {
this->mockReceivePacketType(Fw::ComPacket::FW_PACKET_FILE);
ASSERT_from_commandOut_SIZE(0);
ASSERT_from_fileOut_SIZE(1);
ASSERT_from_bufferDeallocate_SIZE(0);
ASSERT_from_commandOut_SIZE(0); // no command packet emitted
ASSERT_from_fileOut_SIZE(1); // one file packet emitted
ASSERT_from_bufferDeallocate_SIZE(0); // no deallocation (file packets' ownership is transferred to the receiver)
}

void RouterTester ::testRouteUnknownPacket() {
this->mockReceivePacketType(Fw::ComPacket::FW_PACKET_UNKNOWN);
ASSERT_from_commandOut_SIZE(0);
ASSERT_from_fileOut_SIZE(0);
ASSERT_from_bufferDeallocate_SIZE(1);
ASSERT_from_commandOut_SIZE(0); // no command packet emitted
ASSERT_from_fileOut_SIZE(0); // no file packet emitted
ASSERT_from_bufferDeallocate_SIZE(1); // unknown packets are deallocated and dropped
}

void RouterTester ::testCommandResponse() {
Expand Down

0 comments on commit 5c22baf

Please sign in to comment.