Skip to content

Commit 31282aa

Browse files
Lex-ariLex-ari
committed
Validation Fixes
1 parent 97e491b commit 31282aa

7 files changed

Lines changed: 66 additions & 145 deletions

‎Svc/FpySequencer/FpySequencer.hpp‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -645,9 +645,6 @@ class FpySequencer : public FpySequencerComponentBase {
645645
// sequence arguments to push to stack when entering RUNNING state
646646
Svc::SeqArgs m_sequenceArgs{};
647647

648-
// the expected argument size from arg_specs
649-
Fpy::StackSizeType m_expectedArgSize{0};
650-
651648
// the goal state is the state that we're trying to reach in the sequencer
652649
// if it's RUNNING, then we should promptly go to RUNNING once we validate the
653650
// sequence. if it's VALID, we should wait after VALIDATING
@@ -749,9 +746,6 @@ class FpySequencer : public FpySequencerComponentBase {
749746
// reads and validates the header from the m_sequenceBuffer
750747
// return SUCCESS if sequence is valid, FAILURE otherwise
751748
Fw::Success readHeader();
752-
// reads and parses arg_specs from the sequence file (schema version 6+)
753-
// return SUCCESS if successful, FAILURE otherwise
754-
Fw::Success readArgSpecs(Os::File& file);
755749
// helper function to read and deserialize a variable-length string field (length byte + string bytes)
756750
// returns the length via outLength parameter and writes string data to buffer
757751
Fw::Success deserializeStringField(Os::File& file, U8* buffer, U8& outLength);

‎Svc/FpySequencer/FpySequencerStateMachine.cpp‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,6 @@ void FpySequencer ::Svc_FpySequencer_SequencerStateMachine_action_clearSequenceA
305305
Svc_FpySequencer_SequencerStateMachine::Signal signal
306306
) {
307307
this->m_sequenceArgs = {0, 0};
308-
this->m_expectedArgSize = 0;
309308
}
310309

311310
//! Implementation for action clearBreakpoint of state machine Svc_FpySequencer_SequencerStateMachine

‎Svc/FpySequencer/FpySequencerTypes.fpp‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -138,17 +138,11 @@ module Svc {
138138
@ NOTE: This struct does NOT use FPP's auto-generated serialization!
139139
@ Serialization is handled manually in C++ to match Fpy's format:
140140
struct ArgSpec {
141-
@ Length of the argument name (0-255)
142-
argNameLen: U8
143-
@ Argument name as UTF-8 bytes (not null-terminated)
144-
argName: [MAX_ARG_SPEC_NAME_LEN] U8
145-
@ Length of the type name (0-255)
146-
typeNameLen: U8
147-
@ Type name as UTF-8 bytes (not null-terminated)
148-
typeName: [MAX_ARG_SPEC_NAME_LEN] U8
141+
argName: string size MAX_ARG_SPEC_NAME_LEN
142+
typeName: string size MAX_ARG_SPEC_NAME_LEN
149143
@ Size of this argument on the stack in bytes
150144
argSize: StackSizeType
151-
} default { argNameLen = 0, argName = 0, typeNameLen = 0, typeName = 0, argSize = 0 }
145+
} default {argName = "", typeName = "", argSize = 0 }
152146
153147
struct Header {
154148
@ the major version of the FSW

‎Svc/FpySequencer/FpySequencerValidationState.cpp‎

Lines changed: 38 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,6 @@ Fw::Success FpySequencer::validate() {
6161
return Fw::Success::FAILURE;
6262
}
6363

64-
// Read and parse arg_specs
65-
readStatus = this->readArgSpecs(sequenceFile);
66-
if (readStatus != Fw::Success::SUCCESS) {
67-
return Fw::Success::FAILURE;
68-
}
69-
7064
readStatus =
7165
readBytes(sequenceFile, this->m_sequenceObj.get_header().get_bodySize(), FpySequencer_FileReadStage::BODY);
7266

@@ -111,12 +105,6 @@ Fw::Success FpySequencer::validate() {
111105
return Fw::Success::FAILURE;
112106
}
113107

114-
if (this->m_sequenceArgs.get_size() != this->m_expectedArgSize) {
115-
this->log_WARNING_HI_ArgSizeMismatch(this->m_expectedArgSize, this->m_sequenceArgs.get_size(),
116-
this->m_sequenceFilePath);
117-
return Fw::Success::FAILURE;
118-
}
119-
120108
return Fw::Success::SUCCESS;
121109
}
122110

@@ -153,89 +141,56 @@ Fw::Success FpySequencer::readHeader() {
153141
return Fw::Success::SUCCESS;
154142
}
155143

156-
// Helper function to read and deserialize a variable-length string field
157-
// Reads length byte, then the string data into the provided buffer
158-
Fw::Success FpySequencer::deserializeStringField(Os::File& file, U8* buffer, U8& outLength) {
159-
// Read length byte
160-
Fw::Success readStatus = this->readBytes(file, sizeof(U8), FpySequencer_FileReadStage::BODY);
161-
if (readStatus != Fw::Success::SUCCESS) {
162-
return Fw::Success::FAILURE;
163-
}
164-
165-
Fw::SerializeStatus deserStatus = this->m_sequenceBuffer.deserializeTo(outLength);
166-
if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) {
167-
this->log_WARNING_HI_FileReadDeserializeError(
168-
FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast<I32>(deserStatus),
169-
this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize());
170-
return Fw::Success::FAILURE;
171-
}
172-
173-
// Validate length is non-zero (reject empty strings)
174-
if (outLength == 0) {
175-
this->log_WARNING_HI_InvalidArgSpec(this->m_sequenceFilePath);
176-
return Fw::Success::FAILURE;
177-
}
178-
179-
// Validate length doesn't exceed buffer capacity
180-
if (outLength > Fpy::MAX_ARG_SPEC_NAME_LEN) {
181-
this->log_WARNING_HI_ArgSpecStringLengthExceedsMax(outLength, Fpy::MAX_ARG_SPEC_NAME_LEN, this->m_sequenceFilePath);
182-
return Fw::Success::FAILURE;
183-
}
184-
185-
// Read string bytes
186-
readStatus = this->readBytes(file, outLength, FpySequencer_FileReadStage::BODY);
187-
if (readStatus != Fw::Success::SUCCESS) {
188-
return Fw::Success::FAILURE;
189-
}
190-
191-
FwSizeType actualLen = static_cast<FwSizeType>(outLength);
192-
deserStatus = this->m_sequenceBuffer.deserializeTo(buffer, actualLen, Fw::Serialization::OMIT_LENGTH);
193-
if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) {
194-
this->log_WARNING_HI_FileReadDeserializeError(
195-
FpySequencer_FileReadStage::BODY, this->m_sequenceFilePath, static_cast<I32>(deserStatus),
196-
this->m_sequenceBuffer.getDeserializeSizeLeft(), this->m_sequenceBuffer.getSize());
197-
return Fw::Success::FAILURE;
198-
}
199-
200-
return Fw::Success::SUCCESS;
201-
}
202-
203-
// reads and validates arg_specs from the m_sequenceBuffer
204-
// stores them in the Sequence.args array and calculates the total expected argument size
205-
// return SUCCESS if successful, FAILURE otherwise
206-
Fw::Success FpySequencer::readArgSpecs(Os::File& file) {
207-
FW_ASSERT(file.isOpen());
208-
144+
// reads and validates the body from the m_sequenceBuffer
145+
// return SUCCESS if sequence is valid, FAILURE otherwise
146+
Fw::Success FpySequencer::readBody() {
147+
Fw::SerializeStatus deserStatus;
148+
209149
const U8 argumentCount = this->m_sequenceObj.get_header().get_argumentCount();
210150
Fpy::StackSizeType totalExpectedSize = 0;
211-
Fw::SerializeStatus deserStatus;
212-
Fw::Success readStatus;
213-
214-
// Read and deserialize each arg_spec incrementally since they're variable-length
151+
152+
// deser arguments
153+
// Read and deserialize each arg_spec incrementally since they're variable-length
215154
for (U8 i = 0; i < argumentCount; i++) {
216155
Fpy::ArgSpec& argSpec = this->m_sequenceObj.get_args()[i];
217-
156+
Fw::String myString{};
218157
// Read arg_name (length + string)
219-
U8 argNameLen = 0;
220-
readStatus = this->deserializeStringField(file, argSpec.get_argName(), argNameLen);
221-
if (readStatus != Fw::Success::SUCCESS) {
158+
deserStatus = this->m_sequenceBuffer.deserializeTo(myString);
159+
if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) {
160+
this->log_WARNING_HI_FileReadDeserializeError(
161+
FpySequencer_FileReadStage::BODY,
162+
this->m_sequenceFilePath,
163+
static_cast<I32>(deserStatus),
164+
this->m_sequenceBuffer.getDeserializeSizeLeft(),
165+
this->m_sequenceBuffer.getSize()
166+
);
222167
return Fw::Success::FAILURE;
223168
}
224-
argSpec.set_argNameLen(argNameLen);
169+
if (myString.length() == 0) {
170+
this->log_WARNING_HI_InvalidArgSpec(this->m_sequenceFilePath);
171+
return Fw::Success::FAILURE;
172+
}
173+
argSpec.set_argName(myString);
225174

226175
// Read type_name (length + string)
227-
U8 typeNameLen = 0;
228-
readStatus = this->deserializeStringField(file, argSpec.get_typeName(), typeNameLen);
229-
if (readStatus != Fw::Success::SUCCESS) {
176+
deserStatus = this->m_sequenceBuffer.deserializeTo(myString);
177+
if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) {
178+
this->log_WARNING_HI_FileReadDeserializeError(
179+
FpySequencer_FileReadStage::BODY,
180+
this->m_sequenceFilePath,
181+
static_cast<I32>(deserStatus),
182+
this->m_sequenceBuffer.getDeserializeSizeLeft(),
183+
this->m_sequenceBuffer.getSize()
184+
);
230185
return Fw::Success::FAILURE;
231186
}
232-
argSpec.set_typeNameLen(typeNameLen);
233-
234-
// Read and deserialize size field
235-
readStatus = this->readBytes(file, sizeof(Fpy::StackSizeType), FpySequencer_FileReadStage::BODY);
236-
if (readStatus != Fw::Success::SUCCESS) {
187+
if (myString.length() == 0) {
188+
this->log_WARNING_HI_InvalidArgSpec(this->m_sequenceFilePath);
237189
return Fw::Success::FAILURE;
238190
}
191+
argSpec.set_typeName(myString);
192+
193+
// Read and deserialize size field
239194
Fpy::StackSizeType argSize = 0;
240195
deserStatus = this->m_sequenceBuffer.deserializeTo(argSize);
241196
if (deserStatus != Fw::SerializeStatus::FW_SERIALIZE_OK) {
@@ -254,15 +209,7 @@ Fw::Success FpySequencer::readArgSpecs(Os::File& file) {
254209
argSpec.set_argSize(argSize);
255210
totalExpectedSize += argSize;
256211
}
257-
258-
this->m_expectedArgSize = totalExpectedSize;
259-
return Fw::Success::SUCCESS;
260-
}
261-
262-
// reads and validates the body from the m_sequenceBuffer
263-
// return SUCCESS if sequence is valid, FAILURE otherwise
264-
Fw::Success FpySequencer::readBody() {
265-
Fw::SerializeStatus deserStatus;
212+
266213
// deser statements
267214
for (U16 statementIdx = 0; statementIdx < this->m_sequenceObj.get_header().get_statementCount(); statementIdx++) {
268215
// deser statement

‎Svc/FpySequencer/test/ut/FpySequencerTestMain.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2119,8 +2119,8 @@ TEST_F(FpySequencerTester, cmd_RUN) {
21192119

21202120
TEST_F(FpySequencerTester, cmd_RUN_ARGS) {
21212121
allocMem();
2122-
add_ARG_SPEC("arg1", "U32", sizeof(U32));
2123-
add_ARG_SPEC("arg2", "U32", sizeof(U32));
2122+
addArgumentSpec("arg1", "U32", sizeof(U32));
2123+
addArgumentSpec("arg2", "U32", sizeof(U32));
21242124
add_LOAD_REL(0, sizeof(U32)); // Load first arg (U32 at offset 0) - duplicates it on stack
21252125
add_LOAD_REL(4, sizeof(U32)); // Load second arg (U32 at offset 4) - duplicates it on stack
21262126
add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args
@@ -2249,8 +2249,8 @@ TEST_F(FpySequencerTester, cmd_RUN_VALIDATED) {
22492249

22502250
TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS) {
22512251
allocMem();
2252-
add_ARG_SPEC("arg1", "U32", sizeof(U32));
2253-
add_ARG_SPEC("arg2", "U32", sizeof(U32));
2252+
addArgumentSpec("arg1", "U32", sizeof(U32));
2253+
addArgumentSpec("arg2", "U32", sizeof(U32));
22542254
add_LOAD_REL(0, sizeof(U32)); // Load first arg (U32 at offset 0) - duplicates it on stack
22552255
add_LOAD_REL(4, sizeof(U32)); // Load second arg (U32 at offset 4) - duplicates it on stack
22562256
add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args
@@ -2340,7 +2340,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_arg_name) {
23402340
clearSeq();
23412341

23422342
// Create arg_spec with zero-length arg name (invalid)
2343-
add_ARG_SPEC("", "U32", sizeof(U32)); // Empty string for arg name
2343+
addArgumentSpec("", "U32", sizeof(U32)); // Empty string for arg name
23442344
add_NO_OP();
23452345
writeToFile("test.bin");
23462346

@@ -2368,7 +2368,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_zero_length_type_name) {
23682368
clearSeq();
23692369

23702370
// Create arg_spec with zero-length type name (invalid)
2371-
add_ARG_SPEC("arg1", "", sizeof(U32)); // Empty string for type name
2371+
addArgumentSpec("arg1", "", sizeof(U32)); // Empty string for type name
23722372
add_NO_OP();
23732373
writeToFile("test.bin");
23742374

@@ -2404,7 +2404,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_max_length_strings) {
24042404
memset(maxLengthType, 'B', 255);
24052405
maxLengthType[255] = '\0';
24062406

2407-
add_ARG_SPEC(maxLengthName, maxLengthType, sizeof(U32));
2407+
addArgumentSpec(maxLengthName, maxLengthType, sizeof(U32));
24082408
add_LOAD_REL(0, sizeof(U32));
24092409
add_DISCARD(sizeof(U32) * 2); // Discard loaded copy + original
24102410
writeToFile("test.bin");
@@ -2430,7 +2430,7 @@ TEST_F(FpySequencerTester, cmd_VALIDATE_ARGS_size_mismatch) {
24302430
clearSeq();
24312431

24322432
// Create arg_spec claiming U32 (4 bytes) but provide wrong size
2433-
add_ARG_SPEC("badArg", "U32", sizeof(U64)); // Type says U32, size says U64 (8 bytes)
2433+
addArgumentSpec("badArg", "U32", sizeof(U64)); // Type says U32, size says U64 (8 bytes)
24342434
add_NO_OP();
24352435
writeToFile("test.bin");
24362436

@@ -3450,8 +3450,8 @@ TEST_F(FpySequencerTester, seqRunIn) {
34503450

34513451
TEST_F(FpySequencerTester, seqRunInArgs) {
34523452
allocMem();
3453-
add_ARG_SPEC("arg1", "U32", sizeof(U32));
3454-
add_ARG_SPEC("arg2", "U32", sizeof(U32));
3453+
addArgumentSpec("arg1", "U32", sizeof(U32));
3454+
addArgumentSpec("arg2", "U32", sizeof(U32));
34553455
add_LOAD_REL(0, sizeof(U32)); // Load first arg (U32 at offset 0) - duplicates it on stack
34563456
add_LOAD_REL(4, sizeof(U32)); // Load second arg (U32 at offset 4) - duplicates it on stack
34573457
add_DISCARD(16); // Discard all: 2 loaded copies + 2 original args

‎Svc/FpySequencer/test/ut/FpySequencerTester.cpp‎

Lines changed: 14 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,14 @@ void FpySequencerTester::writeToFile(const char* name, FwSizeType maxBytes) {
6363
Fw::ExternalSerializeBuffer buf;
6464
buf.setExtBuffer(data, sizeof(data));
6565

66-
// Calculate body size (statements only, arg_specs go in header section)
66+
// Calculate body size (statements and arg_specs)
6767
for (U32 ii = 0; ii < seq.get_header().get_statementCount(); ii++) {
6868
ASSERT_EQ(buf.serializeFrom(seq.get_statements()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK);
69+
}
70+
for (U32 ii = 0; ii < seq.get_header().get_argumentCount(); ii++) {
71+
ASSERT_EQ(buf.serializeFrom(seq.get_args()[ii]), Fw::SerializeStatus::FW_SERIALIZE_OK);
6972
}
73+
7074
seq.get_header().set_bodySize(static_cast<U32>(buf.getSize()));
7175
buf.resetSer();
7276

@@ -76,18 +80,9 @@ void FpySequencerTester::writeToFile(const char* name, FwSizeType maxBytes) {
7680
// Write arg_specs in Fpy variable-length format
7781
for (U32 ii = 0; ii < seq.get_header().get_argumentCount(); ii++) {
7882
const Fpy::ArgSpec& argSpec = seq.get_args()[ii];
79-
// Write arg_name length and bytes (without F Prime length prefix)
80-
ASSERT_EQ(buf.serializeFrom(argSpec.get_argNameLen()), Fw::SerializeStatus::FW_SERIALIZE_OK);
81-
if (argSpec.get_argNameLen() > 0) {
82-
ASSERT_EQ(buf.serializeFrom(argSpec.get_argName(), argSpec.get_argNameLen(), Fw::Serialization::OMIT_LENGTH),
83-
Fw::SerializeStatus::FW_SERIALIZE_OK);
84-
}
85-
// Write type_name length and bytes (without F Prime length prefix)
86-
ASSERT_EQ(buf.serializeFrom(argSpec.get_typeNameLen()), Fw::SerializeStatus::FW_SERIALIZE_OK);
87-
if (argSpec.get_typeNameLen() > 0) {
88-
ASSERT_EQ(buf.serializeFrom(argSpec.get_typeName(), argSpec.get_typeNameLen(), Fw::Serialization::OMIT_LENGTH),
89-
Fw::SerializeStatus::FW_SERIALIZE_OK);
90-
}
83+
ASSERT_EQ(buf.serializeFrom(argSpec.get_argName()), Fw::SerializeStatus::FW_SERIALIZE_OK);
84+
85+
ASSERT_EQ(buf.serializeFrom(argSpec.get_typeName()), Fw::SerializeStatus::FW_SERIALIZE_OK);
9186
// Write size
9287
ASSERT_EQ(buf.serializeFrom(argSpec.get_argSize()), Fw::SerializeStatus::FW_SERIALIZE_OK);
9388
}
@@ -119,33 +114,25 @@ void FpySequencerTester::removeFile(const char* name) {
119114
Os::FileSystem::removeFile(name);
120115
}
121116

122-
void FpySequencerTester::add_ARG_SPEC(const char* argName, const char* typeName, Fpy::StackSizeType argSize) {
117+
void FpySequencerTester::addArgumentSpec(Fw::String argName, Fw::String typeName, Fpy::StackSizeType argSize) {
123118
U8 argCount = seq.get_header().get_argumentCount();
124119
FW_ASSERT(argCount < Fpy::MAX_SEQUENCE_ARG_COUNT);
125120

126121
Fpy::ArgSpec& argSpec = seq.get_args()[argCount];
127122

128123
// Set arg_name
129-
U8 argNameLen = static_cast<U8>(strlen(argName));
130-
FW_ASSERT(argNameLen <= Fpy::MAX_ARG_SPEC_NAME_LEN);
131-
argSpec.set_argNameLen(argNameLen);
132-
for (U8 i = 0; i < argNameLen; i++) {
133-
argSpec.get_argName()[i] = static_cast<U8>(argName[i]);
134-
}
124+
FW_ASSERT(argName.length() <= Fpy::MAX_ARG_SPEC_NAME_LEN);
125+
argSpec.set_argName(argName);
135126

136127
// Set type_name
137-
U8 typeNameLen = static_cast<U8>(strlen(typeName));
138-
FW_ASSERT(typeNameLen <= Fpy::MAX_ARG_SPEC_NAME_LEN);
139-
argSpec.set_typeNameLen(typeNameLen);
140-
for (U8 i = 0; i < typeNameLen; i++) {
141-
argSpec.get_typeName()[i] = static_cast<U8>(typeName[i]);
142-
}
128+
FW_ASSERT(typeName.length() <= Fpy::MAX_ARG_SPEC_NAME_LEN);
129+
argSpec.set_typeName(typeName);
143130

144131
// Set size
145132
argSpec.set_argSize(argSize);
146133

147134
// Increment argument count
148-
seq.get_header().set_argumentCount(argCount + 1);
135+
seq.get_header().set_argumentCount(++argCount);
149136
}
150137

151138
void FpySequencerTester::resetRuntime() {

‎Svc/FpySequencer/test/ut/FpySequencerTester.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class FpySequencerTester : public FpySequencerGTestBase, public ::testing::Test
7979
void writeToFile(const char* name, FwSizeType maxBytes = Fpy::Sequence::SERIALIZED_SIZE);
8080
void removeFile(const char* name);
8181
void addDirective(Fpy::DirectiveId id, Fw::StatementArgBuffer& buf);
82-
void add_ARG_SPEC(const char* argName, const char* typeName, Fpy::StackSizeType argSize);
82+
void addArgumentSpec(Fw::String argName, Fw::String typeName, Fpy::StackSizeType argSize);
8383

8484
void add_WAIT_REL();
8585
void add_WAIT_REL(FpySequencer_WaitRelDirective dir);

0 commit comments

Comments
 (0)