From f9c98aa1cf17936554e33766913c9b06d9dac4cb Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 12 Oct 2023 16:27:57 -0400 Subject: [PATCH] Fix #118, implement range checking functions Adds inline functions to do range checking, and uses them in all places where the same logic had been copied around. This reduces repetition of logic. Introduces proper data types and wrapper functions to deal with the different types of IDs and indices. --- fsw/src/sc_atsrq.c | 30 +++++++++++++---------- fsw/src/sc_cmds.c | 9 ++++--- fsw/src/sc_index_types.h | 16 +++++++++++++ fsw/src/sc_loads.c | 49 +++++++++++++++++++------------------- fsw/src/sc_rtsrq.c | 32 +++++++++++-------------- fsw/src/sc_state.c | 28 ++++++++++++---------- unit-test/sc_atsrq_tests.c | 25 +++++++++++++++---- unit-test/sc_loads_tests.c | 39 ++++++++++++++++++++++++++++++ unit-test/sc_rtsrq_tests.c | 12 +++++----- 9 files changed, 157 insertions(+), 83 deletions(-) diff --git a/fsw/src/sc_atsrq.c b/fsw/src/sc_atsrq.c index 6976511..0a15021 100644 --- a/fsw/src/sc_atsrq.c +++ b/fsw/src/sc_atsrq.c @@ -56,7 +56,7 @@ void SC_StartAtsCmd(const SC_StartAtsCmd_t *Cmd) AtsNum = Cmd->Payload.AtsNum; /* validate ATS ID */ - if ((AtsNum > 0) && (AtsNum <= SC_NUMBER_OF_ATS)) + if (SC_AtsNumIsValid(AtsNum)) { /* convert ATS ID to array index */ AtsIndex = SC_AtsNumToIndex(AtsNum); @@ -133,19 +133,22 @@ void SC_StopAtsCmd(const SC_StopAtsCmd_t *Cmd) int32 Result = SC_ERROR; /* - ** Set the temp ATS ID if it is valid + ** Check if the ATS ID is valid */ + if (SC_AtsNumIsValid(SC_OperData.AtsCtrlBlckAddr->CurrAtsNum)) + { + Result = CFE_SUCCESS; + } + if (SC_OperData.AtsCtrlBlckAddr->CurrAtsNum == SC_AtsId_ATSA) { TempAtsChar = 'A'; - Result = CFE_SUCCESS; } else { if (SC_OperData.AtsCtrlBlckAddr->CurrAtsNum == SC_AtsId_ATSB) { TempAtsChar = 'B'; - Result = CFE_SUCCESS; } } @@ -186,7 +189,7 @@ bool SC_BeginAts(SC_AtsIndex_t AtsIndex, uint16 TimeOffset) SC_AtsCmdStatusEntry_t * StatusEntryPtr; /* validate ATS array index */ - if (AtsIndex >= SC_NUMBER_OF_ATS) + if (!SC_AtsIndexIsValid(AtsIndex)) { CFE_EVS_SendEvent(SC_BEGINATS_INVLD_INDEX_ERR_EID, CFE_EVS_EventType_ERROR, "Begin ATS error: invalid ATS index %d", AtsIndex); @@ -203,7 +206,7 @@ bool SC_BeginAts(SC_AtsIndex_t AtsIndex, uint16 TimeOffset) */ TimeIndex = SC_SEQUENCE_IDX_FIRST; /* pointer into the time index table */ - while (TimeIndex < AtsInfoPtr->NumberOfCommands) + while (SC_IDX_WITHIN_LIMIT(TimeIndex, AtsInfoPtr->NumberOfCommands)) { /* first get the cmd index at this list entry */ CmdIndex = SC_CommandNumToIndex(SC_GetAtsCommandNumAtSeq(AtsIndex, TimeIndex)->CmdNum); @@ -222,7 +225,7 @@ bool SC_BeginAts(SC_AtsIndex_t AtsIndex, uint16 TimeOffset) StatusEntryPtr->Status = SC_Status_SKIPPED; CmdsSkipped++; - TimeIndex++; + SC_IDX_INCREMENT(TimeIndex); } else { @@ -234,7 +237,7 @@ bool SC_BeginAts(SC_AtsIndex_t AtsIndex, uint16 TimeOffset) /* ** Check to see if the whole ATS was skipped */ - if (TimeIndex == AtsInfoPtr->NumberOfCommands) + if (!SC_IDX_WITHIN_LIMIT(TimeIndex, AtsInfoPtr->NumberOfCommands)) { CFE_EVS_SendEvent(SC_ATS_SKP_ALL_ERR_EID, CFE_EVS_EventType_ERROR, "All ATS commands were skipped, ATS stopped"); @@ -311,7 +314,8 @@ void SC_SwitchAtsCmd(const SC_SwitchAtsCmd_t *Cmd) SC_AtsInfoTable_t *AtsInfoPtr; /* make sure that an ATS is running on the ATP */ - if (SC_OperData.AtsCtrlBlckAddr->AtpState == SC_Status_EXECUTING) + if (SC_AtsNumIsValid(SC_OperData.AtsCtrlBlckAddr->CurrAtsNum) && + SC_OperData.AtsCtrlBlckAddr->AtpState == SC_Status_EXECUTING) { /* get the ATS to switch to */ NewAtsIndex = SC_ToggleAtsIndex(); @@ -530,7 +534,7 @@ void SC_JumpAtsCmd(const SC_JumpAtsCmd_t *Cmd) TimeIndex = SC_SEQUENCE_IDX_FIRST; NumSkipped = 0; - while (TimeIndex < AtsInfoPtr->NumberOfCommands) + while (SC_IDX_WITHIN_LIMIT(TimeIndex, AtsInfoPtr->NumberOfCommands)) { /* first get the cmd index at this list entry */ CmdIndex = SC_CommandNumToIndex(SC_GetAtsCommandNumAtSeq(AtsIndex, TimeIndex)->CmdNum); @@ -559,7 +563,7 @@ void SC_JumpAtsCmd(const SC_JumpAtsCmd_t *Cmd) NumSkipped++; } - TimeIndex++; + SC_IDX_INCREMENT(TimeIndex); } else { @@ -573,7 +577,7 @@ void SC_JumpAtsCmd(const SC_JumpAtsCmd_t *Cmd) /* ** Check to see if the whole ATS was skipped */ - if (TimeIndex == AtsInfoPtr->NumberOfCommands) + if (!SC_IDX_WITHIN_LIMIT(TimeIndex, AtsInfoPtr->NumberOfCommands)) { CFE_EVS_SendEvent(SC_JUMPATS_CMD_STOPPED_ERR_EID, CFE_EVS_EventType_ERROR, "Jump Cmd: All ATS commands were skipped, ATS stopped"); @@ -665,7 +669,7 @@ void SC_AppendAtsCmd(const SC_AppendAtsCmd_t *Cmd) SC_AtsIndex_t AtsIndex; /* index (not ID) of target ATS */ SC_AtsInfoTable_t *AtsInfoPtr; - if ((Cmd->Payload.AtsNum == 0) || (Cmd->Payload.AtsNum > SC_NUMBER_OF_ATS)) + if (!SC_AtsNumIsValid(Cmd->Payload.AtsNum)) { /* invalid target ATS selection */ SC_OperData.HkPacket.Payload.CmdErrCtr++; diff --git a/fsw/src/sc_cmds.c b/fsw/src/sc_cmds.c index 0f24d9a..61e9506 100644 --- a/fsw/src/sc_cmds.c +++ b/fsw/src/sc_cmds.c @@ -313,12 +313,15 @@ void SC_ProcessRtpCommand(void) */ /* convert the RTS number so that it can be directly indexed into the table*/ RtsIndex = SC_RtsNumToIndex(SC_OperData.RtsCtrlBlckAddr->CurrRtsNum); + if (!SC_RtsIndexIsValid(RtsIndex)) + { + return; + } RtsInfoPtr = SC_GetRtsInfoObject(RtsIndex); if ((SC_AppData.NextCmdTime[SC_AppData.NextProcNumber] <= SC_AppData.CurrentTime) && - (SC_AppData.NextProcNumber == SC_Process_RTP) && (SC_OperData.RtsCtrlBlckAddr->CurrRtsNum > 0) && - (SC_OperData.RtsCtrlBlckAddr->CurrRtsNum <= SC_NUMBER_OF_RTS) && (RtsInfoPtr->RtsStatus == SC_Status_EXECUTING)) + (SC_AppData.NextProcNumber == SC_Process_RTP) && (RtsInfoPtr->RtsStatus == SC_Status_EXECUTING)) { /* ** Count the command for the rate limiter @@ -494,7 +497,7 @@ void SC_SendHkCmd(const SC_SendHkCmd_t *Cmd) SC_RtsInfoEntry_t *RtsInfoPtr; /* set during init to power on or processor reset auto-exec RTS */ - if (SC_AppData.AutoStartRTS != 0) + if (SC_RtsNumIsValid(SC_AppData.AutoStartRTS)) { RtsInfoPtr = SC_GetRtsInfoObject(SC_RtsNumToIndex(SC_AppData.AutoStartRTS)); diff --git a/fsw/src/sc_index_types.h b/fsw/src/sc_index_types.h index cfc1e63..9ddf461 100644 --- a/fsw/src/sc_index_types.h +++ b/fsw/src/sc_index_types.h @@ -393,4 +393,20 @@ static inline bool SC_RtsNumValidateRange(SC_RtsNum_t FirstRtsNum, SC_RtsNum_t L SC_RtsNumWithinRange(FirstRtsNum, LastRtsNum); } +/** + * @brief Advance the entry offset by the given amount + * + * Entries in ATS/RTS buffers are always aligned to 32-bit words. This advances the + * offset by the specified number of bytes. The given number of bytes will be converted + * to words, rounding up as needed to get to the next word boundary. + * + * @param Pos Starting entry offset + * @param Bytes Amount to advance, in bytes + * @returns Updated entry offset + */ +static inline SC_EntryOffset_t SC_EntryOffsetAdvance(SC_EntryOffset_t Pos, size_t Bytes) +{ + return (SC_EntryOffset_t) {SC_IDX_AS_UINT(Pos) + ((Bytes + sizeof(uint32) - 1) / sizeof(uint32))}; +} + #endif diff --git a/fsw/src/sc_loads.c b/fsw/src/sc_loads.c index c77d0de..fce45a3 100644 --- a/fsw/src/sc_loads.c +++ b/fsw/src/sc_loads.c @@ -51,10 +51,10 @@ /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ void SC_LoadAts(SC_AtsIndex_t AtsIndex) { - uint16 AtsEntryWords; /* current ats entry length in words */ - SC_CommandNum_t AtsCmdNum; /* current ats entry command number */ - SC_EntryOffset_t AtsEntryIndex; /* index into the load for current ats entry */ - SC_AtsEntry_t * EntryPtr; /* a pointer to an ats entry */ + SC_CommandNum_t AtsCmdNum; /* current ats entry command number */ + SC_EntryOffset_t AtsEntryIndex; /* index into the load for current ats entry */ + SC_EntryOffset_t PendingEntryIndex; /* index into the load for current ats entry */ + SC_AtsEntry_t * EntryPtr; /* a pointer to an ats entry */ CFE_MSG_Size_t MessageSize = 0; int32 Result = CFE_SUCCESS; bool StillProcessing = true; @@ -63,7 +63,7 @@ void SC_LoadAts(SC_AtsIndex_t AtsIndex) SC_AtsCmdEntryOffsetRecord_t *CmdOffsetRec; /* ATS entry location in table */ /* validate ATS array index */ - if (AtsIndex >= SC_NUMBER_OF_ATS) + if (!SC_AtsIndexIsValid(AtsIndex)) { CFE_EVS_SendEvent(SC_LOADATS_INV_INDEX_ERR_EID, CFE_EVS_EventType_ERROR, "ATS load error: invalid ATS index %d", AtsIndex); @@ -85,7 +85,7 @@ void SC_LoadAts(SC_AtsIndex_t AtsIndex) ** Make sure that the pointer as well as the primary packet ** header fit in the buffer, so a G.P fault is not caused. */ - if (AtsEntryIndex < SC_ATS_BUFF_SIZE32) + if (SC_IDX_WITHIN_LIMIT(AtsEntryIndex, SC_ATS_BUFF_SIZE32)) { /* get a pointer to the ats command in the table */ EntryPtr = SC_GetAtsEntryAtOffset(AtsIndex, AtsEntryIndex); @@ -93,7 +93,7 @@ void SC_LoadAts(SC_AtsIndex_t AtsIndex) /* get the next command number from the buffer */ AtsCmdNum = EntryPtr->Header.CmdNumber; - if (AtsCmdNum == 0) + if (SC_IDNUM_IS_NULL(AtsCmdNum)) { /* end of the load reached */ Result = CFE_SUCCESS; @@ -108,7 +108,8 @@ void SC_LoadAts(SC_AtsIndex_t AtsIndex) } /* make sure the CmdPtr can fit in a whole Ats Cmd Header at the very least */ - if (AtsEntryIndex > (SC_ATS_BUFF_SIZE32 - SC_ATS_HDR_WORDS)) + if (!SC_IDX_WITHIN_LIMIT(AtsEntryIndex, + 1 + (SC_ATS_BUFF_SIZE32 - SC_ATS_HDR_WORDS))) /* jphfix - revisit? */ { /* even the smallest command will not fit in the buffer */ Result = SC_ERROR; @@ -127,10 +128,10 @@ void SC_LoadAts(SC_AtsIndex_t AtsIndex) if (MessageSize >= SC_PACKET_MIN_SIZE && MessageSize <= SC_PACKET_MAX_SIZE) { /* get the length of the entry in WORDS (plus 1 to round byte len up to word len) */ - AtsEntryWords = ((MessageSize + SC_ROUND_UP_BYTES) / SC_BYTES_IN_WORD) + SC_ATS_HDR_NOPKT_WORDS; + PendingEntryIndex = SC_EntryOffsetAdvance(AtsEntryIndex, MessageSize + SC_ATS_HEADER_SIZE); /* if the command does not run off of the end of the buffer */ - if (AtsEntryIndex + AtsEntryWords <= SC_ATS_BUFF_SIZE32) + if (SC_IDX_WITHIN_LIMIT(PendingEntryIndex, 1 + SC_ATS_BUFF_SIZE32)) { /* set the command pointer in the command index table */ /* CmdNum starts at one.... */ @@ -144,7 +145,7 @@ void SC_LoadAts(SC_AtsIndex_t AtsIndex) AtsInfoPtr->NumberOfCommands++; /* increment the ats_entry index to the next ats entry */ - AtsEntryIndex = AtsEntryIndex + AtsEntryWords; + AtsEntryIndex = PendingEntryIndex; } else { /* the command runs off the end of the buffer */ @@ -212,7 +213,7 @@ void SC_BuildTimeIndexTable(SC_AtsIndex_t AtsIndex) SC_AtsCmdNumRecord_t *AtsCmdNumRec; /* validate ATS array index */ - if (AtsIndex >= SC_NUMBER_OF_ATS) + if (!SC_AtsIndexIsValid(AtsIndex)) { CFE_EVS_SendEvent(SC_BUILD_TIME_IDXTBL_ERR_EID, CFE_EVS_EventType_ERROR, "Build time index table error: invalid ATS index %d", AtsIndex); @@ -255,7 +256,7 @@ void SC_Insert(SC_AtsIndex_t AtsIndex, SC_CommandIndex_t NewCmdIndex, uint32 Lis SC_AtsCmdNumRecord_t * AtsCmdNumRec; /* validate ATS array index */ - if (AtsIndex >= SC_NUMBER_OF_ATS) + if (!SC_AtsIndexIsValid(AtsIndex)) { CFE_EVS_SendEvent(SC_INSERTATS_INV_INDEX_ERR_EID, CFE_EVS_EventType_ERROR, "ATS insert error: invalid ATS index %d", AtsIndex); @@ -274,9 +275,9 @@ void SC_Insert(SC_AtsIndex_t AtsIndex, SC_CommandIndex_t NewCmdIndex, uint32 Lis } /* start at last element in the sorted by time list */ - TimeBufIndex = ListLength - 1; + TimeBufIndex = SC_SEQUENCE_IDX_C(ListLength - 1); - while (TimeBufIndex >= 0) + while (SC_IDX_WITHIN_LIMIT(TimeBufIndex, ListLength)) { /* first get the cmd index for this list entry */ CmdIndex = SC_CommandNumToIndex(SC_GetAtsCommandNumAtSeq(AtsIndex, TimeBufIndex)->CmdNum); @@ -300,7 +301,7 @@ void SC_Insert(SC_AtsIndex_t AtsIndex, SC_CommandIndex_t NewCmdIndex, uint32 Lis SC_GetAtsCommandNumAtSeq(AtsIndex, TimeBufIndex)->CmdNum; /* back up to previous list entry (ok if -1) */ - TimeBufIndex--; + SC_IDX_DECREMENT(TimeBufIndex); } else { @@ -336,7 +337,7 @@ void SC_InitAtsTables(SC_AtsIndex_t AtsIndex) SC_AtsCmdNumRecord_t * AtsCmdNumRec; /* validate ATS array index */ - if (AtsIndex >= SC_NUMBER_OF_ATS) + if (!SC_AtsIndexIsValid(AtsIndex)) { CFE_EVS_SendEvent(SC_INIT_ATSTBL_INV_INDEX_ERR_EID, CFE_EVS_EventType_ERROR, "ATS table init error: invalid ATS index %d", AtsIndex); @@ -371,7 +372,7 @@ void SC_LoadRts(SC_RtsIndex_t RtsIndex) SC_RtsInfoEntry_t *RtsInfoPtr; /* validate RTS array index */ - if (RtsIndex < SC_NUMBER_OF_RTS) + if (SC_RtsIndexIsValid(RtsIndex)) { /* Clear out the RTS info table */ RtsInfoPtr = SC_GetRtsInfoObject(RtsIndex); @@ -585,7 +586,7 @@ void SC_UpdateAppend(void) { EntryPtr = (SC_AtsEntry_t *)&SC_OperData.AppendTblAddr[EntryIndex]; - if ((EntryPtr->Header.CmdNumber == 0) || (EntryPtr->Header.CmdNumber > SC_MAX_ATS_CMDS)) + if (!SC_AtsCommandNumIsValid(EntryPtr->Header.CmdNumber)) { /* End of valid command numbers */ StillProcessing = false; @@ -636,7 +637,6 @@ void SC_ProcessAppend(SC_AtsIndex_t AtsIndex) { SC_AtsEntry_t * EntryPtr; CFE_MSG_Size_t CommandBytes = 0; - int32 CommandWords; SC_EntryOffset_t EntryIndex; int32 i; SC_CommandIndex_t CmdIndex; @@ -645,7 +645,7 @@ void SC_ProcessAppend(SC_AtsIndex_t AtsIndex) SC_AtsCmdEntryOffsetRecord_t *CmdOffsetRec; /* validate ATS array index */ - if (AtsIndex >= SC_NUMBER_OF_ATS) + if (!SC_AtsIndexIsValid(AtsIndex)) { CFE_EVS_SendEvent(SC_PROCESS_APPEND_INV_INDEX_ERR_EID, CFE_EVS_EventType_ERROR, "ATS process append error: invalid ATS index %d", AtsIndex); @@ -687,8 +687,7 @@ void SC_ProcessAppend(SC_AtsIndex_t AtsIndex) /* update entry index to point to the next entry */ CFE_MSG_GetSize(CFE_MSG_PTR(EntryPtr->Msg), &CommandBytes); - CommandWords = (CommandBytes + SC_ROUND_UP_BYTES) / SC_BYTES_IN_WORD; - EntryIndex += (SC_ATS_HDR_NOPKT_WORDS + CommandWords); + EntryIndex = SC_EntryOffsetAdvance(EntryIndex, CommandBytes + SC_ATS_HEADER_SIZE); } /* rebuild time sorted list of commands */ @@ -710,7 +709,7 @@ void SC_ProcessAppend(SC_AtsIndex_t AtsIndex) } /* notify cFE that we have modified the ats table */ - CFE_TBL_Modified(SC_OperData.AtsTblHandle[AtsIndex]); + CFE_TBL_Modified(SC_OperData.AtsTblHandle[SC_IDX_AS_UINT(AtsIndex)]); } /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ @@ -818,7 +817,7 @@ int32 SC_VerifyAtsEntry(uint32 *Buffer32, int32 EntryIndex, int32 BufferWords) /* All done -- end of in-use portion of buffer */ Result = CFE_SUCCESS; } - else if (EntryPtr->Header.CmdNumber > SC_MAX_ATS_CMDS) + else if (!SC_AtsCommandNumIsValid(EntryPtr->Header.CmdNumber)) { /* Error -- invalid command number */ Result = SC_ERROR; diff --git a/fsw/src/sc_rtsrq.c b/fsw/src/sc_rtsrq.c index c12dc37..b5d4de0 100644 --- a/fsw/src/sc_rtsrq.c +++ b/fsw/src/sc_rtsrq.c @@ -62,7 +62,7 @@ void SC_StartRtsCmd(const SC_StartRtsCmd_t *Cmd) */ RtsNum = Cmd->Payload.RtsNum; - if ((RtsNum > 0) && (RtsNum <= SC_NUMBER_OF_RTS)) + if (SC_RtsNumIsValid(RtsNum)) { /* convert RTS ID to RTS array index */ RtsIndex = SC_RtsNumToIndex(RtsNum); @@ -181,14 +181,13 @@ void SC_StartRtsGrpCmd(const SC_StartRtsGrpCmd_t *Cmd) LastRtsNum = Cmd->Payload.LastRtsNum; /* make sure the specified group is valid */ - if ((FirstRtsNum > 0) && (LastRtsNum > 0) && (FirstRtsNum <= SC_NUMBER_OF_RTS) && - (LastRtsNum <= SC_NUMBER_OF_RTS) && (FirstRtsNum <= LastRtsNum)) + if (SC_RtsNumValidateRange(FirstRtsNum, LastRtsNum)) { /* convert RTS ID to RTS array index */ FirstIndex = SC_RtsNumToIndex(FirstRtsNum); LastIndex = SC_RtsNumToIndex(LastRtsNum); - for (RtsIndex = FirstIndex; RtsIndex <= LastIndex; RtsIndex++) + for (RtsIndex = FirstIndex; SC_IDX_LESS_OR_EQ(RtsIndex, LastIndex); SC_IDX_INCREMENT(RtsIndex)) { RtsInfoPtr = SC_GetRtsInfoObject(RtsIndex); @@ -264,7 +263,7 @@ void SC_StopRtsCmd(const SC_StopRtsCmd_t *Cmd) RtsNum = Cmd->Payload.RtsNum; /* check the command parameter */ - if (RtsNum <= SC_NUMBER_OF_RTS) + if (SC_RtsNumIsValid(RtsNum)) { /* convert RTS ID to RTS array index */ RtsIndex = SC_RtsNumToIndex(RtsNum); @@ -307,14 +306,13 @@ void SC_StopRtsGrpCmd(const SC_StopRtsGrpCmd_t *Cmd) LastRtsNum = Cmd->Payload.LastRtsNum; /* make sure the specified group is valid */ - if ((FirstRtsNum > 0) && (LastRtsNum > 0) && (FirstRtsNum <= SC_NUMBER_OF_RTS) && - (LastRtsNum <= SC_NUMBER_OF_RTS) && (FirstRtsNum <= LastRtsNum)) + if (SC_RtsNumValidateRange(FirstRtsNum, LastRtsNum)) { /* convert RTS ID to RTS array index */ FirstIndex = SC_RtsNumToIndex(FirstRtsNum); LastIndex = SC_RtsNumToIndex(LastRtsNum); - for (RtsIndex = FirstIndex; RtsIndex <= LastIndex; RtsIndex++) + for (RtsIndex = FirstIndex; SC_IDX_LESS_OR_EQ(RtsIndex, LastIndex); SC_IDX_INCREMENT(RtsIndex)) { RtsInfoPtr = SC_GetRtsInfoObject(RtsIndex); @@ -354,7 +352,7 @@ void SC_DisableRtsCmd(const SC_DisableRtsCmd_t *Cmd) RtsNum = Cmd->Payload.RtsNum; /* make sure tha specified rts is valid */ - if (RtsNum <= SC_NUMBER_OF_RTS) + if (SC_RtsNumIsValid(RtsNum)) { /* convert RTS ID to RTS array index */ RtsIndex = SC_RtsNumToIndex(RtsNum); @@ -397,14 +395,13 @@ void SC_DisableRtsGrpCmd(const SC_DisableRtsGrpCmd_t *Cmd) LastRtsNum = Cmd->Payload.LastRtsNum; /* make sure the specified group is valid */ - if ((FirstRtsNum > 0) && (LastRtsNum > 0) && (FirstRtsNum <= SC_NUMBER_OF_RTS) && - (LastRtsNum <= SC_NUMBER_OF_RTS) && (FirstRtsNum <= LastRtsNum)) + if (SC_RtsNumValidateRange(FirstRtsNum, LastRtsNum)) { /* convert RTS ID to RTS array index */ FirstIndex = SC_RtsNumToIndex(FirstRtsNum); LastIndex = SC_RtsNumToIndex(LastRtsNum); - for (RtsIndex = FirstIndex; RtsIndex <= LastIndex; RtsIndex++) + for (RtsIndex = FirstIndex; SC_IDX_LESS_OR_EQ(RtsIndex, LastIndex); SC_IDX_INCREMENT(RtsIndex)) { RtsInfoPtr = SC_GetRtsInfoObject(RtsIndex); @@ -444,7 +441,7 @@ void SC_EnableRtsCmd(const SC_EnableRtsCmd_t *Cmd) RtsNum = Cmd->Payload.RtsNum; /* make sure the specified rts is valid */ - if ((RtsNum > 0) && (RtsNum <= SC_NUMBER_OF_RTS)) + if (SC_RtsNumIsValid(RtsNum)) { /* convert RTS ID to RTS array index */ RtsIndex = SC_RtsNumToIndex(RtsNum); @@ -488,14 +485,13 @@ void SC_EnableRtsGrpCmd(const SC_EnableRtsGrpCmd_t *Cmd) LastRtsNum = Cmd->Payload.LastRtsNum; /* make sure the specified group is valid */ - if ((FirstRtsNum > 0) && (LastRtsNum > 0) && (FirstRtsNum <= SC_NUMBER_OF_RTS) && - (LastRtsNum <= SC_NUMBER_OF_RTS) && (FirstRtsNum <= LastRtsNum)) + if (SC_RtsNumValidateRange(FirstRtsNum, LastRtsNum)) { /* convert RTS ID to RTS array index */ FirstIndex = SC_RtsNumToIndex(FirstRtsNum); LastIndex = SC_RtsNumToIndex(LastRtsNum); - for (RtsIndex = FirstIndex; RtsIndex <= LastIndex; RtsIndex++) + for (RtsIndex = FirstIndex; SC_IDX_LESS_OR_EQ(RtsIndex, LastIndex); SC_IDX_INCREMENT(RtsIndex)) { RtsInfoPtr = SC_GetRtsInfoObject(RtsIndex); @@ -533,7 +529,7 @@ void SC_KillRts(SC_RtsIndex_t RtsIndex) RtsInfoPtr = SC_GetRtsInfoObject(RtsIndex); /* validate RTS array index */ - if (RtsIndex >= SC_NUMBER_OF_RTS) + if (!SC_RtsIndexIsValid(RtsIndex)) { CFE_EVS_SendEvent(SC_KILLRTS_INV_INDEX_ERR_EID, CFE_EVS_EventType_ERROR, "RTS kill error: invalid RTS index %d", RtsIndex); @@ -571,7 +567,7 @@ void SC_AutoStartRts(SC_RtsNum_t RtsNum) memset(&CmdPkt, 0, sizeof(CmdPkt)); /* validate RTS ID */ - if ((RtsNum > 0) && (RtsNum <= SC_NUMBER_OF_RTS)) + if (SC_RtsNumIsValid(RtsNum)) { /* ** Format the command packet to start the first RTS diff --git a/fsw/src/sc_state.c b/fsw/src/sc_state.c index 02fd216..5aba46f 100644 --- a/fsw/src/sc_state.c +++ b/fsw/src/sc_state.c @@ -84,7 +84,7 @@ void SC_GetNextRtsTime(void) } /* end if */ } /* end for */ - if (NextRts == SC_INVALID_RTS_INDEX) + if (!SC_RtsIndexIsValid(NextRts)) { SC_OperData.RtsCtrlBlckAddr->CurrRtsNum = SC_RTS_NUM_NULL; SC_AppData.NextCmdTime[SC_Process_RTP] = SC_MAX_TIME; @@ -125,7 +125,7 @@ void SC_UpdateNextTime(void) ** This is determined by the RTS number in the RTP control block ** If it is zero, there is no RTS that needs to run */ - if (SC_OperData.RtsCtrlBlckAddr->CurrRtsNum > 0) + if (SC_RtsNumIsValid(SC_OperData.RtsCtrlBlckAddr->CurrRtsNum)) { /* ** If the RTP needs to send commands, only send them if @@ -148,6 +148,7 @@ void SC_GetNextRtsCommand(void) { SC_RtsIndex_t RtsIndex; SC_EntryOffset_t CmdOffset; + SC_EntryOffset_t PendingOffset; SC_RtsEntry_t * EntryPtr; CFE_MSG_Size_t CmdLength = 0; SC_RtsInfoEntry_t *RtsInfoPtr; @@ -155,12 +156,11 @@ void SC_GetNextRtsCommand(void) /* ** Make sure that the RTP is executing some RTS */ - - if ((SC_OperData.RtsCtrlBlckAddr->CurrRtsNum > 0) && (SC_OperData.RtsCtrlBlckAddr->CurrRtsNum <= SC_NUMBER_OF_RTS)) + RtsIndex = SC_RtsNumToIndex(SC_OperData.RtsCtrlBlckAddr->CurrRtsNum); + if (SC_RtsIndexIsValid(RtsIndex)) { - /* Get the index of the rts that is running */ - RtsIndex = SC_RtsNumToIndex(SC_OperData.RtsCtrlBlckAddr->CurrRtsNum); RtsInfoPtr = SC_GetRtsInfoObject(RtsIndex); + /* ** Find out if the RTS is EXECUTING or just STARTED */ @@ -181,7 +181,7 @@ void SC_GetNextRtsCommand(void) ** (plus 1 to round byte len up to word len) */ - CmdOffset = CmdOffset + ((CmdLength + SC_ROUND_UP_BYTES) / SC_BYTES_IN_WORD); + CmdOffset = SC_EntryOffsetAdvance(CmdOffset, CmdLength); /* ** if the end of the buffer is not reached. ** This check is made to make sure that at least the minimum @@ -190,7 +190,7 @@ void SC_GetNextRtsCommand(void) */ /* If at least the header for a command plus the RTS header can fit in the buffer */ - if (CmdOffset <= (SC_RTS_BUFF_SIZE32 - SC_RTS_HDR_WORDS)) + if (SC_IDX_WITHIN_LIMIT(CmdOffset, 1 + SC_RTS_BUFF_SIZE32 - SC_RTS_HDR_WORDS)) { /* ** Get the next RTS command @@ -221,7 +221,8 @@ void SC_GetNextRtsCommand(void) ** runs off of the end of the buffer ** (plus 1 to round byte len up to word len) */ - if (CmdOffset + ((CmdLength + SC_ROUND_UP_BYTES) / SC_BYTES_IN_WORD) <= SC_RTS_BUFF_SIZE32) + PendingOffset = SC_EntryOffsetAdvance(CmdOffset, CmdLength); + if (SC_IDX_WITHIN_LIMIT(PendingOffset, 1 + SC_RTS_BUFF_SIZE32)) { /* ** Everything passed! @@ -282,7 +283,7 @@ void SC_GetNextRtsCommand(void) /* Stop the RTS from executing */ SC_KillRts(RtsIndex); - if ((SC_OperData.RtsCtrlBlckAddr->CurrRtsNum) <= SC_LAST_RTS_WITH_EVENTS) + if (SC_RtsNumHasEvent(SC_OperData.RtsCtrlBlckAddr->CurrRtsNum)) { CFE_EVS_SendEvent(SC_RTS_COMPL_INF_EID, CFE_EVS_EventType_INFORMATION, "RTS %03d Execution Completed", SC_OperData.RtsCtrlBlckAddr->CurrRtsNum); @@ -293,7 +294,7 @@ void SC_GetNextRtsCommand(void) { /* The end of the RTS buffer has been reached... */ /* Stop the RTS from executing */ SC_KillRts(RtsIndex); - if ((SC_OperData.RtsCtrlBlckAddr->CurrRtsNum) <= SC_LAST_RTS_WITH_EVENTS) + if (SC_RtsNumHasEvent(SC_OperData.RtsCtrlBlckAddr->CurrRtsNum)) { CFE_EVS_SendEvent(SC_RTS_COMPL_INF_EID, CFE_EVS_EventType_INFORMATION, "RTS %03d Execution Completed", SC_OperData.RtsCtrlBlckAddr->CurrRtsNum); @@ -327,13 +328,14 @@ void SC_GetNextAtsCommand(void) ** Get the information that is needed to find the next command */ AtsIndex = SC_AtsNumToIndex(SC_OperData.AtsCtrlBlckAddr->CurrAtsNum); - TimeIndex = SC_OperData.AtsCtrlBlckAddr->TimeIndexPtr + 1; + TimeIndex = SC_OperData.AtsCtrlBlckAddr->TimeIndexPtr; AtsInfoPtr = SC_GetAtsInfoObject(AtsIndex); + SC_IDX_INCREMENT(TimeIndex); /* ** Check to see if there are more ATS commands */ - if (TimeIndex < AtsInfoPtr->NumberOfCommands) + if (SC_IDX_WITHIN_LIMIT(TimeIndex, AtsInfoPtr->NumberOfCommands)) { /* get the information for the next command in the ATP control block */ AtsCmdNumRec = SC_GetAtsCommandNumAtSeq(AtsIndex, TimeIndex); diff --git a/unit-test/sc_atsrq_tests.c b/unit-test/sc_atsrq_tests.c index 31c35ad..0b60319 100644 --- a/unit-test/sc_atsrq_tests.c +++ b/unit-test/sc_atsrq_tests.c @@ -226,7 +226,7 @@ void SC_StartAtsCmd_Test_InUse(void) UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 1); } -void SC_StartAtsCmd_Test_InvalidAtsId(void) +void SC_StartAtsCmd_Test_InvalidAtsNum(void) { CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(SC_CMD_MID); CFE_MSG_FcnCode_t FcnCode = SC_START_ATS_CC; @@ -247,7 +247,7 @@ void SC_StartAtsCmd_Test_InvalidAtsId(void) UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 1); } -void SC_StartAtsCmd_Test_InvalidAtsIdZero(void) +void SC_StartAtsCmd_Test_InvalidAtsNumZero(void) { CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(SC_CMD_MID); CFE_MSG_FcnCode_t FcnCode = SC_START_ATS_CC; @@ -448,6 +448,20 @@ void SC_SwitchAtsCmd_Test_Nominal(void) UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 1); } +void SC_SwitchAtsCmd_Test_BadId(void) +{ + /* Execute the function being tested */ + UtAssert_VOIDCALL(SC_SwitchAtsCmd(&UT_CmdBuf.SwitchAtsCmd)); + + /* Verify results */ + UtAssert_True(SC_OperData.AtsCtrlBlckAddr->SwitchPendFlag == false, + "SC_OperData.AtsCtrlBlckAddr->SwitchPendFlag == false"); + UtAssert_True(SC_OperData.HkPacket.Payload.CmdErrCtr == 1, "SC_OperData.HkPacket.Payload.CmdErrCtr == 1"); + + UtAssert_INT32_EQ(context_CFE_EVS_SendEvent[0].EventID, SC_SWITCH_ATS_CMD_IDLE_ERR_EID); + UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 1); +} + void SC_SwitchAtsCmd_Test_DestinationAtsNotLoaded(void) { SC_AtsIndex_t AtsIndex = SC_ATS_IDX_C(1); @@ -1146,9 +1160,9 @@ void UtTest_Setup(void) UtTest_Add(SC_StartAtsCmd_Test_NoCommandsA, SC_Test_Setup, SC_Test_TearDown, "SC_StartAtsCmd_Test_NoCommandsA"); UtTest_Add(SC_StartAtsCmd_Test_NoCommandsB, SC_Test_Setup, SC_Test_TearDown, "SC_StartAtsCmd_Test_NoCommandsB"); UtTest_Add(SC_StartAtsCmd_Test_InUse, SC_Test_Setup, SC_Test_TearDown, "SC_StartAtsCmd_Test_InUse"); - UtTest_Add(SC_StartAtsCmd_Test_InvalidAtsId, SC_Test_Setup, SC_Test_TearDown, "SC_StartAtsCmd_Test_InvalidAtsId"); - UtTest_Add(SC_StartAtsCmd_Test_InvalidAtsIdZero, SC_Test_Setup, SC_Test_TearDown, - "SC_StartAtsCmd_Test_InvalidAtsIdZero"); + UtTest_Add(SC_StartAtsCmd_Test_InvalidAtsNum, SC_Test_Setup, SC_Test_TearDown, "SC_StartAtsCmd_Test_InvalidAtsNum"); + UtTest_Add(SC_StartAtsCmd_Test_InvalidAtsNumZero, SC_Test_Setup, SC_Test_TearDown, + "SC_StartAtsCmd_Test_InvalidAtsNumZero"); UtTest_Add(SC_StopAtsCmd_Test_NominalA, SC_Test_Setup, SC_Test_TearDown, "SC_StopAtsCmd_Test_NominalA"); UtTest_Add(SC_StopAtsCmd_Test_NominalB, SC_Test_Setup, SC_Test_TearDown, "SC_StopAtsCmd_Test_NominalB"); UtTest_Add(SC_StopAtsCmd_Test_NoRunningAts, SC_Test_Setup, SC_Test_TearDown, "SC_StopAtsCmd_Test_NoRunningAts"); @@ -1158,6 +1172,7 @@ void UtTest_Setup(void) UtTest_Add(SC_BeginAts_Test_InvalidAtsIndex, SC_Test_Setup, SC_Test_TearDown, "SC_BeginAts_Test_InvalidAtsIndex"); UtTest_Add(SC_KillAts_Test, SC_Test_Setup, SC_Test_TearDown, "SC_KillAts_Test"); UtTest_Add(SC_SwitchAtsCmd_Test_Nominal, SC_Test_Setup, SC_Test_TearDown, "SC_SwitchAtsCmd_Test_Nominal"); + UtTest_Add(SC_SwitchAtsCmd_Test_BadId, SC_Test_Setup, SC_Test_TearDown, "SC_SwitchAtsCmd_Test_BadId"); UtTest_Add(SC_SwitchAtsCmd_Test_DestinationAtsNotLoaded, SC_Test_Setup, SC_Test_TearDown, "SC_SwitchAtsCmd_Test_DestinationAtsNotLoaded"); UtTest_Add(SC_SwitchAtsCmd_Test_AtpIdle, SC_Test_Setup, SC_Test_TearDown, "SC_SwitchAtsCmd_Test_AtpIdle"); diff --git a/unit-test/sc_loads_tests.c b/unit-test/sc_loads_tests.c index 34a5b82..c6c75f6 100644 --- a/unit-test/sc_loads_tests.c +++ b/unit-test/sc_loads_tests.c @@ -1152,6 +1152,43 @@ void SC_ProcessAppend_Test_NotExecuting(void) UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 0); } +void SC_ProcessAppend_Test_IdMismatch(void) +{ + SC_AtsIndex_t AtsIndex = SC_ATS_IDX_C(0); + void * TailPtr; + SC_AtsInfoTable_t * AtsInfoPtr; + SC_AtsCmdStatusEntry_t * StatusEntryPtr; + SC_AtsCmdEntryOffsetRecord_t *CmdOffsetRec; + + CmdOffsetRec = SC_GetAtsEntryOffsetForCmd(AtsIndex, SC_COMMAND_IDX_C(0)); + StatusEntryPtr = SC_GetAtsStatusEntryForCommand(AtsIndex, SC_COMMAND_IDX_C(0)); + + AtsInfoPtr = SC_GetAtsInfoObject(AtsIndex); + + UT_SC_SetupSingleAtsEntry(AtsIndex, 1, UT_SC_NOMINAL_CMD_SIZE); + + TailPtr = UT_SC_GetAppendTable(); + UT_SC_AppendSingleAtsEntry(&TailPtr, 1, UT_SC_NOMINAL_CMD_SIZE); + + SC_AppData.AppendWordCount = 1; + SC_OperData.HkPacket.Payload.AppendEntryCount = 1; + + SC_OperData.AtsCtrlBlckAddr->AtpState = SC_Status_EXECUTING; + SC_OperData.AtsCtrlBlckAddr->CurrAtsNum = SC_AtsIndexToNum(SC_ATS_IDX_C(1)); + + /* Execute the function being tested */ + SC_ProcessAppend(AtsIndex); + + /* Verify results */ + UtAssert_UINT32_EQ(AtsInfoPtr->AtsSize, 1); + UtAssert_UINT32_EQ(AtsInfoPtr->NumberOfCommands, 1); + SC_Assert_IDX_EQ(CmdOffsetRec->Offset, SC_ENTRY_OFFSET_FIRST); + SC_Assert_CmdStatus(StatusEntryPtr->Status, SC_Status_LOADED); + SC_Assert_CmdStatus(SC_OperData.AtsCtrlBlckAddr->AtpState, SC_Status_EXECUTING); + + UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 0); +} + void SC_ProcessAppend_Test_AtsNum(void) { SC_AtsIndex_t AtsIndex = SC_ATS_IDX_C(0); @@ -1479,6 +1516,8 @@ void UtTest_Setup(void) "SC_ProcessAppend_Test_CmdLoaded"); UtTest_Add(SC_ProcessAppend_Test_NotExecuting, UT_SC_Loads_Test_Setup, SC_Test_TearDown, "SC_ProcessAppend_Test_NotExecuting"); + UtTest_Add(SC_ProcessAppend_Test_IdMismatch, UT_SC_Loads_Test_Setup, SC_Test_TearDown, + "SC_ProcessAppend_Test_IdMismatch"); UtTest_Add(SC_ProcessAppend_Test_AtsNum, UT_SC_Loads_Test_Setup, SC_Test_TearDown, "SC_ProcessAppend_Test_AtsNum"); UtTest_Add(SC_ProcessAppend_Test_InvalidIndex, UT_SC_Loads_Test_Setup, SC_Test_TearDown, "SC_ProcessAppend_Test_InvalidIndex"); diff --git a/unit-test/sc_rtsrq_tests.c b/unit-test/sc_rtsrq_tests.c index 5d30161..c4f4a0b 100644 --- a/unit-test/sc_rtsrq_tests.c +++ b/unit-test/sc_rtsrq_tests.c @@ -124,7 +124,7 @@ void SC_StartRtsCmd_Test_StartRtsNoEvents(void) UtAssert_True(SC_OperData.HkPacket.Payload.CmdCtr == 1, "SC_OperData.HkPacket.Payload.CmdCtr == 1"); /* Handle if SC_LAST_RTS_WITH_EVENTS is the same as SC_NUM_OF_RTS */ - if (UT_CmdBuf.DisableRtsCmd.Payload.RtsNum > SC_LAST_RTS_WITH_EVENTS) + if (!SC_RtsNumHasEvent(UT_CmdBuf.DisableRtsCmd.Payload.RtsNum)) { UtAssert_INT32_EQ(context_CFE_EVS_SendEvent[0].EventID, SC_STARTRTS_CMD_DBG_EID); } @@ -246,7 +246,7 @@ void SC_StartRtsCmd_Test_RtsDisabled(void) UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 1); } -void SC_StartRtsCmd_Test_InvalidRtsId(void) +void SC_StartRtsCmd_Test_InvalidRtsNum(void) { UT_CmdBuf.StartRtsCmd.Payload.RtsNum = SC_RTS_NUM_C(SC_NUMBER_OF_RTS * 2); @@ -258,7 +258,7 @@ void SC_StartRtsCmd_Test_InvalidRtsId(void) UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 1); } -void SC_StartRtsCmd_Test_InvalidRtsIdZero(void) +void SC_StartRtsCmd_Test_InvalidRtsNumZero(void) { UT_CmdBuf.StartRtsCmd.Payload.RtsNum = SC_RTS_NUM_C(0); @@ -1072,10 +1072,10 @@ void UtTest_Setup(void) UtTest_Add(SC_StartRtsCmd_Test_RtsNotLoadedOrInUse, SC_Test_Setup, SC_Test_TearDown, "SC_StartRtsCmd_Test_RtsNotLoadedOrInUse"); UtTest_Add(SC_StartRtsCmd_Test_RtsDisabled, SC_Test_Setup, SC_Test_TearDown, "SC_StartRtsCmd_Test_RtsDisabled"); - UtTest_Add(SC_StartRtsCmd_Test_InvalidRtsId, SC_Test_Setup, SC_Test_TearDown, "SC_StartRtsCmd_Test_InvalidRtsId"); + UtTest_Add(SC_StartRtsCmd_Test_InvalidRtsNum, SC_Test_Setup, SC_Test_TearDown, "SC_StartRtsCmd_Test_InvalidRtsNum"); - UtTest_Add(SC_StartRtsCmd_Test_InvalidRtsIdZero, SC_Test_Setup, SC_Test_TearDown, - "SC_StartRtsCmd_Test_InvalidRtsIdZero"); + UtTest_Add(SC_StartRtsCmd_Test_InvalidRtsNumZero, SC_Test_Setup, SC_Test_TearDown, + "SC_StartRtsCmd_Test_InvalidRtsNumZero"); UtTest_Add(SC_StartRtsGrpCmd_Test_Nominal, SC_Test_Setup, SC_Test_TearDown, "SC_StartRtsGrpCmd_Test_Nominal"); UtTest_Add(SC_StartRtsGrpCmd_Test_StartRtsGroupError, SC_Test_Setup, SC_Test_TearDown, "SC_StartRtsGrpCmd_Test_StartRtsGroupError");