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.

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

Fix #2262, separate dispatcher for messages #2263

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

jphickey
Copy link
Contributor

Checklist (Please check before submitting)

Describe the contribution
Isolate the message verification and dispatch from the general message processing. Functions in the "task" source file now strictly handle the command content, and do not get involved in general validation.

Also note: this is mostly a simple code move but does add a const qualifier to the task pipe function where it was missing before. All of the handlers were already const so this just makes it consistent throughout the handling.

Fixes #2262

Testing performed
Build and run CFE and sanity check. Run all tests

Expected behavior changes
None - this just moves code from one source file into another, does not make any change to functionality.

System(s) tested on
Debian

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey added the CCB:Ready Ready for discussion at the Configuration Control Board (CCB) label Mar 23, 2023
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL-coding-standard found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

Comment on lines 84 to 284
case CFE_ES_OVER_WRITE_SYSLOG_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_OverWriteSysLogCmd_t)))
{
CFE_ES_OverWriteSysLogCmd((CFE_ES_OverWriteSysLogCmd_t *)SBBufPtr);
}
break;

case CFE_ES_CLEAR_ER_LOG_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_ClearERLogCmd_t)))
{
CFE_ES_ClearERLogCmd((CFE_ES_ClearERLogCmd_t *)SBBufPtr);
}
break;

case CFE_ES_WRITE_ER_LOG_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_WriteERLogCmd_t)))
{
CFE_ES_WriteERLogCmd((CFE_ES_WriteERLogCmd_t *)SBBufPtr);
}
break;

case CFE_ES_START_PERF_DATA_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_StartPerfDataCmd_t)))
{
CFE_ES_StartPerfDataCmd((CFE_ES_StartPerfDataCmd_t *)SBBufPtr);
}
break;

case CFE_ES_STOP_PERF_DATA_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_StopPerfDataCmd_t)))
{
CFE_ES_StopPerfDataCmd((CFE_ES_StopPerfDataCmd_t *)SBBufPtr);
}
break;

case CFE_ES_SET_PERF_FILTER_MASK_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_SetPerfFilterMaskCmd_t)))
{
CFE_ES_SetPerfFilterMaskCmd((CFE_ES_SetPerfFilterMaskCmd_t *)SBBufPtr);
}
break;

case CFE_ES_SET_PERF_TRIGGER_MASK_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_SetPerfTriggerMaskCmd_t)))
{
CFE_ES_SetPerfTriggerMaskCmd((CFE_ES_SetPerfTriggerMaskCmd_t *)SBBufPtr);
}
break;

case CFE_ES_RESET_PR_COUNT_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_ResetPRCountCmd_t)))
{
CFE_ES_ResetPRCountCmd((CFE_ES_ResetPRCountCmd_t *)SBBufPtr);
}
break;

case CFE_ES_SET_MAX_PR_COUNT_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_SetMaxPRCountCmd_t)))
{
CFE_ES_SetMaxPRCountCmd((CFE_ES_SetMaxPRCountCmd_t *)SBBufPtr);
}
break;

case CFE_ES_DELETE_CDS_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_DeleteCDSCmd_t)))
{
CFE_ES_DeleteCDSCmd((CFE_ES_DeleteCDSCmd_t *)SBBufPtr);
}
break;

case CFE_ES_SEND_MEM_POOL_STATS_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_SendMemPoolStatsCmd_t)))
{
CFE_ES_SendMemPoolStatsCmd((CFE_ES_SendMemPoolStatsCmd_t *)SBBufPtr);
}
break;

case CFE_ES_DUMP_CDS_REGISTRY_CC:
if (CFE_ES_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_ES_DumpCDSRegistryCmd_t)))
{
CFE_ES_DumpCDSRegistryCmd((CFE_ES_DumpCDSRegistryCmd_t *)SBBufPtr);
}
break;

default:
CFE_EVS_SendEvent(CFE_ES_CC1_ERR_EID, CFE_EVS_EventType_ERROR,
"Invalid ground command code: ID = 0x%X, CC = %d",
(unsigned int)CFE_SB_MsgIdToValue(MessageID), (int)CommandCode);
CFE_ES_Global.TaskData.CommandErrorCounter++;
break;
}
break;

default:

CFE_EVS_SendEvent(CFE_ES_MID_ERR_EID, CFE_EVS_EventType_ERROR, "Invalid command pipe message ID: 0x%X",
(unsigned int)CFE_SB_MsgIdToValue(MessageID));
CFE_ES_Global.TaskData.CommandErrorCounter++;
break;
}

Check notice

Code scanning / CodeQL-security

Long switch case

Switch has at least one case that is too long: [... + ... (181 lines)](1).
Comment on lines +83 to +199
break;

