Skip to content

Add destCapacity DeserializeTo for more robustness#5043

Open
bitWarrior wants to merge 9 commits intonasa:develfrom
bitWarrior:deserializeTo
Open

Add destCapacity DeserializeTo for more robustness#5043
bitWarrior wants to merge 9 commits intonasa:develfrom
bitWarrior:deserializeTo

Conversation

@bitWarrior
Copy link
Copy Markdown
Collaborator

@bitWarrior bitWarrior commented Apr 20, 2026

Related Issue(s)
Has Unit Tests (Y) Y
Documentation Included (N) Y
Generative AI was used in this contribution (N) Y

Change Description

Added destCapacity param:
[+] SerializeStatus deserializeTo(U8* buff, SizeType buffCapacity, SizeType& length, bool noLength);

Rationale

Adding a destCapacity parameter allows for an extra check to ensure the memcpy does not overflow. Undesirable things could happen if it does.

What Changed

LinearBufferBase / SerialBufferBase (Fw/Types/Serializable.hpp)

The two byte-buffer overloads of deserializeTo now require an explicit destination capacity:

// Before
SerializeStatus deserializeTo(U8* buff, FwSizeType& length, Endianness endianMode = Endianness::BIG);

SerializeStatus deserializeTo(U8* buff,
                              FwSizeType& length,
                              Serialization::t lengthMode,
                              Endianness endianMode = Endianness::BIG);

// After
SerializeStatus deserializeTo(U8* buff,
                              FwSizeType buffCapacity,
                              FwSizeType& length,
                              Endianness endianMode = Endianness::BIG);

SerializeStatus deserializeTo(U8* buff,
                              FwSizeType buffCapacity,
                              FwSizeType& length,
                              Serialization::t lengthMode,
                              Endianness endianMode = Endianness::BIG);

SerialBuffer (Fw/Types/SerialBuffer.hpp)

popBytes now requires an explicit destination capacity:

// Before
SerializeStatus popBytes(U8* const addr, FwSizeType n);

// After
SerializeStatus popBytes(U8* const addr, FwSizeType buffCapacity, FwSizeType n);

Deprecated overloads removed

The following deprecated deserialize(U8* buff, ...) overloads could not be safely forwarded to the new signatures and have been removed. They were already marked DEPRECATED in prior releases:

Removed overload Replacement
deserialize(U8* buff, FwSizeType& length, bool noLength) deserializeTo(buff, buffCapacity, length, lengthMode)
deserialize(U8* buff, FwSizeType& length) deserializeTo(buff, buffCapacity, length)
deserialize(U8* buff, FwSizeType& length, Serialization::t mode) deserializeTo(buff, buffCapacity, length, mode)

New Runtime Behavior

FW_DESERIALIZE_SIZE_MISMATCH is now returned in two additional situations:

  • The serialized length exceeds buffCapacity (previously only checked against source bytes remaining).
  • buff is nullptr while buffCapacity > 0.

In both cases the deserialization cursor is not advanced, so callers can handle the error and retry or report without corrupting deserialization state.


Migration Guide

Step 1 — Find all call sites

Search your codebase for calls to the affected functions:

grep -rn "deserializeTo\|popBytes" --include="*.cpp" --include="*.hpp" .

Focus on overloads that take a U8* destination buffer. Scalar overloads (e.g., deserializeTo(U32& val)) are not affected.

Step 2 — Add the buffCapacity argument

Insert the size of the destination buffer as the second argument, immediately after the pointer.

deserializeTo — stack array:

// Before
U8 myBuf[256];
FwSizeType len = sizeof(myBuf);
status = serializer.deserializeTo(myBuf, len, Fw::Serialization::OMIT_LENGTH);

// After
U8 myBuf[256];
FwSizeType len = sizeof(myBuf);
status = serializer.deserializeTo(myBuf, sizeof(myBuf), len, Fw::Serialization::OMIT_LENGTH);

deserializeTo — heap / external buffer:

// Before
status = serializer.deserializeTo(myBuffer.getBuffAddr(), size, Fw::Serialization::OMIT_LENGTH);

// After
status = serializer.deserializeTo(myBuffer.getBuffAddr(), myBuffer.getCapacity(), size, Fw::Serialization::OMIT_LENGTH);

popBytes:

// Before
status = serialBuf.popBytes(dest, n);

