From 10d3bde51013d9794613c519f8aa2ebd7d7a1d61 Mon Sep 17 00:00:00 2001 From: Jacob Hageman Date: Mon, 9 Aug 2021 17:50:56 +0000 Subject: [PATCH 1/2] Fix #1775, Add invalid ID Syslog entry for CFE_ES_DeleteApp and CFE_ES_ReloadApp --- modules/es/fsw/src/cfe_es_api.c | 124 +++++++++++++++++--------------- 1 file changed, 68 insertions(+), 56 deletions(-) diff --git a/modules/es/fsw/src/cfe_es_api.c b/modules/es/fsw/src/cfe_es_api.c index f43bfc024..9df403d85 100644 --- a/modules/es/fsw/src/cfe_es_api.c +++ b/modules/es/fsw/src/cfe_es_api.c @@ -240,51 +240,57 @@ CFE_Status_t CFE_ES_ReloadApp(CFE_ES_AppId_t AppID, const char *AppFileName) os_fstat_t FileStatus; CFE_ES_AppRecord_t *AppRecPtr = CFE_ES_LocateAppRecordByID(AppID); - if (AppRecPtr == NULL) + if (AppRecPtr != NULL) { - return CFE_ES_ERR_RESOURCEID_NOT_VALID; - } - CFE_ES_LockSharedData(__func__, __LINE__); + CFE_ES_LockSharedData(__func__, __LINE__); - /* - ** Check to see if the App is an external cFE App. - */ - if (AppRecPtr->Type == CFE_ES_AppType_CORE) - { - CFE_ES_SysLogWrite_Unsync("%s: Cannot Reload a CORE Application: %s.\n", __func__, - CFE_ES_AppRecordGetName(AppRecPtr)); - ReturnCode = CFE_ES_ERR_RESOURCEID_NOT_VALID; - } - else if (AppRecPtr->AppState != CFE_ES_AppState_RUNNING) - { - CFE_ES_SysLogWrite_Unsync("%s: Cannot Reload Application %s, It is not running.\n", __func__, - CFE_ES_AppRecordGetName(AppRecPtr)); - ReturnCode = CFE_ES_ERR_RESOURCEID_NOT_VALID; - } - else - { /* - ** Check to see if the file exists + ** Check to see if the App is an external cFE App. */ - if (OS_stat(AppFileName, &FileStatus) == OS_SUCCESS) + if (AppRecPtr->Type == CFE_ES_AppType_CORE) { - CFE_ES_SysLogWrite_Unsync("%s: Reload Application %s Initiated. New filename = %s\n", __func__, - CFE_ES_AppRecordGetName(AppRecPtr), AppFileName); - strncpy(AppRecPtr->StartParams.BasicInfo.FileName, AppFileName, - sizeof(AppRecPtr->StartParams.BasicInfo.FileName) - 1); - AppRecPtr->StartParams.BasicInfo.FileName[sizeof(AppRecPtr->StartParams.BasicInfo.FileName) - 1] = 0; - AppRecPtr->ControlReq.AppControlRequest = CFE_ES_RunStatus_SYS_RELOAD; + CFE_ES_SysLogWrite_Unsync("%s: Cannot Reload a CORE Application: %s.\n", __func__, + CFE_ES_AppRecordGetName(AppRecPtr)); + ReturnCode = CFE_ES_ERR_RESOURCEID_NOT_VALID; + } + else if (AppRecPtr->AppState != CFE_ES_AppState_RUNNING) + { + CFE_ES_SysLogWrite_Unsync("%s: Cannot Reload Application %s, It is not running.\n", __func__, + CFE_ES_AppRecordGetName(AppRecPtr)); + ReturnCode = CFE_ES_ERR_RESOURCEID_NOT_VALID; } else { - CFE_ES_SysLogWrite_Unsync("%s: Cannot Reload Application %s, File %s does not exist.\n", __func__, - CFE_ES_AppRecordGetName(AppRecPtr), AppFileName); - ReturnCode = CFE_ES_FILE_IO_ERR; + /* + ** Check to see if the file exists + */ + if (OS_stat(AppFileName, &FileStatus) == OS_SUCCESS) + { + CFE_ES_SysLogWrite_Unsync("%s: Reload Application %s Initiated. New filename = %s\n", __func__, + CFE_ES_AppRecordGetName(AppRecPtr), AppFileName); + strncpy(AppRecPtr->StartParams.BasicInfo.FileName, AppFileName, + sizeof(AppRecPtr->StartParams.BasicInfo.FileName) - 1); + AppRecPtr->StartParams.BasicInfo.FileName[sizeof(AppRecPtr->StartParams.BasicInfo.FileName) - 1] = 0; + AppRecPtr->ControlReq.AppControlRequest = CFE_ES_RunStatus_SYS_RELOAD; + } + else + { + CFE_ES_SysLogWrite_Unsync("%s: Cannot Reload Application %s, File %s does not exist.\n", __func__, + CFE_ES_AppRecordGetName(AppRecPtr), AppFileName); + ReturnCode = CFE_ES_FILE_IO_ERR; + } } + + CFE_ES_UnlockSharedData(__func__, __LINE__); } + else /* App ID is not valid */ + { + ReturnCode = CFE_ES_ERR_RESOURCEID_NOT_VALID; - CFE_ES_UnlockSharedData(__func__, __LINE__); + CFE_ES_WriteToSysLog("%s: Invalid Application ID received, AppID = %lu\n", __func__, + CFE_RESOURCEID_TO_ULONG(AppID)); + } return (ReturnCode); } @@ -302,36 +308,42 @@ CFE_Status_t CFE_ES_DeleteApp(CFE_ES_AppId_t AppID) int32 ReturnCode = CFE_SUCCESS; CFE_ES_AppRecord_t *AppRecPtr = CFE_ES_LocateAppRecordByID(AppID); - if (AppRecPtr == NULL) + if (AppRecPtr != NULL) { - return CFE_ES_ERR_RESOURCEID_NOT_VALID; - } - CFE_ES_LockSharedData(__func__, __LINE__); + CFE_ES_LockSharedData(__func__, __LINE__); - /* - ** Check to see if the App is an external cFE App. - */ - if (AppRecPtr->Type == CFE_ES_AppType_CORE) - { - CFE_ES_SysLogWrite_Unsync("%s: Cannot Delete a CORE Application: %s.\n", __func__, - CFE_ES_AppRecordGetName(AppRecPtr)); - ReturnCode = CFE_ES_ERR_RESOURCEID_NOT_VALID; + /* + ** Check to see if the App is an external cFE App. + */ + if (AppRecPtr->Type == CFE_ES_AppType_CORE) + { + CFE_ES_SysLogWrite_Unsync("%s: Cannot Delete a CORE Application: %s.\n", __func__, + CFE_ES_AppRecordGetName(AppRecPtr)); + ReturnCode = CFE_ES_ERR_RESOURCEID_NOT_VALID; + } + else if (AppRecPtr->AppState != CFE_ES_AppState_RUNNING) + { + CFE_ES_SysLogWrite_Unsync("%s: Cannot Delete Application %s, It is not running.\n", __func__, + CFE_ES_AppRecordGetName(AppRecPtr)); + ReturnCode = CFE_ES_ERR_RESOURCEID_NOT_VALID; + } + else + { + CFE_ES_SysLogWrite_Unsync("%s: Delete Application %s Initiated\n", __func__, + CFE_ES_AppRecordGetName(AppRecPtr)); + AppRecPtr->ControlReq.AppControlRequest = CFE_ES_RunStatus_SYS_DELETE; + } + + CFE_ES_UnlockSharedData(__func__, __LINE__); } - else if (AppRecPtr->AppState != CFE_ES_AppState_RUNNING) + else /* App ID is not valid */ { - CFE_ES_SysLogWrite_Unsync("%s: Cannot Delete Application %s, It is not running.\n", __func__, - CFE_ES_AppRecordGetName(AppRecPtr)); ReturnCode = CFE_ES_ERR_RESOURCEID_NOT_VALID; - } - else - { - CFE_ES_SysLogWrite_Unsync("%s: Delete Application %s Initiated\n", __func__, - CFE_ES_AppRecordGetName(AppRecPtr)); - AppRecPtr->ControlReq.AppControlRequest = CFE_ES_RunStatus_SYS_DELETE; - } - CFE_ES_UnlockSharedData(__func__, __LINE__); + CFE_ES_WriteToSysLog("%s: Invalid Application ID received, AppID = %lu\n", __func__, + CFE_RESOURCEID_TO_ULONG(AppID)); + } return (ReturnCode); } From 69d27ea8856849f418542fe826e4c0dd14760cf5 Mon Sep 17 00:00:00 2001 From: Jacob Hageman Date: Mon, 9 Aug 2021 19:48:27 +0000 Subject: [PATCH 2/2] Fix #1773, Verify addition required error reporting from coverage test --- .../ut-stubs/inc/ut_osprintf_stubs.h | 11 ++++++- .../ut-stubs/src/ut_osprintf_stubs.c | 9 ++++++ modules/es/ut-coverage/es_UT.c | 30 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/modules/core_private/ut-stubs/inc/ut_osprintf_stubs.h b/modules/core_private/ut-stubs/inc/ut_osprintf_stubs.h index d0bf231f9..6641ad843 100644 --- a/modules/core_private/ut-stubs/inc/ut_osprintf_stubs.h +++ b/modules/core_private/ut-stubs/inc/ut_osprintf_stubs.h @@ -95,6 +95,15 @@ #define UT_OSP_STARTUP_SYNC_FAIL_2 67 #define UT_OSP_MODULE_UNLOAD_FAILED 68 #define UT_OSP_TASKEXIT_BAD_CONTEXT 69 -#define UT_OSP_BACKGROUND_TAKE 71 +#define UT_OSP_BACKGROUND_TAKE 70 +#define UT_OSP_INVALID_ID 71 +#define UT_OSP_RESTART_NO_FILE 72 +#define UT_OSP_CREATECHILD_FROM_CHILD 73 +#define UT_OSP_DELETECHID_MAIN_TASK 74 +#define UT_OSP_POOLCREATE_TOO_SMALL 75 +#define UT_OSP_GETPOOL_BAD_HANDLE 76 +#define UT_OSP_PUTPOOL_BAD_HANDLE 77 +#define UT_OSP_FORMAT_VOLATILE 78 +#define UT_OSP_RELOAD_NO_FILE 79 #endif /* UT_OSPRINTF_STUBS_H */ diff --git a/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c b/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c index e87e13e91..3973f58ab 100644 --- a/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c +++ b/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c @@ -101,4 +101,13 @@ const char *UT_OSP_MESSAGES[] = { [UT_OSP_RECORD_USED] = "%s: Error: ES_TaskTable slot for ID %lx in use at task creation!\n", [UT_OSP_TASKEXIT_BAD_CONTEXT] = "%s: Called from invalid task context\n", [UT_OSP_BACKGROUND_TAKE] = "%s: Failed to take background sem: %08lx\n", + [UT_OSP_INVALID_ID] = "%s: Invalid Application ID received, AppID = %lu\n", + [UT_OSP_RESTART_NO_FILE] = "%s: Cannot Restart Application %s, File %s does not exist.\n", + [UT_OSP_CREATECHILD_FROM_CHILD] = "%s: Error: Cannot call from a Child Task (for Task '%s').\n", + [UT_OSP_DELETECHID_MAIN_TASK] = "%s: Error: Task %lu is a cFE Main Task.\n", + [UT_OSP_POOLCREATE_TOO_SMALL] = "%s: Pool size(%lu) too small, need >=%lu bytes\n", + [UT_OSP_GETPOOL_BAD_HANDLE] = "%s: Err:Bad handle(0x%08lX) AppId=%lu\n", + [UT_OSP_PUTPOOL_BAD_HANDLE] = "%s: Err:Invalid Memory Handle (0x%08lX).\n", + [UT_OSP_FORMAT_VOLATILE] = "%s: Formatting Volatile(RAM) Volume.\n", + [UT_OSP_RELOAD_NO_FILE] = "%s: Cannot Reload Application %s, File %s does not exist.\n", }; diff --git a/modules/es/ut-coverage/es_UT.c b/modules/es/ut-coverage/es_UT.c index ca7156b5c..7fe7c802e 100644 --- a/modules/es/ut-coverage/es_UT.c +++ b/modules/es/ut-coverage/es_UT.c @@ -820,6 +820,8 @@ void TestStartupErrorPaths(void) CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_INIT_VOLATILE]); CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_MOUNT_VOLATILE]); CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_REFORMAT_VOLATILE]); + /* Verify requirement to report formatting */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_FORMAT_VOLATILE]); /* Test initialization of the file systems specifying a processor reset * following failure to get the volatile disk memory @@ -1105,6 +1107,7 @@ void TestApps(void) UT_SetDefaultReturnValue(UT_KEY(OS_TaskCreate), OS_ERROR); ES_UT_SetupAppStartParams(&StartParams, "ut/filename", "EntryPoint", 170, 4096, 1); UtAssert_INT32_EQ(CFE_ES_AppCreate(&AppId, "AppName", &StartParams), CFE_STATUS_EXTERNAL_RESOURCE_FAIL); + /* Verify requirement to report error */ CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_APP_CREATE]); /* Test application creation with NULL parameters */ @@ -1131,6 +1134,7 @@ void TestApps(void) UT_SetDeferredRetcode(UT_KEY(OS_ModuleLoad), 1, -1); ES_UT_SetupAppStartParams(&StartParams, "ut/filename.x", "EntryPoint", 170, 8192, 1); UtAssert_INT32_EQ(CFE_ES_AppCreate(&AppId, "AppName2", &StartParams), CFE_STATUS_EXTERNAL_RESOURCE_FAIL); + /* Verify requirement to report error */ CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_EXTRACT_FILENAME_UT55]); /* Test application loading and creation where all app slots are taken */ @@ -3525,6 +3529,8 @@ void TestAPI(void) /* Test CFE_ES_ReloadApp with bad AppID argument */ ES_ResetUnitTest(); UtAssert_INT32_EQ(CFE_ES_ReloadApp(CFE_ES_APPID_UNDEFINED, "filename"), CFE_ES_ERR_RESOURCEID_NOT_VALID); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_INVALID_ID]); /* Test reloading a core app */ ES_ResetUnitTest(); @@ -3550,6 +3556,8 @@ void TestAPI(void) ES_UT_SetupSingleAppId(CFE_ES_AppType_EXTERNAL, CFE_ES_AppState_RUNNING, NULL, &UtAppRecPtr, NULL); AppId = CFE_ES_AppRecordGetID(UtAppRecPtr); UtAssert_INT32_EQ(CFE_ES_ReloadApp(AppId, "missingfile"), CFE_ES_FILE_IO_ERR); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_RELOAD_NO_FILE]); /* Test deleting an app that doesn't exist */ ES_ResetUnitTest(); @@ -3557,6 +3565,11 @@ void TestAPI(void) AppId = CFE_ES_AppRecordGetID(UtAppRecPtr); UtAssert_INT32_EQ(CFE_ES_DeleteApp(AppId), CFE_ES_ERR_RESOURCEID_NOT_VALID); + /* Delete app with undefined ID */ + UtAssert_INT32_EQ(CFE_ES_DeleteApp(CFE_ES_APPID_UNDEFINED), CFE_ES_ERR_RESOURCEID_NOT_VALID); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_INVALID_ID]); + /* Test exiting an app with an init error */ ES_ResetUnitTest(); ES_UT_SetupSingleAppId(CFE_ES_AppType_CORE, CFE_ES_AppState_STOPPED, NULL, &UtAppRecPtr, NULL); @@ -3727,6 +3740,8 @@ void TestAPI(void) UT_SetDefaultReturnValue(UT_KEY(OS_TaskCreate), OS_ERROR); UtAssert_INT32_EQ(CFE_ES_CreateChildTask(&TaskId, "TaskName", TestAPI, StackBuf, sizeof(StackBuf), 400, 0), CFE_STATUS_EXTERNAL_RESOURCE_FAIL); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_APP_CREATE]); /* Test creating a child task with a null task ID */ ES_ResetUnitTest(); @@ -3756,6 +3771,8 @@ void TestAPI(void) UT_SetDefaultReturnValue(UT_KEY(OS_TaskGetId), OS_ObjectIdToInteger(TestObjId)); /* Set context to that of child */ UtAssert_INT32_EQ(CFE_ES_CreateChildTask(&TaskId, "TaskName", TestAPI, StackBuf, sizeof(StackBuf), 400, 0), CFE_ES_ERR_CHILD_TASK_CREATE); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_CREATECHILD_FROM_CHILD]); /* Test successfully creating a child task */ ES_ResetUnitTest(); @@ -3794,6 +3811,8 @@ void TestAPI(void) ES_UT_SetupSingleAppId(CFE_ES_AppType_EXTERNAL, CFE_ES_AppState_RUNNING, "UT", NULL, &UtTaskRecPtr); TaskId = CFE_ES_TaskRecordGetID(UtTaskRecPtr); UtAssert_INT32_EQ(CFE_ES_DeleteChildTask(TaskId), CFE_ES_ERR_CHILD_TASK_DELETE_MAIN_TASK); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_DELETECHID_MAIN_TASK]); /* Test deleting a child task with an invalid task ID */ UT_SetDefaultReturnValue(UT_KEY(OS_ObjectIdToArrayIndex), OS_ERROR); @@ -3839,6 +3858,7 @@ void TestAPI(void) ES_UT_SetupSingleAppId(CFE_ES_AppType_EXTERNAL, CFE_ES_AppState_RUNNING, NULL, &UtAppRecPtr, NULL); ES_UT_SetupChildTaskId(UtAppRecPtr, NULL, &UtTaskRecPtr); CFE_ES_ExitChildTask(); + /* Verify requirement to report error */ CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_CANNOT_CALL_APP_MAIN]); /* Test exiting a child task with an error retrieving the app ID */ @@ -4555,12 +4575,16 @@ void TestESMempool(void) */ ES_ResetUnitTest(); UtAssert_INT32_EQ(CFE_ES_PoolCreateNoSem(&PoolID1, Buffer1, 0), CFE_ES_BAD_ARGUMENT); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_POOLCREATE_TOO_SMALL]); /* Test successfully creating memory pool without using a mutex */ CFE_UtAssert_SUCCESS(CFE_ES_PoolCreateNoSem(&PoolID1, Buffer1, sizeof(Buffer1))); /* Test creating memory pool using a mutex with the pool size too small */ UtAssert_INT32_EQ(CFE_ES_PoolCreate(&PoolID2, Buffer2, 0), CFE_ES_BAD_ARGUMENT); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_POOLCREATE_TOO_SMALL]); /* Test successfully creating memory pool using a mutex */ CFE_UtAssert_SUCCESS(CFE_ES_PoolCreate(&PoolID2, Buffer2, sizeof(Buffer2))); @@ -4613,6 +4637,8 @@ void TestESMempool(void) * start address */ UtAssert_INT32_EQ(CFE_ES_GetPoolBuf(&addressp2, PoolID2, 256), CFE_ES_ERR_RESOURCEID_NOT_VALID); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_GETPOOL_BAD_HANDLE]); /* Test getting memory pool statistics where the memory handle is not * the pool start address @@ -4824,10 +4850,14 @@ void TestESMempool(void) /* Test returning a pool buffer using a null handle */ UtAssert_INT32_EQ(CFE_ES_PutPoolBuf(CFE_ES_MEMHANDLE_UNDEFINED, addressp2), CFE_ES_ERR_RESOURCEID_NOT_VALID); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_PUTPOOL_BAD_HANDLE]); /* Test allocating a pool buffer using a null handle */ ES_ResetUnitTest(); UtAssert_INT32_EQ(CFE_ES_GetPoolBuf(&addressp2, CFE_ES_MEMHANDLE_UNDEFINED, 256), CFE_ES_ERR_RESOURCEID_NOT_VALID); + /* Verify requirement to report error */ + CFE_UtAssert_PRINTF(UT_OSP_MESSAGES[UT_OSP_GETPOOL_BAD_HANDLE]); /* Test getting the size of an existing pool buffer using a null handle */ UtAssert_INT32_EQ(CFE_ES_GetPoolBufInfo(CFE_ES_MEMHANDLE_UNDEFINED, addressp1), CFE_ES_ERR_RESOURCEID_NOT_VALID);