Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Result variable in CS_ComputeApp() could be removed #103

Open
2 tasks done
thnkslprpt opened this issue Nov 30, 2024 · 0 comments · May be fixed by #104
Open
2 tasks done

Result variable in CS_ComputeApp() could be removed #103

thnkslprpt opened this issue Nov 30, 2024 · 0 comments · May be fixed by #104

Comments

@thnkslprpt
Copy link
Contributor

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
The Result variable in CS_ComputeApp() duplicates statuses that are already stored in ResultGetResourceID and ResultGetResourceInfo.

Code snips

CS/fsw/src/cs_compute.c

Lines 307 to 432 in f958cc0

CFE_Status_t CS_ComputeApp(CS_Res_App_Table_Entry_t *ResultsEntry, uint32 *ComputedCSValue, bool *DoneWithEntry)
{
uint32 OffsetIntoCurrEntry = 0;
cpuaddr FirstAddrThisCycle = 0;
uint32 NumBytesThisCycle = 0;
int32 NumBytesRemainingCycles = 0;
uint32 NewChecksumValue = 0;
CFE_Status_t Status = CFE_SUCCESS;
CFE_Status_t Result;
CFE_Status_t ResultGetResourceID = CS_ERROR;
CFE_Status_t ResultGetResourceInfo = CS_ERROR;
bool ResultAddressValid = false;
/* variables to get applications address */
CFE_ResourceId_t ResourceID = CFE_RESOURCEID_UNDEFINED;
CFE_ES_AppInfo_t AppInfo;
/* By the time we get here, we know we have an enabled entry */
/* set the done flag to false originally */
*DoneWithEntry = false;
ResultGetResourceID = CFE_ES_GetAppIDByName((CFE_ES_AppId_t *)&ResourceID, ResultsEntry->Name);
if (ResultGetResourceID == CFE_ES_ERR_NAME_NOT_FOUND)
{
/* Also check for a matching library name */
ResultGetResourceID = CFE_ES_GetLibIDByName((CFE_ES_LibId_t *)&ResourceID, ResultsEntry->Name);
}
Result = ResultGetResourceID;
if (Result == CFE_SUCCESS)
{
/* We got a valid ResourceID, so get the Resource info */
ResultGetResourceInfo = CFE_ES_GetModuleInfo(&AppInfo, ResourceID);
Result = ResultGetResourceInfo;
}
if (Result == CFE_SUCCESS)
{
/* We got a valid ResourceID and good App info, so check the for valid addresses */
if (AppInfo.AddressesAreValid == false)
{
CFE_EVS_SendEvent(CS_COMPUTE_APP_PLATFORM_DBG_EID, CFE_EVS_EventType_DEBUG,
"CS cannot get a valid address for %s, due to the platform", ResultsEntry->Name);
Result = CS_ERROR;
}
else
{
/* Push in the data from the module info */
ResultsEntry->NumBytesToChecksum = AppInfo.CodeSize;
ResultsEntry->StartAddress = AppInfo.CodeAddress;
Result = CFE_SUCCESS;
ResultAddressValid = true;
}
}
if (Result == CFE_SUCCESS)
{
/* We got valid ResourceID, good info, and valid addresses, so run the checksum */
OffsetIntoCurrEntry = ResultsEntry->ByteOffset;
FirstAddrThisCycle = ResultsEntry->StartAddress + OffsetIntoCurrEntry;
NumBytesRemainingCycles = ResultsEntry->NumBytesToChecksum - OffsetIntoCurrEntry;
NumBytesThisCycle = ((CS_AppData.MaxBytesPerCycle < NumBytesRemainingCycles) ? CS_AppData.MaxBytesPerCycle
: NumBytesRemainingCycles);
NewChecksumValue = CFE_ES_CalculateCRC((void *)(FirstAddrThisCycle), NumBytesThisCycle,
ResultsEntry->TempChecksumValue, CS_DEFAULT_ALGORITHM);
NumBytesRemainingCycles -= NumBytesThisCycle;
if (NumBytesRemainingCycles <= 0)
{
/* We are finished CS'ing all of the parts for this Entry */
*DoneWithEntry = true;
if (ResultsEntry->ComputedYet == true)
{
/* This is NOT the first time through this Entry.
We have already computed a CS value for this Entry */
if (NewChecksumValue != ResultsEntry->ComparisonValue)
{
/* If the just-computed value differ from the saved value */
Status = CS_ERROR;
}
else
{
/* The checksum passes the test. */
}
}
else
{
/* This is the first time through this Entry */
ResultsEntry->ComputedYet = true;
ResultsEntry->ComparisonValue = NewChecksumValue;
}
*ComputedCSValue = NewChecksumValue;
ResultsEntry->ByteOffset = 0;
ResultsEntry->TempChecksumValue = 0;
}
else
{
/* We have not finished this Entry. Will try to finish during next wakeup */
ResultsEntry->ByteOffset += NumBytesThisCycle;
ResultsEntry->TempChecksumValue = NewChecksumValue;
*ComputedCSValue = NewChecksumValue;
}
} /* end if got module id ok */
else
{
/* Something failed -- either invalid ResourceID, bad App info, or invalid addresses, so notify ground */
CFE_EVS_SendEvent(
CS_COMPUTE_APP_ERR_EID, CFE_EVS_EventType_ERROR,
"CS Apps: Problems getting module %s info, GetResourceID: 0x%08X, GetModuleInfo: 0x%08X, AddressValid: %d",
ResultsEntry->Name, (unsigned int)ResultGetResourceID, (unsigned int)ResultGetResourceInfo,
(unsigned int)ResultAddressValid);
Status = CS_ERR_NOT_FOUND;
}
return Status;
}

Expected behavior
Function could be simplified by removing the Result variable and using ResultGetResourceID and ResultGetResourceInfo directly.

Reporter Info
Avi Weiss   @thnkslprpt
thnkslprpt added a commit to thnkslprpt/CS that referenced this issue Nov 30, 2024
@thnkslprpt
Fix nasa#103, Simplify CS_ComputeApp by removing redundant Result
cef2995 
variable
@thnkslprpt thnkslprpt linked a pull request Nov 30, 2024 that will close this issue
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #103, Simplify CS_ComputeApp by removing redundant Result variable thnkslprpt/CS
1 participant
@thnkslprpt