case CFE_SB_ENABLE_SUB_REPORTING_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_EnableSubReportingCmd_t)))
{
CFE_SB_EnableSubReportingCmd((const CFE_SB_EnableSubReportingCmd_t *)SBBufPtr);
}
break;

case CFE_SB_DISABLE_SUB_REPORTING_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_DisableSubReportingCmd_t)))
{
CFE_SB_DisableSubReportingCmd((const CFE_SB_DisableSubReportingCmd_t *)SBBufPtr);
}
break;

default:
CFE_EVS_SendEvent(CFE_SB_BAD_CMD_CODE_EID, CFE_EVS_EventType_ERROR,
"Invalid Cmd, Unexpected Command Code %u", (unsigned int)FcnCode);
CFE_SB_Global.HKTlmMsg.Payload.CommandErrorCounter++;
break;
} /* end switch on cmd code */
break;

case CFE_SB_CMD_MID:
CFE_MSG_GetFcnCode(&SBBufPtr->Msg, &FcnCode);
switch (FcnCode)
{
case CFE_SB_NOOP_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_NoopCmd_t)))
{
CFE_SB_NoopCmd((const CFE_SB_NoopCmd_t *)SBBufPtr);
}
break;

case CFE_SB_RESET_COUNTERS_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_ResetCountersCmd_t)))
{
/* Note: Command counter not incremented for this command */
CFE_SB_ResetCountersCmd((const CFE_SB_ResetCountersCmd_t *)SBBufPtr);
}
break;

case CFE_SB_SEND_SB_STATS_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_SendSbStatsCmd_t)))
{
CFE_SB_SendStatsCmd((const CFE_SB_SendSbStatsCmd_t *)SBBufPtr);
}
break;

case CFE_SB_WRITE_ROUTING_INFO_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_WriteRoutingInfoCmd_t)))
{
CFE_SB_WriteRoutingInfoCmd((const CFE_SB_WriteRoutingInfoCmd_t *)SBBufPtr);
}
break;

case CFE_SB_ENABLE_ROUTE_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_EnableRouteCmd_t)))
{
CFE_SB_EnableRouteCmd((const CFE_SB_EnableRouteCmd_t *)SBBufPtr);
}
break;

case CFE_SB_DISABLE_ROUTE_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_DisableRouteCmd_t)))
{
CFE_SB_DisableRouteCmd((const CFE_SB_DisableRouteCmd_t *)SBBufPtr);
}
break;

case CFE_SB_WRITE_PIPE_INFO_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_WritePipeInfoCmd_t)))
{
CFE_SB_WritePipeInfoCmd((const CFE_SB_WritePipeInfoCmd_t *)SBBufPtr);
}
break;

case CFE_SB_WRITE_MAP_INFO_CC:
if (CFE_SB_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_SB_WriteMapInfoCmd_t)))
{
CFE_SB_WriteMapInfoCmd((const CFE_SB_WriteMapInfoCmd_t *)SBBufPtr);
}
break;

default:
CFE_EVS_SendEvent(CFE_SB_BAD_CMD_CODE_EID, CFE_EVS_EventType_ERROR,
"Invalid Cmd, Unexpected Command Code %u", FcnCode);
CFE_SB_Global.HKTlmMsg.Payload.CommandErrorCounter++;
break;
} /* end switch on cmd code */
break;

default:
CFE_EVS_SendEvent(CFE_SB_BAD_MSGID_EID, CFE_EVS_EventType_ERROR, "Invalid Cmd, Unexpected Msg Id: 0x%x",
(unsigned int)CFE_SB_MsgIdToValue(MessageID));
CFE_SB_Global.HKTlmMsg.Payload.CommandErrorCounter++;
break;

} /* end switch on MsgId */

Check notice

Code scanning / CodeQL-security

Long switch case

Switch has at least one case that is too long: [... + ... (33 lines)](1). Switch has at least one case that is too long: [... + ... (68 lines)](2).
Comment on lines +78 to +263
}
break;

/*
** Time Clients process "tone delay" commands...
*/
case CFE_TIME_ADD_DELAY_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_AddDelayCmd_t)))
{
CFE_TIME_AddDelayCmd((const CFE_TIME_AddDelayCmd_t *)SBBufPtr);
}
break;

case CFE_TIME_SUB_DELAY_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_SubDelayCmd_t)))
{
CFE_TIME_SubDelayCmd((const CFE_TIME_SubDelayCmd_t *)SBBufPtr);
}
break;

/*
** Time Servers process "set time" commands...
*/
case CFE_TIME_SET_TIME_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_SetTimeCmd_t)))
{
CFE_TIME_SetTimeCmd((const CFE_TIME_SetTimeCmd_t *)SBBufPtr);
}
break;

case CFE_TIME_SET_MET_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_SetMETCmd_t)))
{
CFE_TIME_SetMETCmd((const CFE_TIME_SetMETCmd_t *)SBBufPtr);
}
break;