// After
status = serialBuf.popBytes(dest, sizeof(dest), n);   // stack array
status = serialBuf.popBytes(dest, destCapacity, n);   // dynamic buffer

Step 3 — Remove calls to deprecated overloads

If your code still uses the removed deserialize(U8* buff, ...) overloads (which emitted compiler warnings in prior releases), migrate them to deserializeTo with the capacity argument as shown in Step 2.

Step 4 — Handle the new error path

If your code already checks the return value of deserializeTo or popBytes (as it should), no further changes are needed — FW_DESERIALIZE_SIZE_MISMATCH is not a new status code. However, if you previously assumed that a successful deserialize meant the source data fit the destination, you may want to add a log or assertion to distinguish this case for easier debugging:

SerializeStatus status = serializer.deserializeTo(dest, sizeof(dest), length, Fw::Serialization::OMIT_LENGTH);
if (status == Fw::FW_DESERIALIZE_SIZE_MISMATCH) {
    // Could now be: source underrun OR destination too small — log accordingly
}

Testing/Review Recommendations

  1. Execute new unit test DeserializeToTest
  2. Review attached batch script for regression testing:
    regression_test_deserializeTo.sh

Affected Files (Framework)

File Change
Fw/Types/Serializable.hpp / .cpp New buffCapacity param on deserializeTo byte-buffer overloads; deprecated overloads removed
Fw/Types/SerialBuffer.hpp / .cpp New buffCapacity param on popBytes
Fw/Types/StringBase.cpp Updated deserializeFrom call site
Fw/Tlm/TlmPacket.cpp Updated extractValue and deserializeFrom call sites
Fw/Log/LogPacket.cpp Updated call site
Fw/Log/AmpcsEvrLogPacket.cpp Updated call sites
Fw/Dp/DpContainer.cpp Updated call site
Fw/FilePacket/PathName.cpp Updated popBytes call site
Svc/CmdSequencer/FPrimeSequence.cpp Updated call site
Svc/CmdSequencer/formats/AMPCSSequence.cpp Updated call site
Svc/FpySequencer/FpySequencerRunState.cpp Updated call site
Svc/DpCatalog/DpCatalog.cpp Updated call site
Svc/PrmDb/PrmDbImpl.cpp Updated call site
Ref/RecvBuffApp/RecvBuffComponentImpl.cpp Updated call site

Future Work

N/A

AI Usage (see policy)

AI was used to generate the new unit tests

Comment thread Fw/Tlm/TlmPacket.cpp Fixed
Comment thread Fw/Tlm/TlmPacket.cpp Fixed
Comment thread Fw/Types/SerialBuffer.cpp Fixed
Comment thread Fw/Types/SerialBuffer.cpp Fixed
Comment thread Fw/Types/SerialBuffer.cpp Fixed
Comment thread Fw/Types/Serializable.cpp Fixed
Comment thread Fw/Types/Serializable.cpp Fixed
Comment thread Fw/Types/Serializable.cpp Fixed
Comment thread Fw/Types/StringBase.cpp Fixed
@thomas-bc thomas-bc added the CCB PR needs to go through CCB process label Apr 20, 2026
@thomas-bc
Copy link
Copy Markdown
Collaborator

Blocked on CCB approval

@thomas-bc thomas-bc added the Breaking Changes / Needs Release Notes Need to add instructions in the release notes for updates. label Apr 20, 2026
Comment thread Fw/Tlm/TlmPacket.cpp Fixed
Comment thread Fw/Tlm/TlmPacket.cpp Fixed
Comment thread Fw/Types/Serializable.cpp Fixed
Comment thread Fw/Types/Serializable.cpp Fixed
Comment thread Fw/Types/Serializable.cpp Fixed
Comment thread Fw/Types/Serializable.cpp Fixed
Comment thread Fw/Types/Serializable.cpp
}

SerializeStatus LinearBufferBase::deserializeTo(U8* buff, Serializable::SizeType& length, Endianness endianMode) {
SerializeStatus LinearBufferBase::deserializeTo(U8* buff,
Comment thread Fw/Tlm/TlmPacket.cpp Fixed
@LeStarch LeStarch self-assigned this Apr 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking Changes / Needs Release Notes Need to add instructions in the release notes for updates. CCB PR needs to go through CCB process

Projects

Status: CCB

Development

Successfully merging this pull request may close these issues.

4 participants