Merge pull request nasa#1047 from nasa/integration-candidate

cFE Integration candidate: 2020-12-22

README.md

@@ -10,6 +10,14 @@ The detailed cFE user's guide can be viewed at <https://github.com/nasa/cFS/blob
 
 ## Version History
 
+### Development Build: 6.8.0-rc1+dev228
+
+- Remove use of `osapi-os-loader.h` from ES UT.
+- Use volatile `sig_atomic_t` for system state to avoid race issue if uint32 isn't atomic on a system
+- Set the flags parameter on the OS_ModuleLoad() properly to allow an app to be properly unloaded, which in turn allows the reload command to work as expected. Fixes problem where unload comand resulted in continuous restarting of the same app code.
+- Replaced `Test_MSG_PrintMsg` with `UT_DisplayPkt`. Also removed unused `Test_MSG_Sum`.
+- See <https://github.com/nasa/cFE/pull/1047>
+
 ### Development Build: 6.8.0-rc1+dev218
 
 - Adds `CFE_SB_TransmitMsg`, `CFE_SB_TransmitBuffer`, `CFE_SB_ReceiveBuffer`

fsw/cfe-core/src/es/cfe_es_api.c

@@ -1117,12 +1117,9 @@ int32 CFE_ES_GetLibInfo(CFE_ES_AppInfo_t *LibInfo, CFE_ES_ResourceID_t LibId)
 */
 int32 CFE_ES_GetModuleInfo(CFE_ES_AppInfo_t *ModuleInfo, CFE_ES_ResourceID_t ResourceId)
 {
-    uint32 ResourceType;
     int32 Status;
 
-    ResourceType = CFE_ES_ResourceID_ToInteger(ResourceId);
-    ResourceType -= ResourceType & CFE_ES_RESOURCEID_MAX;
-    switch(ResourceType)
+    switch(CFE_ES_ResourceID_GetBase(ResourceId))
     {
     case CFE_ES_APPID_BASE:
         Status = CFE_ES_GetAppInfo(ModuleInfo, ResourceId);

fsw/cfe-core/src/es/cfe_es_apps.c

@@ -369,25 +369,53 @@ int32 CFE_ES_ParseFileEntry(const char **TokenList, uint32 NumTokens)
 **
 **-------------------------------------------------------------------------------------
 */
-int32 CFE_ES_LoadModule(const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_ModuleLoadStatus_t *LoadStatus)
+int32 CFE_ES_LoadModule(CFE_ES_ResourceID_t ResourceId, const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_ModuleLoadStatus_t *LoadStatus)
 {
     osal_id_t ModuleId;
     cpuaddr StartAddr;
     int32 ReturnCode;
     int32 StatusCode;
+    uint32 LoadFlags;
 
+    LoadFlags = 0;
     StartAddr = 0;
     ReturnCode = CFE_SUCCESS;
 
     if (LoadParams->FileName[0] != 0)
     {
+        switch(CFE_ES_ResourceID_GetBase(ResourceId))
+        {
+        case CFE_ES_APPID_BASE:
+            /* 
+             * Apps should not typically have symbols exposed to other apps.
+             * 
+             * Keeping symbols local/private may help ensure this module is unloadable
+             * in the future, depending on underlying OS/loader implementation.
+             */
+            LoadFlags |= OS_MODULE_FLAG_LOCAL_SYMBOLS;
+            break;
+        case CFE_ES_LIBID_BASE:
+            /* 
+             * Libraries need to have their symbols exposed to other apps.
+             * 
+             * Note on some OS/loader implementations this may make it so the module
+             * cannot be unloaded, if there is no way to ensure that symbols
+             * are not being referenced.  CFE does not currently support unloading
+             * of libraries for this reason, among others.
+             */
+            LoadFlags |= OS_MODULE_FLAG_GLOBAL_SYMBOLS;
+            break;
+        default:
+            break;
+        }
+
         /*
-         ** Load the module via OSAL.
+         * Load the module via OSAL.
          */
         StatusCode = OS_ModuleLoad ( &ModuleId,
                 LoadParams->Name,
                 LoadParams->FileName,
-                OS_MODULE_FLAG_GLOBAL_SYMBOLS );
+                LoadFlags );
 
         if (StatusCode != OS_SUCCESS)
         {
@@ -403,11 +431,11 @@ int32 CFE_ES_LoadModule(const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_Modu
     }
 
     /*
-     ** If the Load was OK, then lookup the address of the entry point
+     * If the Load was OK, then lookup the address of the entry point
      */
     if (ReturnCode == CFE_SUCCESS && LoadParams->EntryPoint[0] != 0)
     {
-        StatusCode = OS_SymbolLookup(&StartAddr, LoadParams->EntryPoint);
+        StatusCode = OS_ModuleSymbolLookup(ModuleId, &StartAddr, LoadParams->EntryPoint);
         if (StatusCode != OS_SUCCESS)
         {
             CFE_ES_WriteToSysLog("ES Startup: Could not find symbol:%s. EC = 0x%08X\n",
@@ -717,7 +745,7 @@ int32 CFE_ES_AppCreate(CFE_ES_ResourceID_t *ApplicationIdPtr,
    /*
     * Load the module based on StartParams configured above.
     */
-   Status = CFE_ES_LoadModule(&AppRecPtr->StartParams.BasicInfo, &AppRecPtr->ModuleInfo);
+   Status = CFE_ES_LoadModule(PendingAppId, &AppRecPtr->StartParams.BasicInfo, &AppRecPtr->ModuleInfo);
 
    /*
     * If the Load was OK, then complete the initialization
@@ -883,7 +911,7 @@ int32 CFE_ES_LoadLibrary(CFE_ES_ResourceID_t       *LibraryIdPtr,
    /*
     * Load the module based on StartParams configured above.
     */
-   Status = CFE_ES_LoadModule(&LibSlotPtr->BasicInfo, &LibSlotPtr->ModuleInfo);
+   Status = CFE_ES_LoadModule(PendingLibId, &LibSlotPtr->BasicInfo, &LibSlotPtr->ModuleInfo);
    if (Status == CFE_SUCCESS)
    {
       FunctionPointer = (CFE_ES_LibraryEntryFuncPtr_t)LibSlotPtr->ModuleInfo.EntryAddress;

fsw/cfe-core/src/es/cfe_es_apps.h

@@ -198,7 +198,7 @@ int32 CFE_ES_ParseFileEntry(const char **TokenList, uint32 NumTokens);
 ** This only loads the code and looks up relevent runtime information.
 ** It does not start any tasks.
 */
-int32 CFE_ES_LoadModule(const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_ModuleLoadStatus_t *LoadStatus);
+int32 CFE_ES_LoadModule(CFE_ES_ResourceID_t ResourceId, const CFE_ES_ModuleLoadParams_t* LoadParams, CFE_ES_ModuleLoadStatus_t *LoadStatus);
 
 /*
 ** Internal function to determine the entry point of an app.

fsw/cfe-core/src/es/cfe_es_global.h

@@ -52,6 +52,8 @@
 #include "cfe_evs.h"
 #include "cfe_psp.h"
 
+#include <signal.h>
+
 
 /*
 ** Typedefs
@@ -104,7 +106,7 @@ typedef struct
    /*
    ** Startup Sync
    */
-   uint32  SystemState;
+   volatile sig_atomic_t SystemState;
 
    /*
    ** ES Task Table

fsw/cfe-core/src/es/cfe_es_resource.h

@@ -55,12 +55,34 @@
 #define CFE_ES_RESOURCEID_MAX   ((1 << CFE_ES_RESOURCEID_SHIFT)-1)
 #define CFE_ES_RESOURCEID_MARK  (0x02000000)
 
+/** 
+ * @defgroup CFEESResourceIDBase ES Resource ID base values 
+ * @{
+ */
 #define CFE_ES_APPID_BASE       (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+1) << CFE_ES_RESOURCEID_SHIFT))
 #define CFE_ES_LIBID_BASE       (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+2) << CFE_ES_RESOURCEID_SHIFT))
 #define CFE_ES_COUNTID_BASE     (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+3) << CFE_ES_RESOURCEID_SHIFT))
 #define CFE_ES_POOLID_BASE      (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+4) << CFE_ES_RESOURCEID_SHIFT))
 #define CFE_ES_CDSBLOCKID_BASE  (CFE_ES_RESOURCEID_MARK | ((OS_OBJECT_TYPE_USER+5) << CFE_ES_RESOURCEID_SHIFT))
+/** @} */
 
+/**
+ * @brief Get the Base value (type/category) from a resource ID value
+ *
+ * This masks out the ID serial number to obtain the base value, which is different
+ * for each resource type.
+ * 
+ * @note The value is NOT shifted or otherwise adjusted.  It should match one of the
+ * defined base values in @ref CFEESResourceIDBase.
+ *
+ * @param[in]   ResourceId   the resource ID to decode
+ * @returns     The base value associated with that ID
+ */
+static inline uint32 CFE_ES_ResourceID_GetBase(CFE_ES_ResourceID_t ResourceId)
+{
+    uint32 ResourceType = CFE_ES_ResourceID_ToInteger(ResourceId);
+    return (ResourceType - (ResourceType & CFE_ES_RESOURCEID_MAX));
+}
 
 /**
  * @brief Locate the next resource ID which does not map to an in-use table entry

fsw/cfe-core/src/inc/cfe_version.h

@@ -35,7 +35,7 @@
 
 
 /* Development Build Macro Definitions */
-#define CFE_BUILD_NUMBER 218 /*!< Development Build: Number of commits since baseline */
+#define CFE_BUILD_NUMBER 228 /*!< Development Build: Number of commits since baseline */
 #define CFE_BUILD_BASELINE "v6.8.0-rc1" /*!< Development Build: git tag that is the base for the current development */
 
 /* Version Macro Definitions */

fsw/cfe-core/unit-test/es_UT.c

@@ -1389,7 +1389,7 @@ void TestApps(void)
      * cannot be found
      */
     ES_ResetUnitTest();
-    UT_SetDeferredRetcode(UT_KEY(OS_SymbolLookup), 1, -1);
+    UT_SetDeferredRetcode(UT_KEY(OS_ModuleSymbolLookup), 1, -1);
     Return = CFE_ES_AppCreate(&Id,
                               "ut/filename.x",
                               "EntryPoint",
@@ -1405,7 +1405,7 @@ void TestApps(void)
      * cannot be found and module unload fails
      */
     ES_ResetUnitTest();
-    UT_SetDeferredRetcode(UT_KEY(OS_SymbolLookup), 1, -1);
+    UT_SetDeferredRetcode(UT_KEY(OS_ModuleSymbolLookup), 1, -1);
     UT_SetDeferredRetcode(UT_KEY(OS_ModuleUnload), 1, -1);
     Return = CFE_ES_AppCreate(&Id,
                               "ut/filename.x",
@@ -2272,7 +2272,7 @@ void TestLibs(void)
      * entry point symbol cannot be found
      */
     ES_ResetUnitTest();
-    UT_SetDeferredRetcode(UT_KEY(OS_SymbolLookup), 1, -1);
+    UT_SetDeferredRetcode(UT_KEY(OS_ModuleSymbolLookup), 1, -1);
     Return = CFE_ES_LoadLibrary(&Id,
                                 "/cf/apps/tst_lib.bundle",
                                 "TST_LIB_Init",

fsw/cfe-core/unit-test/es_UT.h

@@ -50,7 +50,6 @@
 #include "cfe.h"
 #include "common_types.h"
 #include "osapi.h"
-#include "osapi-os-loader.h"
 #include "cfe_version.h"
 #include "cfe_es.h"
 #include "cfe_es_cds.h"

modules/msg/unit-test-coverage/CMakeLists.txt

@@ -15,7 +15,6 @@ target_include_directories(ut_${DEP}_objs PRIVATE
 
 set (ut_${DEP}_tests
     msg_UT.c
-    test_msg_utils.c
     test_msg_not.c
     test_msg_pri_not.c
     test_cfe_msg_init.c

modules/msg/unit-test-coverage/test_cfe_msg_ccsdsext.c

@@ -72,7 +72,7 @@ void Test_MSG_Init_Ext(void)
     memset(&msg, 0xFF, sizeof(msg));
     msgidval_exp = 0;
     ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg)), CFE_SUCCESS);
-    Test_MSG_PrintMsg(&msg, 0);
+    UT_DisplayPkt(&msg, 0);
 
     /* Default EDS version check */
     ASSERT_EQ(CFE_MSG_GetEDSVersion(&msg, &edsver), CFE_SUCCESS);
@@ -104,7 +104,7 @@ void Test_MSG_Init_Ext(void)
     memset(&msg, 0, sizeof(msg));
     msgidval_exp = CFE_PLATFORM_SB_HIGHEST_VALID_MSGID;
     ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg)), CFE_SUCCESS);
-    Test_MSG_PrintMsg(&msg, 0);
+    UT_DisplayPkt(&msg, 0);
 
     /* Default EDS version check */
     ASSERT_EQ(CFE_MSG_GetEDSVersion(&msg, &edsver), CFE_SUCCESS);
@@ -160,7 +160,7 @@ void Test_MSG_EDSVersion(void)
         ASSERT_EQ(CFE_MSG_GetEDSVersion(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, TEST_EDSVER_MAX);
         ASSERT_EQ(CFE_MSG_SetEDSVersion(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetEDSVersion(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == TEST_EDSVER_MAX)
@@ -180,7 +180,7 @@ void Test_MSG_EDSVersion(void)
         ASSERT_EQ(CFE_MSG_GetEDSVersion(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, 0);
         ASSERT_EQ(CFE_MSG_SetEDSVersion(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetEDSVersion(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == 0)
@@ -220,7 +220,7 @@ void Test_MSG_Endian(void)
         ASSERT_EQ(CFE_MSG_GetEndian(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, CFE_MSG_Endian_Little);
         ASSERT_EQ(CFE_MSG_SetEndian(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetEndian(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == CFE_MSG_Endian_Little)
@@ -240,7 +240,7 @@ void Test_MSG_Endian(void)
         ASSERT_EQ(CFE_MSG_GetEndian(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, CFE_MSG_Endian_Big);
         ASSERT_EQ(CFE_MSG_SetEndian(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetEndian(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == CFE_MSG_Endian_Big)
@@ -280,7 +280,7 @@ void Test_MSG_PlaybackFlag(void)
         ASSERT_EQ(CFE_MSG_GetPlaybackFlag(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, CFE_MSG_PlayFlag_Playback);
         ASSERT_EQ(CFE_MSG_SetPlaybackFlag(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetPlaybackFlag(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == CFE_MSG_PlayFlag_Playback)
@@ -300,7 +300,7 @@ void Test_MSG_PlaybackFlag(void)
         ASSERT_EQ(CFE_MSG_GetPlaybackFlag(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, CFE_MSG_PlayFlag_Original);
         ASSERT_EQ(CFE_MSG_SetPlaybackFlag(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetPlaybackFlag(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == CFE_MSG_PlayFlag_Original)
@@ -340,7 +340,7 @@ void Test_MSG_Subsystem(void)
         ASSERT_EQ(CFE_MSG_GetSubsystem(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, TEST_SUBSYS_MAX);
         ASSERT_EQ(CFE_MSG_SetSubsystem(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetSubsystem(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == TEST_SUBSYS_MAX)
@@ -360,7 +360,7 @@ void Test_MSG_Subsystem(void)
         ASSERT_EQ(CFE_MSG_GetSubsystem(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, 0);
         ASSERT_EQ(CFE_MSG_SetSubsystem(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetSubsystem(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == 0)
@@ -397,7 +397,7 @@ void Test_MSG_System(void)
         ASSERT_EQ(CFE_MSG_GetSystem(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, TEST_SYSTEM_MAX);
         ASSERT_EQ(CFE_MSG_SetSystem(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetSystem(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == TEST_SYSTEM_MAX)
@@ -417,7 +417,7 @@ void Test_MSG_System(void)
         ASSERT_EQ(CFE_MSG_GetSystem(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, 0);
         ASSERT_EQ(CFE_MSG_SetSystem(&msg, input[i]), CFE_SUCCESS);
-        Test_MSG_PrintMsg(&msg, sizeof(msg));
+        UT_DisplayPkt(&msg, sizeof(msg));
         ASSERT_EQ(CFE_MSG_GetSystem(&msg, &actual), CFE_SUCCESS);
         ASSERT_EQ(actual, input[i]);
         if (input[i] == 0)