-
Notifications
You must be signed in to change notification settings - Fork 203
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
Conversation
There was a problem hiding this 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.
modules/es/fsw/src/cfe_es_dispatch.c
Outdated
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
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
} | ||
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
0930f46
to
918d433
Compare
Fixed the clang-format issue, this should be good to go now. |
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".
918d433
to
999eb5f
Compare
You have successfully added a new CodeQL configuration |
There was a problem hiding this 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.
You have successfully added a new CodeQL configuration |
*Combines:* cFE v7.0.0-rc4+dev260 **Includes:** *cFE* - nasa/cFE#2259 - nasa/cFE#2263 Co-authored by: Joseph Hickey <[email protected]>
*Combines:* cFE v7.0.0-rc4+dev260 **Includes:** *cFE* - nasa/cFE#2259 - nasa/cFE#2263 Co-authored by: Joseph Hickey <[email protected]>
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 alreadyconst
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.