Fix #118, update pointer arg types #120
Conversation
Pointers are punned to different (incorrect) types throughout CS. This addresses only the most egregious violations, in particular the case where an argument is defined as a pointer to a table and in reality it is a pointer-to-pointer (even qualified with const when it was updating it). The pointer mismatches are the reason for the strange "memcpy" in these spots. This does some other code cleanup but only scratches the surface of what needs to be done. There should be a follow-on task to better structure the data within the global, this would clean up a lot of APIs. In particular the CS_TableInit is a particularly ugly call with many arguments, this can be greatly simplified with a better data structure.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| /* Call respective table update handler function (helper) */ | ||
| /* */ | ||
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void CS_CallTableUpdateHandler(uint32 Table, const void *DefinitionPtr, void *ResultsPtr, size_t NumEntries) |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void CS_ProcessNewEepromMemoryDefinitionTable(const CS_Def_EepromMemory_Table_Entry_t *DefinitionTblPtr, | ||
| const CS_Res_EepromMemory_Table_Entry_t *ResultsTblPtr, uint16 NumEntries, | ||
| void CS_ProcessNewEepromMemoryDefinitionTable(const CS_Def_EepromMemory_Table_Entry_t *StartOfDefTable, |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void CS_ProcessNewEepromMemoryDefinitionTable(const CS_Def_EepromMemory_Table_Entry_t *DefinitionTblPtr, | ||
| const CS_Res_EepromMemory_Table_Entry_t *ResultsTblPtr, uint16 NumEntries, | ||
| void CS_ProcessNewEepromMemoryDefinitionTable(const CS_Def_EepromMemory_Table_Entry_t *StartOfDefTable, |
Check notice
Code scanning / CodeQL
Function too long Note
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void CS_ProcessNewTablesDefinitionTable(const CS_Def_Tables_Table_Entry_t *DefinitionTblPtr, | ||
| const CS_Res_Tables_Table_Entry_t *ResultsTblPtr) | ||
| void CS_ProcessNewTablesDefinitionTable(const CS_Def_Tables_Table_Entry_t *StartOfDefTable, |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void CS_ProcessNewTablesDefinitionTable(const CS_Def_Tables_Table_Entry_t *DefinitionTblPtr, | ||
| const CS_Res_Tables_Table_Entry_t *ResultsTblPtr) | ||
| void CS_ProcessNewTablesDefinitionTable(const CS_Def_Tables_Table_Entry_t *StartOfDefTable, |
Check notice
Code scanning / CodeQL
Function too long Note
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void CS_ProcessNewAppDefinitionTable(const CS_Def_App_Table_Entry_t *DefinitionTblPtr, | ||
| const CS_Res_App_Table_Entry_t *ResultsTblPtr) | ||
| void CS_ProcessNewAppDefinitionTable(const CS_Def_App_Table_Entry_t *StartOfDefTable, |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| void CS_ProcessNewAppDefinitionTable(const CS_Def_App_Table_Entry_t *DefinitionTblPtr, | ||
| const CS_Res_App_Table_Entry_t *ResultsTblPtr) | ||
| void CS_ProcessNewAppDefinitionTable(const CS_Def_App_Table_Entry_t *StartOfDefTable, |
Check notice
Code scanning / CodeQL
Function too long Note
| /* */ | ||
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| CFE_Status_t CS_HandleTableUpdate(void *DefinitionTblPtr, void *ResultsTblPtr, CFE_TBL_Handle_t DefinitionTableHandle, | ||
| CFE_Status_t CS_HandleTableUpdate(void **DefinitionTblPtr, void **ResultsTblPtr, CFE_TBL_Handle_t DefinitionTableHandle, |
Check notice
Code scanning / CodeQL
Long function without assertion Note
| /* */ | ||
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| CFE_Status_t CS_HandleTableUpdate(void *DefinitionTblPtr, void *ResultsTblPtr, CFE_TBL_Handle_t DefinitionTableHandle, | ||
| CFE_Status_t CS_HandleTableUpdate(void **DefinitionTblPtr, void **ResultsTblPtr, CFE_TBL_Handle_t DefinitionTableHandle, |
Check notice
Code scanning / CodeQL
Function too long Note
| /* Gets a table type ID value as a printable string */ | ||
| /* */ | ||
| /* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */ | ||
| const char *CS_GetTableTypeAsString(uint32 TableId) |
Check notice
Code scanning / CodeQL
Long function without assertion Note
3 participants
Checklist (Please check before submitting)
Describe the contribution
Pointers are punned to different (incorrect) types throughout CS. This addresses only the most egregious violations, in particular the case where an argument is defined as a pointer to a table and in reality it is a pointer-to-pointer (even qualified with const when it was updating it). The pointer mismatches are the reason for the strange "memcpy" in these spots.
This does some other code cleanup but only scratches the surface of what needs to be done.
There should be a follow-on task to better structure the data within the global, this would clean up a lot of APIs. In particular the CS_TableInit is a particularly ugly call with many arguments, this can be greatly simplified with a better data structure.
Fixes #118
Testing performed
Build and run CS and check operation, run all tests
Expected behavior changes
N/A
System(s) tested on
Linux
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.