Fix #972, Update FSW for cppcheck issues

jphickey · jphickey · commit a53428c02b4e · 2021-03-09T12:01:08.000-05:00
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.
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/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
@@ -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.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
@@ -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
@@ -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
@@ -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);
 

Original file line number	Diff line number	Diff line change
`@@ -491,7 +491,6 @@ int32 CFE_ES_LoadModule(CFE_ResourceId_t ParentResourceId, const char *ModuleNam`
`491`	`491`	`int32 CFE_ES_GetTaskFunction(CFE_ES_TaskEntryFuncPtr_t *FuncPtr)`
`492`	`492`	`{`
`493`	`493`	`CFE_ES_TaskRecord_t *TaskRecPtr;`
`494`		`- CFE_ES_AppId_t AppId;`
`495`	`494`	`CFE_ES_TaskEntryFuncPtr_t EntryFunc;`
`496`	`495`	`int32 ReturnCode;`
`497`	`496`	`int32 Timeout;`
`@@ -501,7 +500,6 @@ int32 CFE_ES_GetTaskFunction(CFE_ES_TaskEntryFuncPtr_t *FuncPtr)`
`501`	`500`	`*/`
`502`	`501`	`ReturnCode = CFE_ES_ERR_APP_REGISTER;`
`503`	`502`	`Timeout = CFE_PLATFORM_ES_STARTUP_SCRIPT_TIMEOUT_MSEC;`
`504`		`- AppId = CFE_ES_APPID_UNDEFINED;`
`505`	`503`	`EntryFunc = NULL;`
`506`	`504`
`507`	`505`	`while(true)`
`@@ -512,9 +510,8 @@ int32 CFE_ES_GetTaskFunction(CFE_ES_TaskEntryFuncPtr_t *FuncPtr)`
`512`	`510`	`TaskRecPtr = CFE_ES_GetTaskRecordByContext();`
`513`	`511`	`if (TaskRecPtr != NULL)`
`514`	`512`	`{`
`515`		`- AppId = TaskRecPtr->AppId;`
`516`	`513`	`EntryFunc = TaskRecPtr->EntryFunc;`
`517`		`- if (CFE_RESOURCEID_TEST_DEFINED(AppId) && EntryFunc != 0)`
	`514`	`+ if (CFE_RESOURCEID_TEST_DEFINED(TaskRecPtr->AppId) && EntryFunc != 0)`
`518`	`515`	`{`
`519`	`516`	`ReturnCode = CFE_SUCCESS;`
`520`	`517`	`}`
`@@ -1441,8 +1438,6 @@ int32 CFE_ES_CleanUpApp(CFE_ES_AppId_t AppId)`
`1441`	`1438`	`* of resources are freed.`
`1442`	`1439`	`*/`
`1443`	`1440`	`CFE_ES_AppRecordSetUsed(AppRecPtr, CFE_RESOURCEID_RESERVED);`
`1444`		`-`
`1445`		`- ReturnCode = CFE_SUCCESS;`
`1446`	`1441`	`}`
`1447`	`1442`	`else`
`1448`	`1443`	`{`
Original file line number	Diff line number	Diff line change
`@@ -379,7 +379,6 @@ void CFE_ES_SetupResetVariables(uint32 StartType, uint32 StartSubtype, uint32 Bo`
`379`	`379`	`if ( CFE_ES_ResetDataPtr->ResetVars.ES_CausedReset != true )`
`380`	`380`	`{`
`381`	`381`	`CFE_ES_ResetDataPtr->ResetVars.ResetType = CFE_PSP_RST_TYPE_PROCESSOR;`
`382`		`- CFE_ES_ResetDataPtr->ResetVars.ResetSubtype = StartSubtype;`
`383`	`382`	`CFE_ES_ResetDataPtr->ResetVars.ProcessorResetCount++;`
`384`	`383`
`385`	`384`	`/*`
Original file line number	Diff line number	Diff line change
`@@ -1384,8 +1384,7 @@ int32 CFE_ES_QueryAllCmd(const CFE_ES_QueryAllCmd_t *data)`
`1384`	`1384`	`FileSize += Result;`
`1385`	`1385`	`EntryCount ++;`
`1386`	`1386`	`}`
`1387`		`-`
`1388`		`- ++AppRecPtr;`
	`1387`	`+`
`1389`	`1388`	`} /* end for */`
`1390`	`1389`
`1391`	`1390`	`OS_close(FileDescriptor);`
Original file line number	Diff line number	Diff line change
`@@ -144,7 +144,7 @@ void CFE_TBL_TaskMain(void)`
`144`	`144`
`145`	`145`	`int32 CFE_TBL_TaskInit(void)`
`146`	`146`	`{`
`147`		`- int32 Status = CFE_SUCCESS;`
	`147`	`+ int32 Status;`
`148`	`148`
`149`	`149`	`/*`
`150`	`150`	`** Register Table Services with ES`
Original file line number	Diff line number	Diff line change
`@@ -197,7 +197,7 @@ void CFE_TIME_TaskMain(void)`
`197`	`197`
`198`	`198`	`int32 CFE_TIME_TaskInit(void)`
`199`	`199`	`{`
`200`		`- int32 Status = CFE_SUCCESS;`
	`200`	`+ int32 Status;`
`201`	`201`	`osal_id_t TimeBaseId;`
`202`	`202`	`osal_id_t TimerId;`
`203`	`203`