From a53428c02b4e7b34abeed94e6dbf10ea3c129080 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 9 Mar 2021 10:02:57 -0500 Subject: [PATCH] Fix #972, Update FSW for cppcheck issues This updates the cppcheck static analysis rule to run via a script which takes into account the new directory structure. Oddly this reported new (valid) issues in FSW code which have also been corrected. Notably the wrapper script also contains a special provision to run static analysis on TIME only with SERVER mode enabled. The default logic of cppcheck would run all combos and it is not valid to have both of these defined. --- .github/workflows/run_fsw_cppcheck.sh | 13 +++++++++++++ .github/workflows/static-analysis.yml | 3 ++- modules/es/fsw/src/cfe_es_apps.c | 7 +------ modules/es/fsw/src/cfe_es_start.c | 1 - modules/es/fsw/src/cfe_es_task.c | 3 +-- modules/sb/fsw/src/cfe_sb_api.c | 1 - modules/tbl/fsw/src/cfe_tbl_api.c | 8 ++++---- modules/tbl/fsw/src/cfe_tbl_task.c | 2 +- modules/tbl/fsw/src/cfe_tbl_task_cmds.c | 2 +- modules/time/fsw/src/cfe_time_task.c | 2 +- modules/time/fsw/src/cfe_time_tone.c | 2 +- modules/time/fsw/src/cfe_time_utils.h | 2 +- 12 files changed, 26 insertions(+), 20 deletions(-) create mode 100755 .github/workflows/run_fsw_cppcheck.sh diff --git a/.github/workflows/run_fsw_cppcheck.sh b/.github/workflows/run_fsw_cppcheck.sh new file mode 100755 index 000000000..6f6af4b6c --- /dev/null +++ b/.github/workflows/run_fsw_cppcheck.sh @@ -0,0 +1,13 @@ +#!/bin/bash + +cppcheck_common_opts="--force --inline-suppr --std=c99 --language=c --enable=warning,performance,portability,style --suppress=variableScope --inconclusive" + +# When checking time, only the "server" config option is enabled for now. +# Otherwise cppcheck attempts to check with both branches enabled and generates false errors +cppcheck_time_opts="-UCFE_PLATFORM_TIME_CFG_CLIENT -DCFE_PLATFORM_TIME_CFG_SERVER" + +for mod in ${*} +do + cppcheck ${cppcheck_common_opts} $(eval echo \$cppcheck_${mod}_opts) ./modules/${mod}/fsw +done + diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 5e792f3aa..14b2c16e2 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -38,7 +38,8 @@ jobs: - name: cfe strict cppcheck if: ${{matrix.cppcheck =='cfe'}} run: | - cppcheck --force --inline-suppr --std=c99 --language=c --enable=warning,performance,portability,style --suppress=variableScope --inconclusive ./fsw/cfe-core/src ./modules 2> ./${{matrix.cppcheck}}_cppcheck_err.txt + all_fsw_modules="core_api core_private es evs fs msg resourceid sb sbr tbl time" + /bin/bash ./.github/workflows/run_fsw_cppcheck.sh ${all_fsw_modules} 2> ${{matrix.cppcheck}}_cppcheck_err.txt - name: Archive Static Analysis Artifacts uses: actions/upload-artifact@v2 diff --git a/modules/es/fsw/src/cfe_es_apps.c b/modules/es/fsw/src/cfe_es_apps.c index 75aca76c4..2bbc5e2b8 100644 --- a/modules/es/fsw/src/cfe_es_apps.c +++ b/modules/es/fsw/src/cfe_es_apps.c @@ -491,7 +491,6 @@ int32 CFE_ES_LoadModule(CFE_ResourceId_t ParentResourceId, const char *ModuleNam int32 CFE_ES_GetTaskFunction(CFE_ES_TaskEntryFuncPtr_t *FuncPtr) { CFE_ES_TaskRecord_t *TaskRecPtr; - CFE_ES_AppId_t AppId; CFE_ES_TaskEntryFuncPtr_t EntryFunc; int32 ReturnCode; int32 Timeout; @@ -501,7 +500,6 @@ int32 CFE_ES_GetTaskFunction(CFE_ES_TaskEntryFuncPtr_t *FuncPtr) */ ReturnCode = CFE_ES_ERR_APP_REGISTER; Timeout = CFE_PLATFORM_ES_STARTUP_SCRIPT_TIMEOUT_MSEC; - AppId = CFE_ES_APPID_UNDEFINED; EntryFunc = NULL; while(true) @@ -512,9 +510,8 @@ int32 CFE_ES_GetTaskFunction(CFE_ES_TaskEntryFuncPtr_t *FuncPtr) TaskRecPtr = CFE_ES_GetTaskRecordByContext(); if (TaskRecPtr != NULL) { - AppId = TaskRecPtr->AppId; EntryFunc = TaskRecPtr->EntryFunc; - if (CFE_RESOURCEID_TEST_DEFINED(AppId) && EntryFunc != 0) + if (CFE_RESOURCEID_TEST_DEFINED(TaskRecPtr->AppId) && EntryFunc != 0) { ReturnCode = CFE_SUCCESS; } @@ -1441,8 +1438,6 @@ int32 CFE_ES_CleanUpApp(CFE_ES_AppId_t AppId) * of resources are freed. */ CFE_ES_AppRecordSetUsed(AppRecPtr, CFE_RESOURCEID_RESERVED); - - ReturnCode = CFE_SUCCESS; } else { diff --git a/modules/es/fsw/src/cfe_es_start.c b/modules/es/fsw/src/cfe_es_start.c index a14d7ad47..9a1ae2831 100644 --- a/modules/es/fsw/src/cfe_es_start.c +++ b/modules/es/fsw/src/cfe_es_start.c @@ -379,7 +379,6 @@ void CFE_ES_SetupResetVariables(uint32 StartType, uint32 StartSubtype, uint32 Bo if ( CFE_ES_ResetDataPtr->ResetVars.ES_CausedReset != true ) { CFE_ES_ResetDataPtr->ResetVars.ResetType = CFE_PSP_RST_TYPE_PROCESSOR; - CFE_ES_ResetDataPtr->ResetVars.ResetSubtype = StartSubtype; CFE_ES_ResetDataPtr->ResetVars.ProcessorResetCount++; /* diff --git a/modules/es/fsw/src/cfe_es_task.c b/modules/es/fsw/src/cfe_es_task.c index 406ebb2ad..e29d9a796 100644 --- a/modules/es/fsw/src/cfe_es_task.c +++ b/modules/es/fsw/src/cfe_es_task.c @@ -1384,8 +1384,7 @@ int32 CFE_ES_QueryAllCmd(const CFE_ES_QueryAllCmd_t *data) FileSize += Result; EntryCount ++; } - - ++AppRecPtr; + } /* end for */ OS_close(FileDescriptor); diff --git a/modules/sb/fsw/src/cfe_sb_api.c b/modules/sb/fsw/src/cfe_sb_api.c index 0545ff1f2..ea824a4f7 100644 --- a/modules/sb/fsw/src/cfe_sb_api.c +++ b/modules/sb/fsw/src/cfe_sb_api.c @@ -755,7 +755,6 @@ int32 CFE_SB_GetPipeIdByName(CFE_SB_PipeId_t *PipeIdPtr, const char *PipeName) osal_id_t SysQueueId; PendingEventID = 0; - Status = CFE_SUCCESS; SysQueueId = OS_OBJECT_ID_UNDEFINED; if(PipeName == NULL || PipeIdPtr == NULL) diff --git a/modules/tbl/fsw/src/cfe_tbl_api.c b/modules/tbl/fsw/src/cfe_tbl_api.c index 639f306cc..5226bb35d 100644 --- a/modules/tbl/fsw/src/cfe_tbl_api.c +++ b/modules/tbl/fsw/src/cfe_tbl_api.c @@ -55,8 +55,8 @@ int32 CFE_TBL_Register( CFE_TBL_Handle_t *TblHandlePtr, CFE_TBL_LoadBuff_t *WorkingBufferPtr; CFE_TBL_CritRegRec_t *CritRegRecPtr = NULL; int32 Status; - size_t NameLen = 0; - int16 RegIndx = -1; + size_t NameLen; + int16 RegIndx; CFE_ES_AppId_t ThisAppId; char AppName[OS_MAX_API_NAME] = {"UNKNOWN"}; char TblName[CFE_TBL_MAX_FULL_NAME_LEN] = {""}; @@ -518,7 +518,7 @@ int32 CFE_TBL_Share( CFE_TBL_Handle_t *TblHandlePtr, { int32 Status; CFE_ES_AppId_t ThisAppId; - int16 RegIndx = CFE_TBL_NOT_FOUND; + int16 RegIndx; CFE_TBL_AccessDescriptor_t *AccessDescPtr = NULL; CFE_TBL_RegistryRec_t *RegRecPtr = NULL; char AppName[OS_MAX_API_NAME] = {"UNKNOWN"}; @@ -1507,7 +1507,7 @@ int32 CFE_TBL_Modified( CFE_TBL_Handle_t TblHandle ) CFE_TBL_RegistryRec_t *RegRecPtr = NULL; CFE_TBL_Handle_t AccessIterator; CFE_ES_AppId_t ThisAppId; - size_t FilenameLen = 0; + size_t FilenameLen; /* Verify that this application has the right to perform operation */ Status = CFE_TBL_ValidateAccess(TblHandle, &ThisAppId); diff --git a/modules/tbl/fsw/src/cfe_tbl_task.c b/modules/tbl/fsw/src/cfe_tbl_task.c index 22e8bba2b..3e7bed088 100644 --- a/modules/tbl/fsw/src/cfe_tbl_task.c +++ b/modules/tbl/fsw/src/cfe_tbl_task.c @@ -144,7 +144,7 @@ void CFE_TBL_TaskMain(void) int32 CFE_TBL_TaskInit(void) { - int32 Status = CFE_SUCCESS; + int32 Status; /* ** Register Table Services with ES diff --git a/modules/tbl/fsw/src/cfe_tbl_task_cmds.c b/modules/tbl/fsw/src/cfe_tbl_task_cmds.c index 9eef0da01..94fb817d1 100644 --- a/modules/tbl/fsw/src/cfe_tbl_task_cmds.c +++ b/modules/tbl/fsw/src/cfe_tbl_task_cmds.c @@ -1027,7 +1027,7 @@ int32 CFE_TBL_ActivateCmd(const CFE_TBL_ActivateCmd_t *data) const CFE_TBL_ActivateCmd_Payload_t *CmdPtr = &data->Payload; char TableName[CFE_TBL_MAX_FULL_NAME_LEN]; CFE_TBL_RegistryRec_t *RegRecPtr; - bool ValidationStatus = false; + bool ValidationStatus; /* Make sure all strings are null terminated before attempting to process them */ CFE_SB_MessageStringGet(TableName, (char *)CmdPtr->TableName, NULL, diff --git a/modules/time/fsw/src/cfe_time_task.c b/modules/time/fsw/src/cfe_time_task.c index 3770fc0ea..99f1b371c 100644 --- a/modules/time/fsw/src/cfe_time_task.c +++ b/modules/time/fsw/src/cfe_time_task.c @@ -197,7 +197,7 @@ void CFE_TIME_TaskMain(void) int32 CFE_TIME_TaskInit(void) { - int32 Status = CFE_SUCCESS; + int32 Status; osal_id_t TimeBaseId; osal_id_t TimerId; diff --git a/modules/time/fsw/src/cfe_time_tone.c b/modules/time/fsw/src/cfe_time_tone.c index 78c408f75..48b765a94 100644 --- a/modules/time/fsw/src/cfe_time_tone.c +++ b/modules/time/fsw/src/cfe_time_tone.c @@ -1461,7 +1461,7 @@ void CFE_TIME_Local1HzTask(void) void CFE_TIME_NotifyTimeSynchApps(void) { - uint32 i = 0; + uint32 i; CFE_TIME_SynchCallbackPtr_t Func; /* diff --git a/modules/time/fsw/src/cfe_time_utils.h b/modules/time/fsw/src/cfe_time_utils.h index 0c5cb4d3c..8a605ae2a 100644 --- a/modules/time/fsw/src/cfe_time_utils.h +++ b/modules/time/fsw/src/cfe_time_utils.h @@ -437,7 +437,7 @@ static inline volatile CFE_TIME_ReferenceState_t *CFE_TIME_GetReferenceState(voi ** Function prototypes (process time at the tone signal and data packet)... */ void CFE_TIME_ToneSignal(void); -void CFE_TIME_ToneData(const CFE_TIME_ToneDataCmd_Payload_t *Packet); +void CFE_TIME_ToneData(const CFE_TIME_ToneDataCmd_Payload_t *ToneDataCmd); void CFE_TIME_ToneVerify(CFE_TIME_SysTime_t Time1, CFE_TIME_SysTime_t Time2); void CFE_TIME_ToneUpdate(void);