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.
nasa · Mar 9, 2021 · aca9147 · aca9147
1 parent c7788a7
commit aca9147
Show file tree

Hide file tree

Showing 9 changed files with 23 additions and 16 deletions.
diff --git a/.github/workflows/run_fsw_cppcheck.sh 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
@@ -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
@@ -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
@@ -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/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
@@ -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_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_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
@@ -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);