From cff18abec37ba52be6592d2453772287d4393648 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 27 Apr 2020 08:48:51 -0400 Subject: [PATCH] Fix #641, string operations on GCC9 Ensure clean build, no warnings on string operations using GCC 9.3.0. Some string ops were genuinely incorrect (particularly in UT) but some were perfectly OK and handled correctly per the C spec. In particular the new "rules" that GCC9 warns about make the strncat library function (and some others) somewhat off-limits even if used correctly. --- fsw/cfe-core/src/es/cfe_es_api.c | 2 +- fsw/cfe-core/src/es/cfe_es_cds.c | 3 ++- fsw/cfe-core/src/es/cfe_es_task.c | 16 +++++++++++----- fsw/cfe-core/src/tbl/cfe_tbl_internal.c | 6 ++++-- fsw/cfe-core/src/tbl/cfe_tbl_task_cmds.c | 14 ++++++++------ fsw/cfe-core/unit-test/es_UT.c | 2 +- fsw/cfe-core/unit-test/tbl_UT.c | 11 +++++++---- fsw/cfe-core/unit-test/time_UT.c | 16 +++++++--------- fsw/cfe-core/unit-test/ut_support.c | 3 ++- 9 files changed, 43 insertions(+), 30 deletions(-) diff --git a/fsw/cfe-core/src/es/cfe_es_api.c b/fsw/cfe-core/src/es/cfe_es_api.c index 44e528037..985ad9b9d 100644 --- a/fsw/cfe-core/src/es/cfe_es_api.c +++ b/fsw/cfe-core/src/es/cfe_es_api.c @@ -775,7 +775,7 @@ int32 CFE_ES_GetAppName(char *AppName, uint32 AppId, uint32 BufferLength) { if ( CFE_ES_Global.AppTable[AppId].AppState != CFE_ES_AppState_UNDEFINED ) { - strncpy(AppName, (char *)CFE_ES_Global.AppTable[AppId].StartParams.Name, BufferLength); + strncpy(AppName, (char *)CFE_ES_Global.AppTable[AppId].StartParams.Name, BufferLength - 1); AppName[BufferLength - 1] = '\0'; Result = CFE_SUCCESS; } diff --git a/fsw/cfe-core/src/es/cfe_es_cds.c b/fsw/cfe-core/src/es/cfe_es_cds.c index ff8cf85ff..4cd699447 100644 --- a/fsw/cfe-core/src/es/cfe_es_cds.c +++ b/fsw/cfe-core/src/es/cfe_es_cds.c @@ -296,7 +296,8 @@ int32 CFE_ES_RegisterCDSEx(CFE_ES_CDSHandle_t *HandlePtr, int32 BlockSize, const RegRecPtr->Table = CriticalTbl; /* Save CDS Name in Registry */ - strncpy(RegRecPtr->Name, Name, CFE_ES_CDS_MAX_FULL_NAME_LEN); + strncpy(RegRecPtr->Name, Name, sizeof(RegRecPtr->Name)-1); + RegRecPtr->Name[sizeof(RegRecPtr->Name)-1] = 0; /* Return the index into the registry as the handle to the CDS */ *HandlePtr = RegIndx; diff --git a/fsw/cfe-core/src/es/cfe_es_task.c b/fsw/cfe-core/src/es/cfe_es_task.c index bb5dc5fdf..115dbcfbb 100644 --- a/fsw/cfe-core/src/es/cfe_es_task.c +++ b/fsw/cfe-core/src/es/cfe_es_task.c @@ -207,6 +207,7 @@ int32 CFE_ES_TaskInit(void) cpuaddr CfeSegmentAddr; char EventBuffer[CFE_MISSION_EVS_MAX_MESSAGE_LENGTH]; char VersionBuffer[CFE_MISSION_EVS_MAX_MESSAGE_LENGTH]; + uint32 Remaining; /* ** Register the Application @@ -367,17 +368,21 @@ int32 CFE_ES_TaskInit(void) snprintf(EventBuffer, sizeof(EventBuffer), "Mission %s.%s", GLOBAL_CONFIGDATA.MissionVersion, GLOBAL_CONFIGDATA.Config); } - if(strcmp(GLOBAL_CONFIGDATA.MissionVersion, GLOBAL_CONFIGDATA.CfeVersion)) + Remaining = sizeof(EventBuffer)-strlen(EventBuffer)-1; + if(Remaining > 0 && strcmp(GLOBAL_CONFIGDATA.MissionVersion, GLOBAL_CONFIGDATA.CfeVersion)) { snprintf(VersionBuffer, sizeof(VersionBuffer), ", CFE: %s", GLOBAL_CONFIGDATA.CfeVersion); - strncat(EventBuffer, VersionBuffer, sizeof(EventBuffer)-strlen(EventBuffer)-1); + VersionBuffer[Remaining] = 0; + strcat(EventBuffer, VersionBuffer); + Remaining = sizeof(EventBuffer)-strlen(EventBuffer)-1; } - if(strcmp(GLOBAL_CONFIGDATA.MissionVersion, GLOBAL_CONFIGDATA.OsalVersion)) + if(Remaining > 0 && strcmp(GLOBAL_CONFIGDATA.MissionVersion, GLOBAL_CONFIGDATA.OsalVersion)) { snprintf(VersionBuffer, sizeof(VersionBuffer), ", OSAL: %s", GLOBAL_CONFIGDATA.OsalVersion); - strncat(EventBuffer, VersionBuffer, sizeof(EventBuffer)-strlen(EventBuffer)-1); + VersionBuffer[Remaining] = 0; + strcat(EventBuffer, VersionBuffer); } Status = CFE_EVS_SendEvent(CFE_ES_VERSION_INF_EID, @@ -1916,7 +1921,8 @@ int32 CFE_ES_DumpCDSRegistryCmd(const CFE_ES_DumpCDSRegistry_t *data) DumpRecord.ByteAlignSpare1 = 0; /* strncpy will zero out any unused buffer - memset not necessary */ - strncpy(DumpRecord.Name, RegRecPtr->Name, sizeof(DumpRecord.Name)); + strncpy(DumpRecord.Name, RegRecPtr->Name, sizeof(DumpRecord.Name)-1); + DumpRecord.Name[sizeof(DumpRecord.Name)-1] = 0; /* Output Registry Dump Record to Registry Dump File */ Status = OS_write(FileDescriptor, diff --git a/fsw/cfe-core/src/tbl/cfe_tbl_internal.c b/fsw/cfe-core/src/tbl/cfe_tbl_internal.c index 68e563c4d..109b2b612 100644 --- a/fsw/cfe-core/src/tbl/cfe_tbl_internal.c +++ b/fsw/cfe-core/src/tbl/cfe_tbl_internal.c @@ -1110,10 +1110,12 @@ int32 CFE_TBL_UpdateInternal( CFE_TBL_Handle_t TblHandle, /* Save source description with active buffer */ strncpy(RegRecPtr->Buffers[0].DataSource, CFE_TBL_TaskData.LoadBuffs[RegRecPtr->LoadInProgress].DataSource, - OS_MAX_PATH_LEN); + sizeof(RegRecPtr->Buffers[0].DataSource)-1); + RegRecPtr->Buffers[0].DataSource[sizeof(RegRecPtr->Buffers[0].DataSource)-1] = 0; strncpy(RegRecPtr->LastFileLoaded, CFE_TBL_TaskData.LoadBuffs[RegRecPtr->LoadInProgress].DataSource, - OS_MAX_PATH_LEN); + sizeof(RegRecPtr->LastFileLoaded)-1); + RegRecPtr->LastFileLoaded[sizeof(RegRecPtr->LastFileLoaded)-1] = 0; /* Save the file creation time from the loaded file into the Table Registry */ RegRecPtr->Buffers[0].FileCreateTimeSecs = diff --git a/fsw/cfe-core/src/tbl/cfe_tbl_task_cmds.c b/fsw/cfe-core/src/tbl/cfe_tbl_task_cmds.c index e697ce72d..6c83341c7 100644 --- a/fsw/cfe-core/src/tbl/cfe_tbl_task_cmds.c +++ b/fsw/cfe-core/src/tbl/cfe_tbl_task_cmds.c @@ -768,7 +768,8 @@ CFE_TBL_CmdProcRet_t CFE_TBL_DumpToFile( const char *DumpFilename, const char *T if (Status == sizeof(CFE_FS_Header_t)) { /* Initialize the Table Image Header for the Dump File */ - strncpy(TblFileHeader.TableName, TableName, sizeof(TblFileHeader.TableName)); + strncpy(TblFileHeader.TableName, TableName, sizeof(TblFileHeader.TableName)-1); + TblFileHeader.TableName[sizeof(TblFileHeader.TableName)-1] = 0; TblFileHeader.Offset = 0; TblFileHeader.NumBytes = TblSizeInBytes; TblFileHeader.Reserved = 0; @@ -814,7 +815,8 @@ CFE_TBL_CmdProcRet_t CFE_TBL_DumpToFile( const char *DumpFilename, const char *T /* Save file information statistics for housekeeping telemetry */ strncpy(CFE_TBL_TaskData.HkPacket.Payload.LastFileDumped, DumpFilename, - sizeof(CFE_TBL_TaskData.HkPacket.Payload.LastFileDumped)); + sizeof(CFE_TBL_TaskData.HkPacket.Payload.LastFileDumped)-1); + CFE_TBL_TaskData.HkPacket.Payload.LastFileDumped[sizeof(CFE_TBL_TaskData.HkPacket.Payload.LastFileDumped)-1] = 0; /* Increment Successful Command Counter */ ReturnCode = CFE_TBL_INC_CMD_CTR; @@ -1206,8 +1208,8 @@ int32 CFE_TBL_DumpRegistryCmd(const CFE_TBL_DumpRegistry_t *data) memset(DumpRecord.LastFileLoaded, 0, OS_MAX_PATH_LEN); memset(DumpRecord.OwnerAppName, 0, OS_MAX_API_NAME); - strncpy(DumpRecord.Name, RegRecPtr->Name, CFE_TBL_MAX_FULL_NAME_LEN); - strncpy(DumpRecord.LastFileLoaded, RegRecPtr->LastFileLoaded, OS_MAX_PATH_LEN); + strncpy(DumpRecord.Name, RegRecPtr->Name, sizeof(DumpRecord.Name)-1); + strncpy(DumpRecord.LastFileLoaded, RegRecPtr->LastFileLoaded, sizeof(DumpRecord.LastFileLoaded)-1); /* Walk the access descriptor list to determine the number of users */ DumpRecord.NumUsers = 0; @@ -1221,11 +1223,11 @@ int32 CFE_TBL_DumpRegistryCmd(const CFE_TBL_DumpRegistry_t *data) /* Determine the name of the owning application */ if (RegRecPtr->OwnerAppId != CFE_TBL_NOT_OWNED) { - CFE_ES_GetAppName(DumpRecord.OwnerAppName, RegRecPtr->OwnerAppId, OS_MAX_API_NAME); + CFE_ES_GetAppName(DumpRecord.OwnerAppName, RegRecPtr->OwnerAppId, sizeof(DumpRecord.OwnerAppName)); } else { - strncpy(DumpRecord.OwnerAppName, "--UNOWNED--", OS_MAX_API_NAME); + strncpy(DumpRecord.OwnerAppName, "--UNOWNED--", sizeof(DumpRecord.OwnerAppName)-1); } /* Output Registry Dump Record to Registry Dump File */ diff --git a/fsw/cfe-core/unit-test/es_UT.c b/fsw/cfe-core/unit-test/es_UT.c index 065f3a061..534bf4f4b 100644 --- a/fsw/cfe-core/unit-test/es_UT.c +++ b/fsw/cfe-core/unit-test/es_UT.c @@ -2696,7 +2696,7 @@ void TestTask(void) sizeof(CmdBuf.StartAppCmd.Payload.AppFileName)); strncpy((char *) CmdBuf.StartAppCmd.Payload.AppEntryPoint, "entrypoint", sizeof(CmdBuf.StartAppCmd.Payload.AppEntryPoint)); - strncpy((char *) CmdBuf.StartAppCmd.Payload.Application, "appNameIntentionallyTooLongToFitIntoDestinationBuffer", + memset(CmdBuf.StartAppCmd.Payload.Application, 'x', sizeof(CmdBuf.StartAppCmd.Payload.Application)); CmdBuf.StartAppCmd.Payload.Priority = 160; CmdBuf.StartAppCmd.Payload.StackSize = 8192; diff --git a/fsw/cfe-core/unit-test/tbl_UT.c b/fsw/cfe-core/unit-test/tbl_UT.c index d878ccb2c..da8baf873 100644 --- a/fsw/cfe-core/unit-test/tbl_UT.c +++ b/fsw/cfe-core/unit-test/tbl_UT.c @@ -3725,11 +3725,14 @@ void Test_CFE_TBL_Manage(void) * later */ CFE_TBL_TaskData.DumpControlBlocks[0].DumpBufferPtr = WorkingBufferPtr; - memcpy(CFE_TBL_TaskData.DumpControlBlocks[0]. + strncpy(CFE_TBL_TaskData.DumpControlBlocks[0]. DumpBufferPtr->DataSource, - "MyDumpFilename", OS_MAX_PATH_LEN); - memcpy(CFE_TBL_TaskData.DumpControlBlocks[0].TableName, - "ut_cfe_tbl.UT_Table2", CFE_TBL_MAX_FULL_NAME_LEN); + "MyDumpFilename", OS_MAX_PATH_LEN-1); + CFE_TBL_TaskData.DumpControlBlocks[0]. + DumpBufferPtr->DataSource[OS_MAX_PATH_LEN-1] = 0; + strncpy(CFE_TBL_TaskData.DumpControlBlocks[0].TableName, + "ut_cfe_tbl.UT_Table2", CFE_TBL_MAX_FULL_NAME_LEN-1); + CFE_TBL_TaskData.DumpControlBlocks[0].TableName[CFE_TBL_MAX_FULL_NAME_LEN-1] = 0; CFE_TBL_TaskData.DumpControlBlocks[0].Size = RegRecPtr->Size; RegRecPtr->DumpControlIndex = 0; RtnCode = CFE_TBL_Manage(App1TblHandle2); diff --git a/fsw/cfe-core/unit-test/time_UT.c b/fsw/cfe-core/unit-test/time_UT.c index 7e1d9e94d..a43d426e9 100644 --- a/fsw/cfe-core/unit-test/time_UT.c +++ b/fsw/cfe-core/unit-test/time_UT.c @@ -1266,7 +1266,7 @@ void Test_ConvertCFEFS(void) void Test_Print(void) { int result; - char testDesc[UT_MAX_MESSAGE_LENGTH]; + char testDesc[1+UT_MAX_MESSAGE_LENGTH]; CFE_TIME_SysTime_t time; #ifdef UT_VERBOSE @@ -1279,8 +1279,8 @@ void Test_Print(void) time.Seconds = 0; CFE_TIME_Print(testDesc, time); result = !strcmp(testDesc, "1980-001-00:00:00.00000"); - strncat(testDesc," Zero time value", UT_MAX_MESSAGE_LENGTH); - testDesc[UT_MAX_MESSAGE_LENGTH - 1] = '\0'; + strncat(testDesc," Zero time value", + UT_MAX_MESSAGE_LENGTH - strlen(testDesc)); UT_Report(__FILE__, __LINE__, result, "CFE_TIME_Print", @@ -1296,8 +1296,7 @@ void Test_Print(void) result = !strcmp(testDesc, "1980-001-00:00:59.00000"); strncat(testDesc, " Seconds overflow if CFE_MISSION_TIME_EPOCH_SECOND > 0", - UT_MAX_MESSAGE_LENGTH); - testDesc[UT_MAX_MESSAGE_LENGTH - 1] = '\0'; + UT_MAX_MESSAGE_LENGTH - strlen(testDesc)); UT_Report(__FILE__, __LINE__, result, "CFE_TIME_Print", @@ -1309,8 +1308,8 @@ void Test_Print(void) time.Seconds = 1041472984; CFE_TIME_Print(testDesc, time); result = !strcmp(testDesc, "2013-001-02:03:04.00005"); - strncat(testDesc," Mission representative time", UT_MAX_MESSAGE_LENGTH); - testDesc[UT_MAX_MESSAGE_LENGTH - 1] = '\0'; + strncat(testDesc," Mission representative time", + UT_MAX_MESSAGE_LENGTH - strlen(testDesc)); UT_Report(__FILE__, __LINE__, result, "CFE_TIME_Print", @@ -1323,8 +1322,7 @@ void Test_Print(void) CFE_TIME_Print(testDesc, time); result = !strcmp(testDesc, "2116-038-06:28:15.99999"); strncat(testDesc," Maximum seconds/subseconds values", - UT_MAX_MESSAGE_LENGTH); - testDesc[UT_MAX_MESSAGE_LENGTH - 1] = '\0'; + UT_MAX_MESSAGE_LENGTH - strlen(testDesc)); UT_Report(__FILE__, __LINE__, result, "CFE_TIME_Print", diff --git a/fsw/cfe-core/unit-test/ut_support.c b/fsw/cfe-core/unit-test/ut_support.c index f1a363716..7c9281e8b 100644 --- a/fsw/cfe-core/unit-test/ut_support.c +++ b/fsw/cfe-core/unit-test/ut_support.c @@ -82,7 +82,8 @@ void UT_Init(const char *subsys) int8 i; /* Copy the application name for later use */ - strncpy(UT_subsys, subsys, 5); + strncpy(UT_subsys, subsys, sizeof(UT_subsys)-1); + UT_subsys[sizeof(UT_subsys)-1] = 0; snprintf(UT_appname, 80, "ut_cfe_%s", subsys); /* Convert to upper case */