Skip to content

Commit

Permalink
Fix #118, implement range checking functions
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jphickey
jphickey committed Nov 9, 2023
1 parent 99467c4 commit 089a61b
Show file tree
Hide file tree
Showing 8 changed files with 103 additions and 83 deletions.
30 changes: 17 additions & 13 deletions fsw/src/sc_atsrq.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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
{
Expand All @@ -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");
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -559,7 +563,7 @@ void SC_JumpAtsCmd(const SC_JumpAtsCmd_t *Cmd)
NumSkipped++;
}

TimeIndex++;
SC_IDX_INCREMENT(TimeIndex);
}
else
{
Expand All @@ -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");
Expand Down Expand Up @@ -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++;
Expand Down
9 changes: 6 additions & 3 deletions fsw/src/sc_cmds.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));

Expand Down
16 changes: 16 additions & 0 deletions fsw/src/sc_index_types.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
49 changes: 24 additions & 25 deletions fsw/src/sc_loads.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@
/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
void SC_LoadAts(SC_AtsIndex_t AtsIndex)

Check notice

Code scanning / CodeQL-coding-standard

Function too long Note

SC_LoadAts has too many lines (150, while 60 are allowed).

Check notice

Code scanning / CodeQL-coding-standard

Long function without assertion Note

All functions of more than 10 lines should have at least one assertion.
{
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;
Expand All @@ -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);
Expand All @@ -85,15 +85,15 @@ 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);

/* 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;
Expand All @@ -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;
Expand All @@ -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.... */
Expand All @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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 */
Expand All @@ -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)]);
}

/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 089a61b

Please sign in to comment.