Skip to content

Commit 8d2d73a

Browse files
bitWarriorbitWarriorLeStarch
authored
PR 5038 - Force Framers to setSize on Fw::Buffer before outputting it (#5079)
* Force Framers to setSize on Fw::Buffer before outputting it * Fix missing test coverage * Remove unused variable --------- Co-authored-by: bitWarrior <contact.bitWarrior@proton.me> Co-authored-by: M Starch <LeStarch@googlemail.com>
1 parent f411430 commit 8d2d73a

8 files changed

Lines changed: 85 additions & 3 deletions

File tree

Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ void SpacePacketFramer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, c
7272
status = frameSerializer.serializeFrom(data.getData(), data.getSize(), Fw::Serialization::OMIT_LENGTH);
7373
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, status);
7474

75+
// Trim to actual frame size in case allocator returned a larger buffer
76+
frameBuffer.setSize(static_cast<Fw::Buffer::SizeType>(frameSize));
77+
7578
this->dataOut_out(0, frameBuffer, context);
7679
this->dataReturnOut_out(0, data, context); // return ownership of the original data buffer
7780
}

Svc/Ccsds/SpacePacketFramer/test/ut/SpacePacketFramerTestMain.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ TEST(SpacePacketFramer, testNominalFraming) {
2121
tester.testNominalFraming();
2222
}
2323

24+
TEST(SpacePacketFramer, OversizedAllocatorBufferIsTrimmed) {
25+
Svc::Ccsds::SpacePacketFramerTester tester;
26+
tester.testOversizedAllocatorBufferIsTrimmed();
27+
}
28+
2429
int main(int argc, char** argv) {
2530
::testing::InitGoogleTest(&argc, argv);
2631
return RUN_ALL_TESTS();

Svc/Ccsds/SpacePacketFramer/test/ut/SpacePacketFramerTester.cpp

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,33 @@ void SpacePacketFramerTester::testNominalFraming() {
8181
ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getSize(), sizeof(payload));
8282
}
8383

84+
void SpacePacketFramerTester ::testOversizedAllocatorBufferIsTrimmed() {
85+
U8 payload[16];
86+
for (U32 i = 0; i < sizeof(payload); ++i) {
87+
payload[i] = static_cast<U8>(STest::Random::lowerUpper(0, 0xFF));
88+
}
89+
Fw::Buffer data(payload, sizeof(payload));
90+
ComCfg::FrameContext context;
91+
context.set_apid(static_cast<ComCfg::Apid::T>(0x01));
92+
this->m_nextSeqCount = 0;
93+
94+
// Signal the allocator handler to return a larger-than-needed buffer,
95+
// simulating a BufferManager bin that is bigger than the exact packet size.
96+
this->m_useOversizedAlloc = true;
97+
this->invoke_to_dataIn(0, data, context);
98+
this->m_useOversizedAlloc = false;
99+
100+
ASSERT_from_dataOut_SIZE(1);
101+
ASSERT_from_dataReturnOut_SIZE(1);
102+
103+
const FwSizeType expectedFrameSize = sizeof(payload) + SpacePacketHeader::SERIALIZED_SIZE;
104+
105+
Fw::Buffer outBuffer = this->fromPortHistory_dataOut->at(0).data;
106+
// If setSize() is missing from SpacePacketFramer, getSize() returns the
107+
// oversized allocation (2 * expectedFrameSize) and this assertion fails.
108+
ASSERT_EQ(outBuffer.getSize(), expectedFrameSize);
109+
}
110+
84111
// ----------------------------------------------------------------------
85112
// Output port handler overrides
86113
// ----------------------------------------------------------------------
@@ -92,7 +119,8 @@ U16 SpacePacketFramerTester ::from_getApidSeqCount_handler(FwIndexType portNum,
92119
}
93120

94121
Fw::Buffer SpacePacketFramerTester ::from_bufferAllocate_handler(FwIndexType portNum, FwSizeType size) {
95-
return Fw::Buffer(this->m_internalDataBuffer, size);
122+
FwSizeType allocation = (this->m_useOversizedAlloc) ? sizeof(this->m_internalDataBuffer) : size;
123+
return Fw::Buffer(this->m_internalDataBuffer, allocation);
96124
}
97125

98126
} // namespace Ccsds

Svc/Ccsds/SpacePacketFramer/test/ut/SpacePacketFramerTester.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class SpacePacketFramerTester final : public SpacePacketFramerGTestBase {
4646
void testComStatusPassthrough();
4747
void testDataReturnPassthrough();
4848
void testNominalFraming();
49+
void testOversizedAllocatorBufferIsTrimmed();
4950

5051
private:
5152
// ----------------------------------------------------------------------
@@ -80,6 +81,9 @@ class SpacePacketFramerTester final : public SpacePacketFramerGTestBase {
8081
U8 m_internalDataBuffer[SpacePacketHeader::SERIALIZED_SIZE + MAX_TEST_PACKET_DATA_SIZE];
8182

8283
U16 m_nextSeqCount; // Sequence count to be returned by getApidSeqCount output port
84+
85+
// Flag read by from_bufferAllocate_handler to choose which path to take
86+
bool m_useOversizedAlloc = false;
8387
};
8488

8589
} // namespace Ccsds

