From 7f0c08eee0e4d88443fe85bb903898cc2385daac Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Wed, 2 Sep 2020 11:22:48 -0400 Subject: [PATCH] Fix #797, Update TIME subsystem for abstract IDs Update the TIME subsystem to use the new ES-supplied ID abstractions. Do not use AppID directly as array index when registering sync callbacks, use the ES-supplied conversion to array index before accessing local table. Also update logging to use ES-supplied conversion --- fsw/cfe-core/src/time/cfe_time_api.c | 22 ++++++++++++++++------ fsw/cfe-core/src/time/cfe_time_utils.c | 11 ++++++++--- fsw/cfe-core/unit-test/time_UT.c | 21 ++++++++++++--------- 3 files changed, 36 insertions(+), 18 deletions(-) diff --git a/fsw/cfe-core/src/time/cfe_time_api.c b/fsw/cfe-core/src/time/cfe_time_api.c index 4f03f65c7..af4b0471d 100644 --- a/fsw/cfe-core/src/time/cfe_time_api.c +++ b/fsw/cfe-core/src/time/cfe_time_api.c @@ -819,22 +819,27 @@ int32 CFE_TIME_RegisterSynchCallback(CFE_TIME_SynchCallbackPtr_t CallbackFuncPt { int32 Status; uint32 AppId; + uint32 AppIndex; Status = CFE_ES_GetAppID(&AppId); + if (Status == CFE_SUCCESS) + { + Status = CFE_ES_AppID_ToIndex(AppId, &AppIndex); + } if (Status != CFE_SUCCESS) { /* Called from an invalid context */ return Status; } - if (AppId >= (sizeof(CFE_TIME_TaskData.SynchCallback) / sizeof(CFE_TIME_TaskData.SynchCallback[0])) || - CFE_TIME_TaskData.SynchCallback[AppId].Ptr != NULL) + if (AppIndex >= (sizeof(CFE_TIME_TaskData.SynchCallback) / sizeof(CFE_TIME_TaskData.SynchCallback[0])) || + CFE_TIME_TaskData.SynchCallback[AppIndex].Ptr != NULL) { Status = CFE_TIME_TOO_MANY_SYNCH_CALLBACKS; } else { - CFE_TIME_TaskData.SynchCallback[AppId].Ptr = CallbackFuncPtr; + CFE_TIME_TaskData.SynchCallback[AppIndex].Ptr = CallbackFuncPtr; } return Status; @@ -848,22 +853,27 @@ int32 CFE_TIME_UnregisterSynchCallback(CFE_TIME_SynchCallbackPtr_t CallbackFunc { int32 Status; uint32 AppId; + uint32 AppIndex; Status = CFE_ES_GetAppID(&AppId); + if (Status == CFE_SUCCESS) + { + Status = CFE_ES_AppID_ToIndex(AppId, &AppIndex); + } if (Status != CFE_SUCCESS) { /* Called from an invalid context */ return Status; } - if (AppId >= (sizeof(CFE_TIME_TaskData.SynchCallback) / sizeof(CFE_TIME_TaskData.SynchCallback[0])) || - CFE_TIME_TaskData.SynchCallback[AppId].Ptr != CallbackFuncPtr) + if (AppIndex >= (sizeof(CFE_TIME_TaskData.SynchCallback) / sizeof(CFE_TIME_TaskData.SynchCallback[0])) || + CFE_TIME_TaskData.SynchCallback[AppIndex].Ptr != CallbackFuncPtr) { Status = CFE_TIME_CALLBACK_NOT_REGISTERED; } else { - CFE_TIME_TaskData.SynchCallback[AppId].Ptr = NULL; + CFE_TIME_TaskData.SynchCallback[AppIndex].Ptr = NULL; } return Status; diff --git a/fsw/cfe-core/src/time/cfe_time_utils.c b/fsw/cfe-core/src/time/cfe_time_utils.c index 6e2cb80d7..64e6b92a7 100644 --- a/fsw/cfe-core/src/time/cfe_time_utils.c +++ b/fsw/cfe-core/src/time/cfe_time_utils.c @@ -1139,11 +1139,16 @@ void CFE_TIME_Set1HzAdj(CFE_TIME_SysTime_t NewAdjust, int16 Direction) int32 CFE_TIME_CleanUpApp(uint32 AppId) { int32 Status; + uint32 AppIndex; - if (AppId < (sizeof(CFE_TIME_TaskData.SynchCallback) / sizeof(CFE_TIME_TaskData.SynchCallback[0]))) + Status = CFE_ES_AppID_ToIndex(AppId, &AppIndex); + if (Status != CFE_SUCCESS) { - CFE_TIME_TaskData.SynchCallback[AppId].Ptr = NULL; - Status = CFE_SUCCESS; + /* Do nothing */ + } + else if (AppIndex < (sizeof(CFE_TIME_TaskData.SynchCallback) / sizeof(CFE_TIME_TaskData.SynchCallback[0]))) + { + CFE_TIME_TaskData.SynchCallback[AppIndex].Ptr = NULL; } else { diff --git a/fsw/cfe-core/unit-test/time_UT.c b/fsw/cfe-core/unit-test/time_UT.c index ca1e870d7..01554ec8e 100644 --- a/fsw/cfe-core/unit-test/time_UT.c +++ b/fsw/cfe-core/unit-test/time_UT.c @@ -3297,6 +3297,7 @@ void Test_CleanUpApp(void) uint16 i; uint16 Count; int32 Status = CFE_SUCCESS; + uint32 AppIndex; uint32 TestAppId; #ifdef UT_VERBOSE @@ -3313,16 +3314,19 @@ void Test_CleanUpApp(void) } /* Add callbacks for 3 apps into callback registry table */ - UT_SetAppID(1); + AppIndex = 1; + UT_SetDataBuffer(UT_KEY(CFE_ES_AppID_ToIndex), &AppIndex, sizeof(AppIndex), false); CFE_TIME_RegisterSynchCallback(&ut_time_MyCallbackFunc); - UT_SetAppID(2); + AppIndex = 2; + UT_SetDataBuffer(UT_KEY(CFE_ES_AppID_ToIndex), &AppIndex, sizeof(AppIndex), false); CFE_TIME_RegisterSynchCallback(&ut_time_MyCallbackFunc); - UT_SetAppID(3); + AppIndex = 3; + UT_SetDataBuffer(UT_KEY(CFE_ES_AppID_ToIndex), &AppIndex, sizeof(AppIndex), false); CFE_TIME_RegisterSynchCallback(&ut_time_MyCallbackFunc); /* Clean up an app which did not have a callback */ - TestAppId = 4; - UT_SetAppID(TestAppId); + AppIndex = 4; + UT_SetDataBuffer(UT_KEY(CFE_ES_AppID_ToIndex), &AppIndex, sizeof(AppIndex), false); Status = CFE_TIME_CleanUpApp(TestAppId); UT_Report(__FILE__, __LINE__, Status == CFE_SUCCESS, @@ -3345,8 +3349,8 @@ void Test_CleanUpApp(void) "No Sync Callback entry cleared"); /* Clean up an app which did have a callback */ - TestAppId = 2; - UT_SetAppID(TestAppId); + AppIndex = 2; + UT_SetDataBuffer(UT_KEY(CFE_ES_AppID_ToIndex), &AppIndex, sizeof(AppIndex), false); Status = CFE_TIME_CleanUpApp(TestAppId); UT_Report(__FILE__, __LINE__, Status == CFE_SUCCESS, @@ -3370,8 +3374,7 @@ void Test_CleanUpApp(void) /* Test response to a bad application ID - * This is effectively a no-op but here for coverage */ - UT_SetAppID(CFE_PLATFORM_ES_MAX_APPLICATIONS); - Status = CFE_TIME_CleanUpApp(CFE_PLATFORM_ES_MAX_APPLICATIONS); + Status = CFE_TIME_CleanUpApp(99999); UT_Report(__FILE__, __LINE__, Status == CFE_TIME_CALLBACK_NOT_REGISTERED, "CFE_TIME_CleanUpApp",