case CFE_TIME_SET_STCF_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_SetSTCFCmd_t)))
{
CFE_TIME_SetSTCFCmd((const CFE_TIME_SetSTCFCmd_t *)SBBufPtr);
}
break;

case CFE_TIME_SET_LEAP_SECONDS_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_SetLeapSecondsCmd_t)))
{
CFE_TIME_SetLeapSecondsCmd((const CFE_TIME_SetLeapSecondsCmd_t *)SBBufPtr);
}
break;

case CFE_TIME_ADD_ADJUST_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_AddAdjustCmd_t)))
{
CFE_TIME_AddAdjustCmd((const CFE_TIME_AddAdjustCmd_t *)SBBufPtr);
}
break;

case CFE_TIME_SUB_ADJUST_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_SubAdjustCmd_t)))
{
CFE_TIME_SubAdjustCmd((const CFE_TIME_SubAdjustCmd_t *)SBBufPtr);
}
break;

case CFE_TIME_ADD_1HZ_ADJUSTMENT_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_Add1HZAdjustmentCmd_t)))
{
CFE_TIME_Add1HZAdjustmentCmd((const CFE_TIME_Add1HZAdjustmentCmd_t *)SBBufPtr);
}
break;

case CFE_TIME_SUB_1HZ_ADJUSTMENT_CC:
if (CFE_TIME_VerifyCmdLength(&SBBufPtr->Msg, sizeof(CFE_TIME_Sub1HZAdjustmentCmd_t)))
{
CFE_TIME_Sub1HZAdjustmentCmd((const CFE_TIME_Sub1HZAdjustmentCmd_t *)SBBufPtr);
}
break;

default:

CFE_TIME_Global.CommandErrorCounter++;
CFE_EVS_SendEvent(CFE_TIME_CC_ERR_EID, CFE_EVS_EventType_ERROR,
"Invalid command code -- ID = 0x%X, CC = %d",
(unsigned int)CFE_SB_MsgIdToValue(MessageID), (int)CommandCode);
break;
} /* switch (CFE_TIME_CMD_MID -- command code)*/
break;

default:

/*
** Note: we only increment the command error counter when
** processing CFE_TIME_CMD_MID commands...
*/
CFE_EVS_SendEvent(CFE_TIME_ID_ERR_EID, CFE_EVS_EventType_ERROR, "Invalid message ID -- ID = 0x%X",
(unsigned int)CFE_SB_MsgIdToValue(MessageID));
break;

} /* switch (message ID) */

Check notice

Code scanning / CodeQL-security

Long switch case

Switch has at least one case that is too long: [... + ... (132 lines)](1).
@jphickey
Copy link
Contributor Author

Fixed the clang-format issue, this should be good to go now.

@jphickey
Copy link
Contributor Author

This was reviewed approved during the 2023-03-23 CCB, pending the format fix (pushed 5 days ago, above)

Isolate the message verification and dispatch from the general message
processing.  Functions in the "task" source file now strictly handle the
command content, and do not get involved in general validation.

Also note: this is mostly a simple code move but does add a "const"
qualifier to the task pipe function where it was missing before.  All of
the handlers were already "const".
@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration coding-standard. As part of the setup process, we have scanned this repository and found 1905 existing alerts. Please check the repository Security tab to see all alerts.

Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

@github-advanced-security
Copy link

You have successfully added a new CodeQL configuration security. As part of the setup process, we have scanned this repository and found 29 existing alerts. Please check the repository Security tab to see all alerts.

@dzbaker dzbaker added CCB:Approved Indicates code review and approval by community CCB and removed CCB:Ready Ready for discussion at the Configuration Control Board (CCB) labels Mar 30, 2023
dzbaker added a commit to nasa/cFS that referenced this pull request Mar 30, 2023
*Combines:*

cFE v7.0.0-rc4+dev260

**Includes:**

*cFE*
- nasa/cFE#2259
- nasa/cFE#2263

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
@dzbaker dzbaker mentioned this pull request Mar 30, 2023
2 tasks
@dzbaker dzbaker merged commit 916d7d9 into nasa:main Mar 30, 2023
dzbaker added a commit to nasa/cFS that referenced this pull request Mar 30, 2023
*Combines:*

cFE v7.0.0-rc4+dev260

**Includes:**

*cFE*
- nasa/cFE#2259
- nasa/cFE#2263

Co-authored by: Joseph Hickey <jphickey@users.noreply.github.com>
@jphickey jphickey deleted the fix-2262-dispatch branch March 31, 2023 14:11
@dmknutsen dmknutsen added this to the Equuleus milestone May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CCB:Approved Indicates code review and approval by community CCB Equuleus-rc1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move cmd pipe processing into separate dispatch file
3 participants