Svc/FprimeFramer/FprimeFramer.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ void FprimeFramer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, const
5555
trailer.set_crcField(hashBuffer.asBigEndianU32());
5656
status = frameSerializer.serializeFrom(trailer);
5757
FW_ASSERT(status == Fw::FW_SERIALIZE_OK, status);
58+
// Trim to actual frame size in case allocator returned a larger buffer
59+
frameBuffer.setSize(static_cast<Fw::Buffer::SizeType>(frameSize));
5860

5961
// Send the full frame out - this port shall always be connected
6062
this->dataOut_out(0, frameBuffer, context);

Svc/FprimeFramer/test/ut/FprimeFramerTestMain.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ TEST(Nominal, testNominalFraming) {
2121
tester.testNominalFraming();
2222
}
2323

24+
TEST(OffNominal, OversizedAllocatorBufferIsTrimmed) {
25+
Svc::FprimeFramerTester tester;
26+
tester.testOversizedAllocatorBufferIsTrimmed();
27+
}
28+
2429
int main(int argc, char** argv) {
2530
::testing::InitGoogleTest(&argc, argv);
2631
return RUN_ALL_TESTS();

Svc/FprimeFramer/test/ut/FprimeFramerTester.cpp

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,38 @@ void FprimeFramerTester ::testNominalFraming() {
8585
Fw::Buffer FprimeFramerTester::from_bufferAllocate_handler(FwIndexType portNum, FwSizeType size) {
8686
this->pushFromPortEntry_bufferAllocate(size);
8787
this->m_buffer.setData(this->m_buffer_slot);
88-
this->m_buffer.setSize(size);
89-
::memset(this->m_buffer.getData(), 0, size);
88+
// When m_useOversizedAlloc is set, simulate a pool allocator returning
89+
// a larger block than requested — the component must trim it before sending
90+
FwSizeType allocatedSize = this->m_useOversizedAlloc ? sizeof(this->m_buffer_slot) : size;
91+
this->m_buffer.setSize(allocatedSize);
92+
::memset(this->m_buffer.getData(), 0, allocatedSize);
9093
return this->m_buffer;
9194
}
9295

96+
// ----------------------------------------------------------------------
97+
// Test Harness: Handler oversized output
98+
// ----------------------------------------------------------------------
99+
100+
void FprimeFramerTester::testOversizedAllocatorBufferIsTrimmed() {
101+
U8 bufferData[100];
102+
Fw::Buffer buffer(bufferData, sizeof(bufferData));
103+
ComCfg::FrameContext context;
104+
105+
for (U32 i = 0; i < sizeof(bufferData); ++i) {
106+
bufferData[i] = static_cast<U8>(i);
107+
}
108+
109+
this->m_useOversizedAlloc = true;
110+
this->invoke_to_dataIn(0, buffer, context);
111+
this->m_useOversizedAlloc = false;
112+
113+
ASSERT_from_dataOut_SIZE(1);
114+
ASSERT_from_dataReturnOut_SIZE(1);
115+
116+
Fw::Buffer outputBuffer = this->fromPortHistory_dataOut->at(0).data;
117+
FwSizeType expectedSize = sizeof(bufferData) + FprimeProtocol::FrameHeader::SERIALIZED_SIZE +
118+
FprimeProtocol::FrameTrailer::SERIALIZED_SIZE;
119+
ASSERT_EQ(outputBuffer.getSize(), expectedSize);
120+
}
121+
93122
} // namespace Svc

Svc/FprimeFramer/test/ut/FprimeFramerTester.hpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ class FprimeFramerTester final : public FprimeFramerGTestBase {
4949
//! Test framing of data
5050
void testNominalFraming();
5151

52+
//! Test oversized buffer allocation (trimming) of data
53+
void testOversizedAllocatorBufferIsTrimmed();
54+
5255
private:
5356
// ----------------------------------------------------------------------
5457
// Helper functions
@@ -76,6 +79,9 @@ class FprimeFramerTester final : public FprimeFramerGTestBase {
7679

7780
U8 m_buffer_slot[2048];
7881
Fw::Buffer m_buffer; // buffer to be returned by mocked allocate call
82+
83+
// Flag read by from_bufferAllocate_handler to choose which path to take
84+
bool m_useOversizedAlloc = false;
7985
};
8086

8187
} // namespace Svc

0 commit comments

Comments
 (0)