From 1ae46c0b16242dff943b74e86c2e30ebca776fe1 Mon Sep 17 00:00:00 2001 From: Jacob Hageman Date: Thu, 8 Apr 2021 15:32:54 +0000 Subject: [PATCH 01/16] Fix #1285, Remove broken BUILDDIR reference --- cmake/Makefile.sample | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmake/Makefile.sample b/cmake/Makefile.sample index db232e35c..48eddce98 100644 --- a/cmake/Makefile.sample +++ b/cmake/Makefile.sample @@ -2,7 +2,6 @@ # Core Flight Software CMake / GNU make wrapper # # ABOUT THIS MAKEFILE: -# This is actually part of the CMake alternative build system. # It is a GNU-make wrapper that calls the CMake tools appropriately # so that setting up a new build is fast and easy with no need to # learn the CMake commands. It also makes it easier to integrate @@ -124,7 +123,7 @@ install: prep $(O)/.prep: mkdir -p "$(O)" - (cd "$(O)/$(BUILDDIR)" && cmake $(PREP_OPTS) "$(CURDIR)/cfe") + (cd "$(O)" && cmake $(PREP_OPTS) "$(CURDIR)/cfe") echo "$(PREP_OPTS)" > "$(O)/.prep" clean: From 85584aa6efa948084b87e6453a9fb680231e44cd Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 12 Apr 2021 16:02:49 -0400 Subject: [PATCH 02/16] Fix #1301, remove option for "osal_compatible" Currently it is assumed/required that resource IDs follow the "osal compatible" pattern. Perhaps in a future version this could change, but for now it is simpler to just require this configuration, rather than having an option with only one choice. This renames/moves the header file but the content is not fundamentally changed. --- modules/resourceid/CMakeLists.txt | 1 + .../inc/cfe_resourceid_basevalue.h} | 6 +++--- modules/resourceid/mission_build.cmake | 10 ---------- 3 files changed, 4 insertions(+), 13 deletions(-) rename modules/resourceid/{option_inc/cfe_resourceid_osal_compatible.h => fsw/inc/cfe_resourceid_basevalue.h} (95%) diff --git a/modules/resourceid/CMakeLists.txt b/modules/resourceid/CMakeLists.txt index 34e568bb8..9acc860b9 100644 --- a/modules/resourceid/CMakeLists.txt +++ b/modules/resourceid/CMakeLists.txt @@ -13,6 +13,7 @@ set(resourceid_SOURCES add_library(resourceid STATIC ${resourceid_SOURCES}) target_link_libraries(resourceid PRIVATE core_private) +target_include_directories(resourceid PUBLIC fsw/inc) # Add unit test coverage subdirectory if(ENABLE_UNIT_TESTS) diff --git a/modules/resourceid/option_inc/cfe_resourceid_osal_compatible.h b/modules/resourceid/fsw/inc/cfe_resourceid_basevalue.h similarity index 95% rename from modules/resourceid/option_inc/cfe_resourceid_osal_compatible.h rename to modules/resourceid/fsw/inc/cfe_resourceid_basevalue.h index 93bbaffa7..ec0b2aa85 100644 --- a/modules/resourceid/option_inc/cfe_resourceid_osal_compatible.h +++ b/modules/resourceid/fsw/inc/cfe_resourceid_basevalue.h @@ -38,8 +38,8 @@ * OSAL ID structure. */ -#ifndef CFE_RESOURCEID_OSAL_COMPATIBLE_H -#define CFE_RESOURCEID_OSAL_COMPATIBLE_H +#ifndef CFE_RESOURCEID_BASEVALUE_H +#define CFE_RESOURCEID_BASEVALUE_H /* ** Include Files @@ -74,4 +74,4 @@ */ #define CFE_RESOURCEID_MAKE_BASE(offset) (CFE_RESOURCEID_MARK | ((offset) << CFE_RESOURCEID_SHIFT)) -#endif /* CFE_RESOURCEID_OSAL_COMPATIBLE_H */ +#endif /* CFE_RESOURCEID_BASEVALUE_H */ diff --git a/modules/resourceid/mission_build.cmake b/modules/resourceid/mission_build.cmake index 9319c48d6..158f4c9a2 100644 --- a/modules/resourceid/mission_build.cmake +++ b/modules/resourceid/mission_build.cmake @@ -20,13 +20,3 @@ generate_config_includefile( FILE_NAME "cfe_resourceid_typedef.h" FALLBACK_FILE "${CMAKE_CURRENT_LIST_DIR}/option_inc/${RESOURCEID_HDR_FILE}" ) - -# Resource ID base value header -# Currently the "osal compatible" version is the only provided implementation, -# but missions can provide their own if desired to override this. -generate_config_includefile( - FILE_NAME "cfe_resourceid_basevalue.h" - FALLBACK_FILE "${CMAKE_CURRENT_LIST_DIR}/option_inc/cfe_resourceid_osal_compatible.h" -) - -include_directories(${CMAKE_CURRENT_LIST_DIR}/inc) From 96341caf067902c3ab3ca74e54104fb7f842d06b Mon Sep 17 00:00:00 2001 From: Alex Campbell Date: Tue, 13 Apr 2021 10:10:21 -0400 Subject: [PATCH 03/16] Fix #1314, Remove Unused Error Codes --- modules/core_api/fsw/inc/cfe_error.h | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/modules/core_api/fsw/inc/cfe_error.h b/modules/core_api/fsw/inc/cfe_error.h index 8b9461cb8..c1b1923b2 100644 --- a/modules/core_api/fsw/inc/cfe_error.h +++ b/modules/core_api/fsw/inc/cfe_error.h @@ -347,14 +347,6 @@ typedef int32 CFE_Status_t; */ #define CFE_ES_ERR_CHILD_TASK_REGISTER ((CFE_Status_t)0xc400000b) -/** - * @brief Shell Command Error - * - * Error occured ehen trying to pass a system call to the OS shell - * - */ -#define CFE_ES_ERR_SHELL_CMD ((CFE_Status_t)0xc400000c) - /** * @brief CDS Already Exists * @@ -825,15 +817,6 @@ typedef int32 CFE_Status_t; */ #define CFE_SB_BUFFER_INVALID ((CFE_Status_t)0xca00000e) -/** - * @brief No Message Recieved - * - * When trying to determine the last senders ID, this return - * value indicates that there was not a message recived on the pipe. - * - */ -#define CFE_SB_NO_MSG_RECV ((CFE_Status_t)0xca00000f) - /** * @brief Not Implemented * From da861461cddfc44fe541b4228728e3e25e2d49b6 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 16 Apr 2021 15:40:01 -0400 Subject: [PATCH 04/16] Fix #1365, assert CFE_RESOURCEID_MAX is a bitmask Add a compile time assert to ensure that this value is actually a power of two-1. Notes in the comments that it serves as both a numeric limit and a mask. --- modules/resourceid/fsw/src/cfe_resourceid_api.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/modules/resourceid/fsw/src/cfe_resourceid_api.c b/modules/resourceid/fsw/src/cfe_resourceid_api.c index b05e4ab70..77317b36f 100644 --- a/modules/resourceid/fsw/src/cfe_resourceid_api.c +++ b/modules/resourceid/fsw/src/cfe_resourceid_api.c @@ -41,6 +41,15 @@ #include "cfe_resourceid.h" #include "cfe_resourceid_basevalue.h" +/* + * The "CFE_RESOURCEID_MAX" limit is used as both a numeric maximum as well + * as a mask to separate the serial number bits from the base value bits. + * + * This sanity checks that the value is one less than a power of two so it + * works as a mask and the logic in this file works as expected. + */ +CompileTimeAssert(((CFE_RESOURCEID_MAX + 1) & CFE_RESOURCEID_MAX) == 0, CFE_RESOURCEID_MAX_BITMASK); + /*********************************************************************/ /* * CFE_ResourceId_GetBase From c518d3a42268db1268f927ba07cd0c608eaa683b Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 16 Apr 2021 16:11:34 -0400 Subject: [PATCH 05/16] Fix #1346, error if alignment size not a power of two Instead of "fixing" the alignment mask, return an error if the passed-in value is not actually a power of two. In ES this only gets passed from two possible sources: one is hardcoded (4) and the other is sourced from the ALIGN_OF macro. --- modules/es/fsw/src/cfe_es_generic_pool.c | 16 ++++++++++------ modules/es/ut-coverage/es_UT.c | 7 +++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/modules/es/fsw/src/cfe_es_generic_pool.c b/modules/es/fsw/src/cfe_es_generic_pool.c index 742c4da43..2ffdcfb8d 100644 --- a/modules/es/fsw/src/cfe_es_generic_pool.c +++ b/modules/es/fsw/src/cfe_es_generic_pool.c @@ -252,7 +252,6 @@ int32 CFE_ES_GenPoolInitialize(CFE_ES_GenPoolRecord_t *PoolRecPtr, size_t StartO /* * Convert alignment to a bit mask. - * This sets all LSBs if the passed in value was not actually a power of 2. */ if (AlignSize <= 1) { @@ -261,11 +260,16 @@ int32 CFE_ES_GenPoolInitialize(CFE_ES_GenPoolRecord_t *PoolRecPtr, size_t StartO else { AlignMask = AlignSize - 1; - AlignMask |= AlignMask >> 1; - AlignMask |= AlignMask >> 2; - AlignMask |= AlignMask >> 4; - AlignMask |= AlignMask >> 8; - AlignMask |= AlignMask >> 16; + } + + /* + * This confirms that the passed in value is a power of 2. + * The result of this check should always be 0 if so. + */ + if ((AlignMask & AlignSize) != 0) + { + CFE_ES_WriteToSysLog("%s(): invalid alignment for pool: %lu\n", __func__, (unsigned long)AlignSize); + return CFE_ES_BAD_ARGUMENT; } /* complete initialization of pool record entry */ diff --git a/modules/es/ut-coverage/es_UT.c b/modules/es/ut-coverage/es_UT.c index 9910a2aa2..0ff8ff529 100644 --- a/modules/es/ut-coverage/es_UT.c +++ b/modules/es/ut-coverage/es_UT.c @@ -1987,6 +1987,13 @@ void TestGenericPool(void) ES_ResetUnitTest(); + /* Test Attempt to create pool with bad alignment / non power of 2 - should reject. */ + memset(&UT_MemPoolDirectBuffer, 0xee, sizeof(UT_MemPoolDirectBuffer)); + OffsetEnd = sizeof(UT_MemPoolDirectBuffer.Data); + UtAssert_INT32_EQ(CFE_ES_GenPoolInitialize(&Pool1, 0, OffsetEnd, 42, CFE_PLATFORM_ES_POOL_MAX_BUCKETS, + UT_POOL_BLOCK_SIZES, ES_UT_PoolDirectRetrieve, ES_UT_PoolDirectCommit), + CFE_ES_BAD_ARGUMENT); + /* Test successfully creating direct access pool, with alignment, no mutex */ memset(&UT_MemPoolDirectBuffer, 0xee, sizeof(UT_MemPoolDirectBuffer)); OffsetEnd = sizeof(UT_MemPoolDirectBuffer.Data); From 14faa472e6f703bb74a6d56125693831b63ace20 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 16 Apr 2021 16:39:18 -0400 Subject: [PATCH 06/16] Fix #1311, CFE_SUCCESS constant type Ensures that the CFE_SUCCESS constant is the CFE_Status_t type. --- modules/core_api/fsw/inc/cfe_error.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core_api/fsw/inc/cfe_error.h b/modules/core_api/fsw/inc/cfe_error.h index 8b9461cb8..eebe92a8d 100644 --- a/modules/core_api/fsw/inc/cfe_error.h +++ b/modules/core_api/fsw/inc/cfe_error.h @@ -117,7 +117,7 @@ typedef int32 CFE_Status_t; * * Operation was performed successfully */ -#define CFE_SUCCESS (0) +#define CFE_SUCCESS ((CFE_Status_t)0) /** * @brief No Counter Increment From 469d970e62129d9fd0c58e85b542843cca2337de Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 16 Apr 2021 15:13:56 -0400 Subject: [PATCH 07/16] Fix #1330, better warning about malformed startup line This improves the log message when a line in the startup script is not formed correctly, such as being too long or having too many tokens. --- .../ut-stubs/src/ut_osprintf_stubs.c | 2 +- modules/es/fsw/src/cfe_es_apps.c | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) 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 dcf28a370..5558f57a3 100644 --- a/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c +++ b/modules/core_private/ut-stubs/src/ut_osprintf_stubs.c @@ -69,7 +69,7 @@ const char *UT_OSP_MESSAGES[] = { /* Warning: System Log full, log entry discarded. */ [UT_OSP_SYSTEM_LOG_FULL] = "Warning: System Log full, log entry discarded.\n", /* ES Startup: ES Startup File Line is too long: 137 bytes. */ - [UT_OSP_FILE_LINE_TOO_LONG] = "ES Startup: ES Startup File Line is too long: %u bytes.\n", + [UT_OSP_FILE_LINE_TOO_LONG] = "ES Startup: **WARNING** File Line %u is malformed: %u bytes, %u tokens.\n", /* ES Startup: Load Shared Library Init Error. */ [UT_OSP_SHARED_LIBRARY_INIT] = "ES Startup: Load Shared Library Init Error = 0x%08x\n", /* ES Startup: Error Removing Volatile(RAM) Volume. EC = 0x~ */ diff --git a/modules/es/fsw/src/cfe_es_apps.c b/modules/es/fsw/src/cfe_es_apps.c index aed7353c8..0322e6efe 100644 --- a/modules/es/fsw/src/cfe_es_apps.c +++ b/modules/es/fsw/src/cfe_es_apps.c @@ -78,6 +78,7 @@ void CFE_ES_StartApplications(uint32 ResetType, const char *StartFilePath) char ScriptFileName[OS_MAX_PATH_LEN]; const char *TokenList[CFE_ES_STARTSCRIPT_MAX_TOKENS_PER_LINE]; uint32 NumTokens; + uint32 NumLines; uint32 BuffLen; /* Length of the current buffer */ osal_id_t AppFile = OS_OBJECT_ID_UNDEFINED; int32 Status; @@ -151,6 +152,7 @@ void CFE_ES_StartApplications(uint32 ResetType, const char *StartFilePath) memset(ES_AppLoadBuffer, 0x0, ES_START_BUFF_SIZE); BuffLen = 0; NumTokens = 0; + NumLines = 0; TokenList[0] = ES_AppLoadBuffer; /* @@ -197,14 +199,18 @@ void CFE_ES_StartApplications(uint32 ResetType, const char *StartFilePath) } BuffLen++; - if (NumTokens < (CFE_ES_STARTSCRIPT_MAX_TOKENS_PER_LINE - 1)) + ++NumTokens; + if (NumTokens < CFE_ES_STARTSCRIPT_MAX_TOKENS_PER_LINE) { /* * NOTE: pointer never deferenced unless "LineTooLong" is false. */ - ++NumTokens; TokenList[NumTokens] = &ES_AppLoadBuffer[BuffLen]; } + else + { + LineTooLong = true; + } } else if (c != ';') { @@ -223,13 +229,16 @@ void CFE_ES_StartApplications(uint32 ResetType, const char *StartFilePath) } else { + ++NumLines; + if (LineTooLong == true) { /* - ** The was too big for the buffer + ** The line was not formed correctly */ - CFE_ES_WriteToSysLog("ES Startup: ES Startup File Line is too long: %u bytes.\n", - (unsigned int)BuffLen); + CFE_ES_WriteToSysLog( + "ES Startup: **WARNING** File Line %u is malformed: %u bytes, %u tokens.\n", + (unsigned int)NumLines, (unsigned int)BuffLen, (unsigned int)NumTokens); LineTooLong = false; } else From 24b9a3b1ff7ea123fa2ddb5fdca195a3cd951b34 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 19 Apr 2021 11:31:22 -0400 Subject: [PATCH 08/16] Fix #1317, memory pool pointer type Changes the type of pointer in the API from uint8* to void* to be more consistent and easier to use. Should be backward compatible. This also updates the doxygen documentation for this parameter, as it was specifying a 32-bit alignment requirement whereas the alignment requirement is processor dependent - 64 bit processors typically will need 64 bit alignment. Link to the macro which is intended to aid in aligning the static pool buffer. --- modules/core_api/fsw/inc/cfe_es.h | 33 ++++++++++++--------- modules/core_api/ut-stubs/src/ut_es_stubs.c | 6 ++-- modules/es/fsw/src/cfe_es_mempool.c | 6 ++-- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/modules/core_api/fsw/inc/cfe_es.h b/modules/core_api/fsw/inc/cfe_es.h index 3af8c2f6d..fb645f160 100644 --- a/modules/core_api/fsw/inc/cfe_es.h +++ b/modules/core_api/fsw/inc/cfe_es.h @@ -1178,10 +1178,12 @@ CFE_Status_t CFE_ES_RestoreFromCDS(void *RestoreToMemory, CFE_ES_CDSHandle_t Han ** \param[in, out] PoolID A pointer to the variable the caller wishes to have the memory pool handle kept in. ** PoolID is the memory pool handle. ** -** \param[in] MemPtr A Pointer to the pool of memory created by the calling application. This address must -** be on a 32-bit boundary. +** \param[in] MemPtr A Pointer to the pool of memory created by the calling application. This address must +** be aligned suitably for the processor architecture. The #CFE_ES_STATIC_POOL_TYPE +** macro may be used to assist in creating properly aligned memory pools. ** -** \param[in] Size The size of the pool of memory. Note that this must be an integral number of 32 bit words. +** \param[in] Size The size of the pool of memory. Note that this must be an integral multiple of the +** memory alignment of the processor architecture. ** ** \return Execution status, see \ref CFEReturnCodes ** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS @@ -1190,7 +1192,7 @@ CFE_Status_t CFE_ES_RestoreFromCDS(void *RestoreToMemory, CFE_ES_CDSHandle_t Han ** \sa #CFE_ES_PoolCreate, #CFE_ES_PoolCreateEx, #CFE_ES_GetPoolBuf, #CFE_ES_PutPoolBuf, #CFE_ES_GetMemPoolStats ** ******************************************************************************/ -CFE_Status_t CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t Size); +CFE_Status_t CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, void *MemPtr, size_t Size); /*****************************************************************************/ /** @@ -1208,10 +1210,12 @@ CFE_Status_t CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, s ** \param[in, out] PoolID A pointer to the variable the caller wishes to have the memory pool handle kept in. ** PoolID is the memory pool handle. ** -** \param[in] MemPtr A Pointer to the pool of memory created by the calling application. This address must -** be on a 32-bit boundary. +** \param[in] MemPtr A Pointer to the pool of memory created by the calling application. This address must +** be aligned suitably for the processor architecture. The #CFE_ES_STATIC_POOL_TYPE +** macro may be used to assist in creating properly aligned memory pools. ** -** \param[in] Size The size of the pool of memory. Note that this must be an integral number of 32 bit words. +** \param[in] Size The size of the pool of memory. Note that this must be an integral multiple of the +** memory alignment of the processor architecture. ** ** \return Execution status, see \ref CFEReturnCodes ** \retval #CFE_SUCCESS \copybrief CFE_SUCCESS @@ -1220,7 +1224,7 @@ CFE_Status_t CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, s ** \sa #CFE_ES_PoolCreateNoSem, #CFE_ES_PoolCreateEx, #CFE_ES_GetPoolBuf, #CFE_ES_PutPoolBuf, #CFE_ES_GetMemPoolStats ** ******************************************************************************/ -CFE_Status_t CFE_ES_PoolCreate(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t Size); +CFE_Status_t CFE_ES_PoolCreate(CFE_ES_MemHandle_t *PoolID, void *MemPtr, size_t Size); /*****************************************************************************/ /** @@ -1234,14 +1238,15 @@ CFE_Status_t CFE_ES_PoolCreate(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t ** -# The start address of the pool must be 32-bit aligned ** -# 168 bytes are used for internal bookkeeping, therefore, they will not be available for allocation. ** -** \param[in, out] PoolID A pointer to the variable the caller wishes to have the memory pool handle kept in. -** PoolID is the memory pool handle. +** \param[in, out] PoolID A pointer to the variable the caller wishes to have the memory pool handle kept in. +** PoolID is the memory pool handle. ** ** \param[in] MemPtr A Pointer to the pool of memory created by the calling application. This address must -** be on a 32-bit boundary. +** be aligned suitably for the processor architecture. The #CFE_ES_STATIC_POOL_TYPE +** macro may be used to assist in creating properly aligned memory pools. ** -** \param[in] Size The size of the pool of memory. Note that this must be an integral number of 32 bit -** words. +** \param[in] Size The size of the pool of memory. Note that this must be an integral multiple of the +** memory alignment of the processor architecture. ** ** \param[in] NumBlockSizes The number of different block sizes specified in the \c BlockSizes array. If set equal to ** zero or if greater than 17, then default block sizes are used. @@ -1260,7 +1265,7 @@ CFE_Status_t CFE_ES_PoolCreate(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t ** \sa #CFE_ES_PoolCreate, #CFE_ES_PoolCreateNoSem, #CFE_ES_GetPoolBuf, #CFE_ES_PutPoolBuf, #CFE_ES_GetMemPoolStats ** ******************************************************************************/ -CFE_Status_t CFE_ES_PoolCreateEx(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t Size, uint16 NumBlockSizes, +CFE_Status_t CFE_ES_PoolCreateEx(CFE_ES_MemHandle_t *PoolID, void *MemPtr, size_t Size, uint16 NumBlockSizes, const size_t *BlockSizes, bool UseMutex); /*****************************************************************************/ diff --git a/modules/core_api/ut-stubs/src/ut_es_stubs.c b/modules/core_api/ut-stubs/src/ut_es_stubs.c index 21c70c26c..c1cdc24b7 100644 --- a/modules/core_api/ut-stubs/src/ut_es_stubs.c +++ b/modules/core_api/ut-stubs/src/ut_es_stubs.c @@ -535,7 +535,7 @@ int32 CFE_ES_GetPoolBuf(CFE_ES_MemPoolBuf_t *BufPtr, CFE_ES_MemHandle_t PoolID, ** Returns either a user-defined status flag or OS_SUCCESS. ** ******************************************************************************/ -CFE_Status_t CFE_ES_PoolCreate(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t Size) +CFE_Status_t CFE_ES_PoolCreate(CFE_ES_MemHandle_t *PoolID, void *MemPtr, size_t Size) { UT_Stub_RegisterContext(UT_KEY(CFE_ES_PoolCreate), PoolID); UT_Stub_RegisterContext(UT_KEY(CFE_ES_PoolCreate), MemPtr); @@ -568,7 +568,7 @@ CFE_Status_t CFE_ES_PoolCreate(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t ** Returns OS_SUCCESS. ** ******************************************************************************/ -CFE_Status_t CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t Size) +CFE_Status_t CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, void *MemPtr, size_t Size) { UT_Stub_RegisterContext(UT_KEY(CFE_ES_PoolCreateNoSem), PoolID); UT_Stub_RegisterContext(UT_KEY(CFE_ES_PoolCreateNoSem), MemPtr); @@ -606,7 +606,7 @@ CFE_Status_t CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, s ** Returns either a user-defined status flag or CFE_SUCCESS. ** ******************************************************************************/ -CFE_Status_t CFE_ES_PoolCreateEx(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t Size, uint16 NumBlockSizes, +CFE_Status_t CFE_ES_PoolCreateEx(CFE_ES_MemHandle_t *PoolID, void *MemPtr, size_t Size, uint16 NumBlockSizes, const size_t *BlockSizes, bool UseMutex) { UT_Stub_RegisterContext(UT_KEY(CFE_ES_PoolCreateEx), PoolID); diff --git a/modules/es/fsw/src/cfe_es_mempool.c b/modules/es/fsw/src/cfe_es_mempool.c index 6a7107547..f4db31da2 100644 --- a/modules/es/fsw/src/cfe_es_mempool.c +++ b/modules/es/fsw/src/cfe_es_mempool.c @@ -134,7 +134,7 @@ CFE_ES_MemPoolRecord_t *CFE_ES_LocateMemPoolRecordByID(CFE_ES_MemHandle_t PoolID /* ** CFE_ES_PoolCreateNoSem will initialize a pre-allocated memory pool without using a mutex. */ -int32 CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t Size) +int32 CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, void *MemPtr, size_t Size) { return CFE_ES_PoolCreateEx(PoolID, MemPtr, Size, CFE_PLATFORM_ES_POOL_MAX_BUCKETS, &CFE_ES_MemPoolDefSize[0], CFE_ES_NO_MUTEX); @@ -143,13 +143,13 @@ int32 CFE_ES_PoolCreateNoSem(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t S /* ** CFE_ES_PoolCreate will initialize a pre-allocated memory pool while using a mutex. */ -int32 CFE_ES_PoolCreate(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t Size) +int32 CFE_ES_PoolCreate(CFE_ES_MemHandle_t *PoolID, void *MemPtr, size_t Size) { return CFE_ES_PoolCreateEx(PoolID, MemPtr, Size, CFE_PLATFORM_ES_POOL_MAX_BUCKETS, &CFE_ES_MemPoolDefSize[0], CFE_ES_USE_MUTEX); } -int32 CFE_ES_PoolCreateEx(CFE_ES_MemHandle_t *PoolID, uint8 *MemPtr, size_t Size, uint16 NumBlockSizes, +int32 CFE_ES_PoolCreateEx(CFE_ES_MemHandle_t *PoolID, void *MemPtr, size_t Size, uint16 NumBlockSizes, const size_t *BlockSizes, bool UseMutex) { int32 Status; From e531836d26ea3cc0cbd25d647f461d81714d1550 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 19 Apr 2021 17:26:48 -0400 Subject: [PATCH 09/16] Fix #1355, improve documentation for resourceID patterns Improve the doxygen documentation on the various helper functions and common patterns dealing with Resource IDs. Specifically, document that the "IsMatch()" functions specifically accept NULL pointers to allow use with initial validation (gatekeeper), but all other helper functions assume a non-NULL pointer. --- modules/es/fsw/src/cfe_es_cds.h | 47 ++++++- modules/es/fsw/src/cfe_es_mempool.h | 42 +++++- modules/es/fsw/src/cfe_es_resource.h | 190 +++++++++++++++++++++++++-- modules/sb/fsw/src/cfe_sb_priv.h | 44 ++++++- 4 files changed, 300 insertions(+), 23 deletions(-) diff --git a/modules/es/fsw/src/cfe_es_cds.h b/modules/es/fsw/src/cfe_es_cds.h index 0a952a58a..eb5a2d7a3 100644 --- a/modules/es/fsw/src/cfe_es_cds.h +++ b/modules/es/fsw/src/cfe_es_cds.h @@ -267,12 +267,24 @@ int32 CFE_ES_CDSHandle_ToIndex(CFE_ES_CDSHandle_t BlockID, uint32 *Idx); /** * @brief Get a registry record within the CDS, given a block ID/handle * - * Retrieves a pointer to the registry record associated with a CDS block ID/handle - * Returns NULL if the handle is outside the valid range + * This only returns a pointer to the table entry where the record + * should reside, but does _not_ actually check/validate the entry. * - * @note This only does the lookup, it does not validate that the handle - * actually matches the returned record. The caller should lock the CDS and - * confirm that the record is a match to the expected ID before using it. + * If the passed-in ID parameter is not within the acceptable range of ID + * values for CDS blocks, such that it could never be valid under + * any circumstances, then NULL is returned. Otherwise, a pointer to the + * corresponding table entry is returned, indicating the location where + * that ID _should_ reside, if it is currently in use. + * + * @note This only returns where the ID should reside, not that it actually + * resides there. If looking up an existing ID, then caller must additionally + * confirm that the returned record is a match to the expected ID before using + * or modifying the data within the returned record pointer. + * + * The CFE_ES_CDSBlockRecordIsMatch() function can be used to check/confirm + * if the returned table entry is a positive match for the given ID. + * + * @sa CFE_ES_CDSBlockRecordIsMatch() * * @param[in] BlockID the ID/handle of the CDS block to retrieve * @returns Pointer to registry record, or NULL if ID/handle invalid. @@ -287,6 +299,9 @@ CFE_ES_CDS_RegRec_t *CFE_ES_LocateCDSBlockRecordByID(CFE_ES_CDSHandle_t BlockID) * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CDSBlockRecPtr pointer to Pool table entry * @returns true if the entry is in use/configured, or false if it is free/empty */ @@ -300,6 +315,9 @@ static inline bool CFE_ES_CDSBlockRecordIsUsed(const CFE_ES_CDS_RegRec_t *CDSBlo * * This routine converts the table entry back to an abstract ID. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CDSBlockRecPtr pointer to Pool table entry * @returns BlockID of entry */ @@ -314,6 +332,9 @@ static inline CFE_ES_CDSHandle_t CFE_ES_CDSBlockRecordGetID(const CFE_ES_CDS_Reg * This sets the internal field(s) within this entry, and marks * it as being associated with the given Pool ID. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CDSBlockRecPtr pointer to Pool table entry * @param[in] PendingId the Pool ID of this entry */ @@ -328,6 +349,9 @@ static inline void CFE_ES_CDSBlockRecordSetUsed(CFE_ES_CDS_RegRec_t *CDSBlockRec * This clears the internal field(s) within this entry, and allows the * memory to be re-used in the future. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CDSBlockRecPtr pointer to Pool table entry */ static inline void CFE_ES_CDSBlockRecordSetFree(CFE_ES_CDS_RegRec_t *CDSBlockRecPtr) @@ -344,6 +368,16 @@ static inline void CFE_ES_CDSBlockRecordSetFree(CFE_ES_CDS_RegRec_t *CDSBlockRec * As this dereferences fields within the record, CDS access mutex must be * locked prior to invoking this function. * + * This function may be used in conjunction with CFE_ES_LocateCDSBlockRecordByID() + * to confirm that the located record is a positive match to the expected ID. + * As such, the record pointer is also permitted to be NULL, to alleviate the + * need for the caller to handle this possibility explicitly. + * + * Once a record pointer has been successfully validated using this routine, + * it may be safely passed to all other internal functions. + * + * @sa CFE_ES_LocateCDSBlockRecordByID + * * @param[in] CDSBlockRecPtr pointer to registry table entry * @param[in] BlockID expected block ID * @returns true if the entry matches the given block ID @@ -365,6 +399,9 @@ static inline bool CFE_ES_CDSBlockRecordIsMatch(const CFE_ES_CDS_RegRec_t *CDSBl * which contains error checking information. Therefore the usable data * size is less than the raw block size. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CDSBlockRecPtr pointer to registry table entry * @returns Usable size of the CDS */ diff --git a/modules/es/fsw/src/cfe_es_mempool.h b/modules/es/fsw/src/cfe_es_mempool.h index 5c15f4cf6..5ba2ddfe7 100644 --- a/modules/es/fsw/src/cfe_es_mempool.h +++ b/modules/es/fsw/src/cfe_es_mempool.h @@ -100,8 +100,24 @@ int32 CFE_ES_MemPoolID_ToIndex(CFE_ES_MemHandle_t PoolID, uint32 *Idx); /** * @brief Locate the Pool table entry correlating with a given Pool ID. * - * This only returns a pointer to the table entry and does _not_ - * otherwise check/validate the entry. + * This only returns a pointer to the table entry where the record + * should reside, but does _not_ actually check/validate the entry. + * + * If the passed-in ID parameter is not within the acceptable range of ID + * values for memory pools, such that it could never be valid under + * any circumstances, then NULL is returned. Otherwise, a pointer to the + * corresponding table entry is returned, indicating the location where + * that ID _should_ reside, if it is currently in use. + * + * @note This only returns where the ID should reside, not that it actually + * resides there. If looking up an existing ID, then caller must additionally + * confirm that the returned record is a match to the expected ID before using + * or modifying the data within the returned record pointer. + * + * The CFE_ES_MemPoolRecordIsMatch() function can be used to check/confirm + * if the returned table entry is a positive match for the given ID. + * + * @sa CFE_ES_MemPoolRecordIsMatch() * * @param[in] PoolID the Pool ID to locate * @return pointer to Pool Table entry for the given Pool ID @@ -116,6 +132,9 @@ CFE_ES_MemPoolRecord_t *CFE_ES_LocateMemPoolRecordByID(CFE_ES_MemHandle_t PoolID * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] PoolRecPtr pointer to Pool table entry * @returns true if the entry is in use/configured, or false if it is free/empty */ @@ -129,6 +148,9 @@ static inline bool CFE_ES_MemPoolRecordIsUsed(const CFE_ES_MemPoolRecord_t *Pool * * This routine converts the table entry back to an abstract ID. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] PoolRecPtr pointer to Pool table entry * @returns PoolID of entry */ @@ -143,6 +165,9 @@ static inline CFE_ES_MemHandle_t CFE_ES_MemPoolRecordGetID(const CFE_ES_MemPoolR * This sets the internal field(s) within this entry, and marks * it as being associated with the given Pool ID. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] PoolRecPtr pointer to Pool table entry * @param[in] PendingId the Pool ID of this entry */ @@ -157,6 +182,9 @@ static inline void CFE_ES_MemPoolRecordSetUsed(CFE_ES_MemPoolRecord_t *PoolRecPt * This clears the internal field(s) within this entry, and allows the * memory to be re-used in the future. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] PoolRecPtr pointer to Pool table entry */ static inline void CFE_ES_MemPoolRecordSetFree(CFE_ES_MemPoolRecord_t *PoolRecPtr) @@ -173,6 +201,16 @@ static inline void CFE_ES_MemPoolRecordSetFree(CFE_ES_MemPoolRecord_t *PoolRecPt * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * This function may be used in conjunction with CFE_ES_LocateMemPoolRecordByID() + * to confirm that the located record is a positive match to the expected ID. + * As such, the record pointer is also permitted to be NULL, to alleviate the + * need for the caller to handle this possibility explicitly. + * + * Once a record pointer has been successfully validated using this routine, + * it may be safely passed to all other internal functions. + * + * @sa CFE_ES_LocateMemPoolRecordByID + * * @param[in] PoolRecPtr pointer to Pool table entry * @param[in] PoolID expected Pool ID * @returns true if the entry matches the given pool ID diff --git a/modules/es/fsw/src/cfe_es_resource.h b/modules/es/fsw/src/cfe_es_resource.h index 6d6f387b4..316e1e525 100644 --- a/modules/es/fsw/src/cfe_es_resource.h +++ b/modules/es/fsw/src/cfe_es_resource.h @@ -41,44 +41,108 @@ /** * @brief Locate the app table entry correlating with a given app ID. * - * This only returns a pointer to the table entry and does _not_ - * otherwise check/validate the entry. + * This only returns a pointer to the table entry where the record + * should reside, but does _not_ actually check/validate the entry. + * + * If the passed-in ID parameter is not within the acceptable range of ID + * values for applications, such that it could never be valid under + * any circumstances, then NULL is returned. Otherwise, a pointer to the + * corresponding table entry is returned, indicating the location where + * that ID _should_ reside, if it is currently in use. + * + * @note This only returns where the ID should reside, not that it actually + * resides there. If looking up an existing ID, then caller must additionally + * confirm that the returned record is a match to the expected ID before using + * or modifying the data within the returned record pointer. + * + * The CFE_ES_AppRecordIsMatch() function can be used to check/confirm + * if the returned table entry is a positive match for the given ID. + * + * @sa CFE_ES_AppRecordIsMatch() * * @param[in] AppID the app ID to locate - * @return pointer to App Table entry for the given app ID + * @return pointer to App Table entry for the given app ID, or NULL if out of range */ extern CFE_ES_AppRecord_t *CFE_ES_LocateAppRecordByID(CFE_ES_AppId_t AppID); /** * @brief Locate the Library table entry correlating with a given Lib ID. * - * This only returns a pointer to the table entry and does _not_ - * otherwise check/validate the entry. + * This only returns a pointer to the table entry where the record + * should reside, but does _not_ actually check/validate the entry. + * + * If the passed-in ID parameter is not within the acceptable range of ID + * values for libraries, such that it could never be valid under + * any circumstances, then NULL is returned. Otherwise, a pointer to the + * corresponding table entry is returned, indicating the location where + * that ID _should_ reside, if it is currently in use. + * + * @note This only returns where the ID should reside, not that it actually + * resides there. If looking up an existing ID, then caller must additionally + * confirm that the returned record is a match to the expected ID before using + * or modifying the data within the returned record pointer. + * + * The CFE_ES_LibRecordIsMatch() function can be used to check/confirm + * if the returned table entry is a positive match for the given ID. + * + * @sa CFE_ES_LibRecordIsMatch() * * @param[in] LibID the Lib ID to locate - * @return pointer to Library Table entry for the given Lib ID + * @return pointer to Library Table entry for the given Lib ID, or NULL if out of range */ extern CFE_ES_LibRecord_t *CFE_ES_LocateLibRecordByID(CFE_ES_LibId_t LibID); /** * @brief Locate the task table entry correlating with a given task ID. * - * This only returns a pointer to the table entry and does _not_ - * otherwise check/validate the entry. + * This only returns a pointer to the table entry where the record + * should reside, but does _not_ actually check/validate the entry. + * + * If the passed-in ID parameter is not within the acceptable range of ID + * values for tasks, such that it could never be valid under + * any circumstances, then NULL is returned. Otherwise, a pointer to the + * corresponding table entry is returned, indicating the location where + * that ID _should_ reside, if it is currently in use. + * + * @note This only returns where the ID should reside, not that it actually + * resides there. If looking up an existing ID, then caller must additionally + * confirm that the returned record is a match to the expected ID before using + * or modifying the data within the returned record pointer. + * + * The CFE_ES_TaskRecordIsMatch() function can be used to check/confirm + * if the returned table entry is a positive match for the given ID. + * + * @sa CFE_ES_TaskRecordIsMatch() * * @param[in] TaskID the task ID to locate - * @return pointer to Task Table entry for the given task ID + * @return pointer to Task Table entry for the given task ID, or NULL if out of range */ extern CFE_ES_TaskRecord_t *CFE_ES_LocateTaskRecordByID(CFE_ES_TaskId_t TaskID); /** * @brief Locate the Counter table entry correlating with a given Counter ID. * - * This only returns a pointer to the table entry and does _not_ - * otherwise check/validate the entry. + * This only returns a pointer to the table entry where the record + * should reside, but does _not_ actually check/validate the entry. + * + * If the passed-in ID parameter is not within the acceptable range of ID + * values for counters, such that it could never be valid under + * any circumstances, then NULL is returned. Otherwise, a pointer to the + * corresponding table entry is returned, indicating the location where + * that ID _should_ reside, if it is currently in use. + * + * @note This only returns where the ID should reside, not that it actually + * resides there. If looking up an existing ID, then caller must additionally + * confirm that the returned record is a match to the expected ID before using + * or modifying the data within the returned record pointer. + * + * The CFE_ES_CounterRecordIsMatch() function can be used to check/confirm + * if the returned table entry is a positive match for the given ID. + * + * @sa CFE_ES_CounterRecordIsMatch() * * @param[in] CounterID the Counter ID to locate - * @return pointer to Counter Table entry for the given Counter ID + * @return pointer to Counter Table entry for the given Counter ID, or NULL if out of range */ extern CFE_ES_GenCounterRecord_t *CFE_ES_LocateCounterRecordByID(CFE_ES_CounterId_t CounterID); @@ -90,6 +154,9 @@ extern CFE_ES_GenCounterRecord_t *CFE_ES_LocateCounterRecordByID(CFE_ES_CounterI * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] AppRecPtr pointer to app table entry * @returns true if the entry is in use/configured, or false if it is free/empty */ @@ -103,6 +170,9 @@ static inline bool CFE_ES_AppRecordIsUsed(const CFE_ES_AppRecord_t *AppRecPtr) * * This routine converts the table entry back to an abstract ID. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] AppRecPtr pointer to app table entry * @returns AppID of entry */ @@ -120,6 +190,9 @@ static inline CFE_ES_AppId_t CFE_ES_AppRecordGetID(const CFE_ES_AppRecord_t *App * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] AppRecPtr pointer to app table entry * @param[in] PendingId the app ID of this entry */ @@ -137,6 +210,9 @@ static inline void CFE_ES_AppRecordSetUsed(CFE_ES_AppRecord_t *AppRecPtr, CFE_Re * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] AppRecPtr pointer to app table entry */ static inline void CFE_ES_AppRecordSetFree(CFE_ES_AppRecord_t *AppRecPtr) @@ -153,7 +229,17 @@ static inline void CFE_ES_AppRecordSetFree(CFE_ES_AppRecord_t *AppRecPtr) * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * - * @param[in] AppRecPtr pointer to app table entry + * This function may be used in conjunction with CFE_ES_LocateAppRecordByID() + * to confirm that the located record is a positive match to the expected ID. + * As such, the record pointer is also permitted to be NULL, to alleviate the + * need for the caller to handle this possibility explicitly. + * + * Once a record pointer has been successfully validated using this routine, + * it may be safely passed to all other internal functions. + * + * @sa CFE_ES_LocateAppRecordByID + * + * @param[in] AppRecPtr pointer to app table entry, or NULL * @param[in] AppID expected app ID * @returns true if the entry matches the given app ID */ @@ -167,6 +253,9 @@ static inline bool CFE_ES_AppRecordIsMatch(const CFE_ES_AppRecord_t *AppRecPtr, * * Returns the name field from within the Application record * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] AppRecPtr pointer to App table entry * @returns Pointer to Application name */ @@ -183,6 +272,9 @@ static inline const char *CFE_ES_AppRecordGetName(const CFE_ES_AppRecord_t *AppR * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] LibRecPtr pointer to Lib table entry * @returns true if the entry is in use/configured, or false if it is free/empty */ @@ -196,6 +288,9 @@ static inline bool CFE_ES_LibRecordIsUsed(const CFE_ES_LibRecord_t *LibRecPtr) * * This routine converts the table entry back to an abstract ID. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] LibRecPtr pointer to Lib table entry * @returns LibID of entry */ @@ -214,6 +309,9 @@ static inline CFE_ES_LibId_t CFE_ES_LibRecordGetID(const CFE_ES_LibRecord_t *Lib * This sets the internal field(s) within this entry, and marks * it as being associated with the given Lib ID. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] LibRecPtr pointer to Lib table entry * @param[in] PendingId the Lib ID of this entry */ @@ -228,6 +326,9 @@ static inline void CFE_ES_LibRecordSetUsed(CFE_ES_LibRecord_t *LibRecPtr, CFE_Re * This clears the internal field(s) within this entry, and allows the * memory to be re-used in the future. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] LibRecPtr pointer to Lib table entry */ static inline void CFE_ES_LibRecordSetFree(CFE_ES_LibRecord_t *LibRecPtr) @@ -244,6 +345,16 @@ static inline void CFE_ES_LibRecordSetFree(CFE_ES_LibRecord_t *LibRecPtr) * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * This function may be used in conjunction with CFE_ES_LocateLibRecordByID() + * to confirm that the located record is a positive match to the expected ID. + * As such, the record pointer is also permitted to be NULL, to alleviate the + * need for the caller to handle this possibility explicitly. + * + * Once a record pointer has been successfully validated using this routine, + * it may be safely passed to all other internal functions. + * + * @sa CFE_ES_LocateLibRecordByID + * * @param[in] LibRecPtr pointer to Lib table entry * @param[in] LibID expected Lib ID * @returns true if the entry matches the given Lib ID @@ -258,6 +369,9 @@ static inline bool CFE_ES_LibRecordIsMatch(const CFE_ES_LibRecord_t *LibRecPtr, * * Returns the name field from within the Library record * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] LibRecPtr pointer to Lib table entry * @returns Pointer to Library name */ @@ -274,6 +388,9 @@ static inline const char *CFE_ES_LibRecordGetName(const CFE_ES_LibRecord_t *LibR * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] TaskRecPtr pointer to Task table entry * @returns TaskID of entry */ @@ -290,6 +407,9 @@ static inline CFE_ES_TaskId_t CFE_ES_TaskRecordGetID(const CFE_ES_TaskRecord_t * * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] TaskRecPtr pointer to task table entry * @returns true if the entry is in use/configured, or false if it is free/empty */ @@ -307,6 +427,9 @@ static inline bool CFE_ES_TaskRecordIsUsed(const CFE_ES_TaskRecord_t *TaskRecPtr * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] TaskRecPtr pointer to Task table entry * @param[in] PendingId the Task ID of this entry */ @@ -323,6 +446,9 @@ static inline void CFE_ES_TaskRecordSetUsed(CFE_ES_TaskRecord_t *TaskRecPtr, CFE * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] TaskRecPtr pointer to task table entry * @returns true if the entry is in use/configured, or false if it is free/empty */ @@ -340,6 +466,16 @@ static inline void CFE_ES_TaskRecordSetFree(CFE_ES_TaskRecord_t *TaskRecPtr) * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * This function may be used in conjunction with CFE_ES_LocateTaskRecordByID() + * to confirm that the located record is a positive match to the expected ID. + * As such, the record pointer is also permitted to be NULL, to alleviate the + * need for the caller to handle this possibility explicitly. + * + * Once a record pointer has been successfully validated using this routine, + * it may be safely passed to all other internal functions. + * + * @sa CFE_ES_LocateTaskRecordByID + * * @param[in] TaskRecPtr pointer to task table entry * @param[in] TaskID The expected task ID to verify * @returns true if the entry matches the given task ID @@ -354,6 +490,9 @@ static inline bool CFE_ES_TaskRecordIsMatch(const CFE_ES_TaskRecord_t *TaskRecPt * * Returns the name field from within the Task record * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] TaskRecPtr pointer to Task table entry * @returns Pointer to Task name */ @@ -370,6 +509,9 @@ static inline const char *CFE_ES_TaskRecordGetName(const CFE_ES_TaskRecord_t *Ta * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CounterRecPtr pointer to Counter table entry * @returns true if the entry is in use/configured, or false if it is free/empty */ @@ -383,6 +525,9 @@ static inline bool CFE_ES_CounterRecordIsUsed(const CFE_ES_GenCounterRecord_t *C * * This routine converts the table entry back to an abstract ID. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CounterRecPtr pointer to Counter table entry * @returns CounterID of entry */ @@ -400,6 +545,9 @@ static inline CFE_ES_CounterId_t CFE_ES_CounterRecordGetID(const CFE_ES_GenCount * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CounterRecPtr pointer to Counter table entry * @param[in] PendingId the Counter ID of this entry */ @@ -417,6 +565,9 @@ static inline void CFE_ES_CounterRecordSetUsed(CFE_ES_GenCounterRecord_t *Counte * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CounterRecPtr pointer to Counter table entry */ static inline void CFE_ES_CounterRecordSetFree(CFE_ES_GenCounterRecord_t *CounterRecPtr) @@ -433,6 +584,16 @@ static inline void CFE_ES_CounterRecordSetFree(CFE_ES_GenCounterRecord_t *Counte * As this dereferences fields within the record, global data must be * locked prior to invoking this function. * + * This function may be used in conjunction with CFE_ES_LocateCounterRecordByID() + * to confirm that the located record is a positive match to the expected ID. + * As such, the record pointer is also permitted to be NULL, to alleviate the + * need for the caller to handle this possibility explicitly. + * + * Once a record pointer has been successfully validated using this routine, + * it may be safely passed to all other internal functions. + * + * @sa CFE_ES_LocateCounterRecordByID + * * @param[in] CounterRecPtr pointer to Counter table entry * @param[in] CounterID expected Counter ID * @returns true if the entry matches the given Counter ID @@ -448,6 +609,9 @@ static inline bool CFE_ES_CounterRecordIsMatch(const CFE_ES_GenCounterRecord_t * * * Returns the name field from within the counter record * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] CounterRecPtr pointer to Counter table entry * @returns Pointer to counter name */ diff --git a/modules/sb/fsw/src/cfe_sb_priv.h b/modules/sb/fsw/src/cfe_sb_priv.h index a607853fb..db93107c4 100644 --- a/modules/sb/fsw/src/cfe_sb_priv.h +++ b/modules/sb/fsw/src/cfe_sb_priv.h @@ -524,8 +524,24 @@ int32 CFE_SB_SendPrevSubsCmd(const CFE_SB_SendPrevSubsCmd_t *data); /** * @brief Locate the Pipe table entry correlating with a given Pipe ID. * - * This only returns a pointer to the table entry and does _not_ - * otherwise check/validate the entry. + * This only returns a pointer to the table entry where the record + * should reside, but does _not_ actually check/validate the entry. + * + * If the passed-in ID parameter is not within the acceptable range of ID + * values for pipe IDs, such that it could never be valid under + * any circumstances, then NULL is returned. Otherwise, a pointer to the + * corresponding table entry is returned, indicating the location where + * that ID _should_ reside, if it is currently in use. + * + * @note This only returns where the ID should reside, not that it actually + * resides there. If looking up an existing ID, then caller must additionally + * confirm that the returned record is a match to the expected ID before using + * or modifying the data within the returned record pointer. + * + * The CFE_SB_PipeDescIsMatch() function can be used to check/confirm + * if the returned table entry is a positive match for the given ID. + * + * @sa CFE_SB_PipeDescIsMatch() * * @param[in] PipeId the Pipe ID to locate * @return pointer to Pipe Table entry for the given Pipe ID @@ -540,6 +556,9 @@ extern CFE_SB_PipeD_t *CFE_SB_LocatePipeDescByID(CFE_SB_PipeId_t PipeId); * As this dereferences fields within the descriptor, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] PipeDscPtr pointer to Pipe table entry * @returns true if the entry is in use/configured, or false if it is free/empty */ @@ -553,6 +572,9 @@ static inline bool CFE_SB_PipeDescIsUsed(const CFE_SB_PipeD_t *PipeDscPtr) * * This routine converts the table entry back to an abstract ID. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] PipeDscPtr pointer to Pipe table entry * @returns PipeID of entry */ @@ -570,8 +592,11 @@ static inline CFE_SB_PipeId_t CFE_SB_PipeDescGetID(const CFE_SB_PipeD_t *PipeDsc * As this dereferences fields within the descriptor, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] PipeDscPtr pointer to Pipe table entry - * @param[in] PipeID the Pipe ID of this entry + * @param[in] PendingID the Pipe ID of this entry */ static inline void CFE_SB_PipeDescSetUsed(CFE_SB_PipeD_t *PipeDscPtr, CFE_ResourceId_t PendingID) { @@ -587,6 +612,9 @@ static inline void CFE_SB_PipeDescSetUsed(CFE_SB_PipeD_t *PipeDscPtr, CFE_Resour * As this dereferences fields within the descriptor, global data must be * locked prior to invoking this function. * + * @note This internal helper function must only be used on record pointers + * that are known to refer to an actual table location (i.e. non-null). + * * @param[in] PipeDscPtr pointer to Pipe table entry */ static inline void CFE_SB_PipeDescSetFree(CFE_SB_PipeD_t *PipeDscPtr) @@ -603,6 +631,16 @@ static inline void CFE_SB_PipeDescSetFree(CFE_SB_PipeD_t *PipeDscPtr) * As this dereferences fields within the descriptor, global data must be * locked prior to invoking this function. * + * This function may be used in conjunction with CFE_SB_LocatePipeDescByID() + * to confirm that the located record is a positive match to the expected ID. + * As such, the record pointer is also permitted to be NULL, to alleviate the + * need for the caller to handle this possibility explicitly. + * + * Once a record pointer has been successfully validated using this routine, + * it may be safely passed to all other internal functions. + * + * @sa CFE_SB_LocatePipeDescByID + * * @param[in] PipeDscPtr pointer to Pipe table entry * @param[in] PipeID expected Pipe ID * @returns true if the entry matches the given Pipe ID From 9ed820fbc28703eae5068296d03bbeaee47132ae Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 19 Apr 2021 17:43:34 -0400 Subject: [PATCH 10/16] Fix #1340, update documentation for CFE_ES_DeleteCDS Noted that this does not actually wipe or erase the block, it only returns resources to the pool for re-use. --- modules/core_api/fsw/inc/cfe_es_core_internal.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/modules/core_api/fsw/inc/cfe_es_core_internal.h b/modules/core_api/fsw/inc/cfe_es_core_internal.h index e9e38a4b3..257c7d24f 100644 --- a/modules/core_api/fsw/inc/cfe_es_core_internal.h +++ b/modules/core_api/fsw/inc/cfe_es_core_internal.h @@ -112,8 +112,14 @@ int32 CFE_ES_RegisterCDSEx(CFE_ES_CDSHandle_t *HandlePtr, size_t UserBlockSize, ** Removes the record of the specified CDS from the CDS Registry and ** frees the associated CDS memory for future use. ** +** This operation invalidates the registry entry and returns +** the underlying data storage back to the CDS pool for re-use. +** ** \par Assumptions, External Events, and Notes: -** None +** The actual data block is not modified by this call. Specifically, this does not +** "wipe" or otherwise overwrite the data block. If the application needs to ensure +** that the data is actually erased, it should explicitly do so (by e.g. writing all +** zeros or all ones) before deleting the block. ** ** \param[in] CDSName - Pointer to character string containing complete ** CDS Name (of the format "AppName.CDSName"). From e0a90796449dcacf9c47a6b2984c991a99753104 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Mon, 19 Apr 2021 17:56:59 -0400 Subject: [PATCH 11/16] Fix #1345, exception logic when app/task is not found Adds comments to describe the logic when an exception cannot be traced back to a specific app, in that it should fall back to the PSP reset. Restarting only an app is a special opt-in case that is only done if specifically selected when starting the app, and the exception is also positively traced back to that app. --- modules/es/fsw/src/cfe_es_erlog.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/modules/es/fsw/src/cfe_es_erlog.c b/modules/es/fsw/src/cfe_es_erlog.c index 7370df0bd..8b2b6ffea 100644 --- a/modules/es/fsw/src/cfe_es_erlog.c +++ b/modules/es/fsw/src/cfe_es_erlog.c @@ -335,6 +335,14 @@ bool CFE_ES_RunExceptionScan(uint32 ElapsedTime, void *Arg) * * Otherwise, if it was related to a task, determine the associated AppID * so the exception action can be checked. + * + * NOTE: the default exception handling is to restart the processor/system. + * There is an option to only restart the specific app needs, but this must + * be "opt-in", that is, the app was created initially with this option, and + * the exception is also traced back to that app. If either is not possible + * for whatever reason then the restart action (default) should be taken, as + * this gets the highest assurance that the system will be returned to a coherent + * state. */ if (OS_ObjectIdDefined(ExceptionTaskID)) { @@ -342,6 +350,9 @@ bool CFE_ES_RunExceptionScan(uint32 ElapsedTime, void *Arg) /* * The App ID was found, now see if the ExceptionAction is set for a reset + * + * NOTE: if anything in this logic fails and the app which caused the exception is not + * postively identified, then this will just follow the default case of PSP reset. */ if (Status == CFE_SUCCESS) { From daa62c1cb19287b2831fe6d121e915af3300ae72 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 16 Apr 2021 13:47:38 -0400 Subject: [PATCH 12/16] Fix #1349, remove unneeded CFE_ES_SYSLOG_APPEND macro Replace remaining uses of this internal macro with the CFE_ES_WriteToSysLog API. --- modules/es/fsw/src/cfe_es_cds.c | 14 +++++----- modules/es/fsw/src/cfe_es_cds_mempool.c | 34 ++++++++----------------- modules/es/fsw/src/cfe_es_log.h | 20 --------------- 3 files changed, 16 insertions(+), 52 deletions(-) diff --git a/modules/es/fsw/src/cfe_es_cds.c b/modules/es/fsw/src/cfe_es_cds.c index 9e6d58603..3cbd1db4c 100644 --- a/modules/es/fsw/src/cfe_es_cds.c +++ b/modules/es/fsw/src/cfe_es_cds.c @@ -885,10 +885,9 @@ int32 CFE_ES_DeleteCDS(const char *CDSName, bool CalledByTblServices) /* Report any errors incurred while freeing the CDS Memory Block */ if (Status != CFE_SUCCESS) { - CFE_ES_SysLog_snprintf( - LogMessage, sizeof(LogMessage), - "CFE_ES:DeleteCDS-Failed to free CDS Mem Block (Handle=0x%08lX)(Stat=0x%08X)\n", - (unsigned long)RegRecPtr->BlockOffset, (unsigned int)Status); + snprintf(LogMessage, sizeof(LogMessage), + "Failed to free CDS Mem Block (Handle=0x%08lX)(Stat=0x%08X)\n", + (unsigned long)RegRecPtr->BlockOffset, (unsigned int)Status); } else { @@ -899,9 +898,8 @@ int32 CFE_ES_DeleteCDS(const char *CDSName, bool CalledByTblServices) if (Status != CFE_SUCCESS) { - CFE_ES_SysLog_snprintf(LogMessage, sizeof(LogMessage), - "CFE_ES:DeleteCDS-Failed to update CDS Registry (Stat=0x%08X)\n", - (unsigned int)Status); + snprintf(LogMessage, sizeof(LogMessage), "Failed to update CDS Registry (Stat=0x%08X)\n", + (unsigned int)Status); } } } @@ -922,7 +920,7 @@ int32 CFE_ES_DeleteCDS(const char *CDSName, bool CalledByTblServices) /* Output the message to syslog once the CDS registry resource is unlocked */ if (LogMessage[0] != 0) { - CFE_ES_SYSLOG_APPEND(LogMessage); + CFE_ES_WriteToSysLog("%s(): %s", __func__, LogMessage); } return Status; diff --git a/modules/es/fsw/src/cfe_es_cds_mempool.c b/modules/es/fsw/src/cfe_es_cds_mempool.c index 4f02386fc..ee2845058 100644 --- a/modules/es/fsw/src/cfe_es_cds_mempool.c +++ b/modules/es/fsw/src/cfe_es_cds_mempool.c @@ -201,14 +201,12 @@ int32 CFE_ES_CDSBlockWrite(CFE_ES_CDSHandle_t Handle, const void *DataToWrite) Status = CFE_ES_GenPoolGetBlockSize(&CDS->Pool, &BlockSize, CDSRegRecPtr->BlockOffset); if (Status != CFE_SUCCESS) { - CFE_ES_SysLog_snprintf(LogMessage, sizeof(LogMessage), - "CFE_ES:CDSBlkWrite-Invalid Handle or Block Descriptor.\n"); + snprintf(LogMessage, sizeof(LogMessage), "Invalid Handle or Block Descriptor.\n"); } else if (BlockSize <= sizeof(CFE_ES_CDS_BlockHeader_t) || BlockSize != CDSRegRecPtr->BlockSize) { - CFE_ES_SysLog_snprintf(LogMessage, sizeof(LogMessage), - "CFE_ES:CDSBlkWrite-Block size %lu invalid, expected %lu\n", - (unsigned long)BlockSize, (unsigned long)CDSRegRecPtr->BlockSize); + snprintf(LogMessage, sizeof(LogMessage), "Block size %lu invalid, expected %lu\n", (unsigned long)BlockSize, + (unsigned long)CDSRegRecPtr->BlockSize); Status = CFE_ES_CDS_INVALID_SIZE; } else @@ -227,20 +225,18 @@ int32 CFE_ES_CDSBlockWrite(CFE_ES_CDSHandle_t Handle, const void *DataToWrite) Status = CFE_ES_CDS_CacheFlush(&CDS->Cache); if (Status != CFE_SUCCESS) { - CFE_ES_SysLog_snprintf( - LogMessage, sizeof(LogMessage), - "CFE_ES:CDSBlkWrite-Err writing header data to CDS (Stat=0x%08x) @Offset=0x%08lx\n", - (unsigned int)CDS->Cache.AccessStatus, (unsigned long)CDSRegRecPtr->BlockOffset); + snprintf(LogMessage, sizeof(LogMessage), + "Err writing header data to CDS (Stat=0x%08x) @Offset=0x%08lx\n", + (unsigned int)CDS->Cache.AccessStatus, (unsigned long)CDSRegRecPtr->BlockOffset); } else { Status = CFE_PSP_WriteToCDS(DataToWrite, UserDataOffset, UserDataSize); if (Status != CFE_PSP_SUCCESS) { - CFE_ES_SysLog_snprintf( - LogMessage, sizeof(LogMessage), - "CFE_ES:CDSBlkWrite-Err writing user data to CDS (Stat=0x%08x) @Offset=0x%08lx\n", - (unsigned int)Status, (unsigned long)UserDataOffset); + snprintf(LogMessage, sizeof(LogMessage), + "Err writing user data to CDS (Stat=0x%08x) @Offset=0x%08lx\n", (unsigned int)Status, + (unsigned long)UserDataOffset); } } } @@ -255,7 +251,7 @@ int32 CFE_ES_CDSBlockWrite(CFE_ES_CDSHandle_t Handle, const void *DataToWrite) /* Do the actual syslog if something went wrong */ if (LogMessage[0] != 0) { - CFE_ES_SYSLOG_APPEND(LogMessage); + CFE_ES_WriteToSysLog("%s(): %s", __func__, LogMessage); } return Status; @@ -271,7 +267,6 @@ int32 CFE_ES_CDSBlockWrite(CFE_ES_CDSHandle_t Handle, const void *DataToWrite) int32 CFE_ES_CDSBlockRead(void *DataRead, CFE_ES_CDSHandle_t Handle) { CFE_ES_CDS_Instance_t *CDS = &CFE_ES_Global.CDSVars; - char LogMessage[CFE_ES_MAX_SYSLOG_MSG_SIZE]; int32 Status; uint32 CrcOfCDSData; size_t BlockSize; @@ -279,9 +274,6 @@ int32 CFE_ES_CDSBlockRead(void *DataRead, CFE_ES_CDSHandle_t Handle) size_t UserDataOffset; CFE_ES_CDS_RegRec_t * CDSRegRecPtr; - /* Validate the handle before doing anything */ - LogMessage[0] = 0; - CDSRegRecPtr = CFE_ES_LocateCDSBlockRecordByID(Handle); /* @@ -345,12 +337,6 @@ int32 CFE_ES_CDSBlockRead(void *DataRead, CFE_ES_CDSHandle_t Handle) CFE_ES_UnlockCDS(); - /* Do the actual syslog if something went wrong */ - if (LogMessage[0] != 0) - { - CFE_ES_SYSLOG_APPEND(LogMessage); - } - return Status; } diff --git a/modules/es/fsw/src/cfe_es_log.h b/modules/es/fsw/src/cfe_es_log.h index cac9267c7..6dfb7c55b 100644 --- a/modules/es/fsw/src/cfe_es_log.h +++ b/modules/es/fsw/src/cfe_es_log.h @@ -85,26 +85,6 @@ */ #define CFE_ES_SYSLOG_READ_BUFFER_SIZE (3 * CFE_ES_MAX_SYSLOG_MSG_SIZE) -/** - * \brief Self-synchronized macro to call CFE_ES_SysLogAppend_Unsync - * - * Calls CFE_ES_SysLogAppend_Unsync() with appropriate synchronization. - * It will acquire the shared data lock and release it after appending the log. - * - * This is implemented as a macro such that the "__func__" and "__LINE__" directives - * will reflect the actual place that the append was done, rather than where this - * wrapper was defined. - * - * \sa CFE_ES_SysLogAppend_Unsync() - */ -#define CFE_ES_SYSLOG_APPEND(LogString) \ - { \ - CFE_ES_LockSharedData(__func__, __LINE__); \ - CFE_ES_SysLogAppend_Unsync(LogString); \ - CFE_ES_UnlockSharedData(__func__, __LINE__); \ - OS_printf("%s", LogString); \ - } - /** * \brief Indicates no context information Error Logs * From 85bfba24dd2cf73d88cfe318526d6a8cca7dc167 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 16 Apr 2021 16:27:32 -0400 Subject: [PATCH 13/16] Fix #1338, ignore status of call to CFE_ES_CDS_CachePreload The function cannot fail in this context --- modules/es/fsw/src/cfe_es_cds.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/es/fsw/src/cfe_es_cds.c b/modules/es/fsw/src/cfe_es_cds.c index 9e6d58603..d5d45af4e 100644 --- a/modules/es/fsw/src/cfe_es_cds.c +++ b/modules/es/fsw/src/cfe_es_cds.c @@ -529,9 +529,11 @@ int32 CFE_ES_ClearCDS(void) size_t RemainSize; int32 Status; + Status = CFE_SUCCESS; + /* Clear the CDS to ensure everything is gone */ /* Create a block of zeros to write to the CDS */ - Status = CFE_ES_CDS_CachePreload(&CDS->Cache, NULL, 0, sizeof(CDS->Cache.Data.Zero)); + CFE_ES_CDS_CachePreload(&CDS->Cache, NULL, 0, sizeof(CDS->Cache.Data.Zero)); /* While there is space to write another block of zeros, then do so */ while (CDS->Cache.Offset < CDS->TotalSize) From 403df809eadaecb81e429d780c0a82b01bfe7afc Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 20 Apr 2021 10:10:53 -0400 Subject: [PATCH 14/16] Fix #1330, add unit test case for too many tokens --- modules/es/ut-coverage/es_UT.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/modules/es/ut-coverage/es_UT.c b/modules/es/ut-coverage/es_UT.c index 9910a2aa2..1a9da9ef1 100644 --- a/modules/es/ut-coverage/es_UT.c +++ b/modules/es/ut-coverage/es_UT.c @@ -1065,6 +1065,16 @@ void TestApps(void) UtAssert_NONZERO(UT_PrintfIsInHistory(UT_OSP_MESSAGES[UT_OSP_FILE_LINE_TOO_LONG])); UtAssert_NONZERO(UT_PrintfIsInHistory(UT_OSP_MESSAGES[UT_OSP_ES_APP_STARTUP_OPEN])); + /* Test starting an application where the startup script has extra tokens */ + ES_ResetUnitTest(); + strncpy(StartupScript, "A,B,C,D,E,F,G,H,I,J,K; !", sizeof(StartupScript) - 1); + StartupScript[sizeof(StartupScript) - 1] = '\0'; + NumBytes = strlen(StartupScript); + UT_SetReadBuffer(StartupScript, NumBytes); + CFE_ES_StartApplications(CFE_PSP_RST_TYPE_PROCESSOR, "ut_startup"); + UtAssert_NONZERO(UT_PrintfIsInHistory(UT_OSP_MESSAGES[UT_OSP_FILE_LINE_TOO_LONG])); + UtAssert_NONZERO(UT_PrintfIsInHistory(UT_OSP_MESSAGES[UT_OSP_ES_APP_STARTUP_OPEN])); + /* Create a valid startup script for subsequent tests */ strncpy(StartupScript, "CFE_LIB, /cf/apps/tst_lib.bundle, TST_LIB_Init, TST_LIB, 0, 0, 0x0, 1; " From d785db86c8b972357c5ff45789d4ce82b185f20d Mon Sep 17 00:00:00 2001 From: Alex Campbell Date: Thu, 22 Apr 2021 14:02:00 -0400 Subject: [PATCH 15/16] Fix #809, add ES Child Task Functional Test Fix #1291, fix spelling typo --- cmake/sample_defs/cpu1_cfe_es_startup.scr | 1 - modules/cfe_testcase/CMakeLists.txt | 1 + modules/cfe_testcase/src/cfe_test.c | 1 + modules/cfe_testcase/src/cfe_test.h | 3 +- modules/cfe_testcase/src/es_info_test.c | 4 +- modules/cfe_testcase/src/es_task_test.c | 115 ++++++++++++++++++++++ 6 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 modules/cfe_testcase/src/es_task_test.c diff --git a/cmake/sample_defs/cpu1_cfe_es_startup.scr b/cmake/sample_defs/cpu1_cfe_es_startup.scr index e48880e15..379f4bce4 100644 --- a/cmake/sample_defs/cpu1_cfe_es_startup.scr +++ b/cmake/sample_defs/cpu1_cfe_es_startup.scr @@ -31,4 +31,3 @@ CFE_APP, sch_lab, SCH_Lab_AppMain, SCH_LAB_APP, 80, 16384, 0x0, 0; ! 3. The filename field (2) no longer requires a fully-qualified filename; the path and extension ! may be omitted. If omitted, the standard virtual path (/cf) and a platform-specific default ! extension will be used, which is derived from the build system. - diff --git a/modules/cfe_testcase/CMakeLists.txt b/modules/cfe_testcase/CMakeLists.txt index 64f4180cf..106f28eba 100644 --- a/modules/cfe_testcase/CMakeLists.txt +++ b/modules/cfe_testcase/CMakeLists.txt @@ -3,6 +3,7 @@ add_cfe_app(cfe_testcase src/cfe_test.c src/es_info_test.c + src/es_task_test.c ) # register the dependency on cfe_assert diff --git a/modules/cfe_testcase/src/cfe_test.c b/modules/cfe_testcase/src/cfe_test.c index f4015320c..1cfafd301 100644 --- a/modules/cfe_testcase/src/cfe_test.c +++ b/modules/cfe_testcase/src/cfe_test.c @@ -51,6 +51,7 @@ void CFE_TestMain(void) * Register test cases in UtAssert */ ESInfoTestSetup(); + ESTaskTestSetup(); /* * Execute the tests diff --git a/modules/cfe_testcase/src/cfe_test.h b/modules/cfe_testcase/src/cfe_test.h index 87223e978..0eb9296bb 100644 --- a/modules/cfe_testcase/src/cfe_test.h +++ b/modules/cfe_testcase/src/cfe_test.h @@ -49,10 +49,11 @@ CFE_RESOURCEID_TO_ULONG(actual), #expect, CFE_RESOURCEID_TO_ULONG(expect)) /* Check if a Resource ID is Undefined */ -#define UtAssert_ResourceID_Undifeined(id) \ +#define UtAssert_ResourceID_Undefined(id) \ UtAssert_True(!CFE_RESOURCEID_TEST_DEFINED(id), "%s (%lu) not defined", #id, CFE_RESOURCEID_TO_ULONG(id)) void CFE_TestMain(void); void ESInfoTestSetup(void); +void ESTaskTestSetup(void); #endif /* CFE_TEST_H */ diff --git a/modules/cfe_testcase/src/es_info_test.c b/modules/cfe_testcase/src/es_info_test.c index f107b611c..f251d5325 100644 --- a/modules/cfe_testcase/src/es_info_test.c +++ b/modules/cfe_testcase/src/es_info_test.c @@ -122,7 +122,7 @@ void TestAppInfo(void) UtAssert_True(ESAppInfo.NumOfChildTasks > 0, "ES App Info -> Child Tasks = %d", (int)ESAppInfo.NumOfChildTasks); UtAssert_INT32_EQ(CFE_ES_GetAppIDByName(&AppIdByName, INVALID_APP_NAME), CFE_ES_ERR_NAME_NOT_FOUND); - UtAssert_ResourceID_Undifeined(AppIdByName); + UtAssert_ResourceID_Undefined(AppIdByName); UtAssert_INT32_EQ(CFE_ES_GetAppID(NULL), CFE_ES_BAD_ARGUMENT); UtAssert_INT32_EQ(CFE_ES_GetAppIDByName(NULL, TEST_EXPECTED_APP_NAME), CFE_ES_BAD_ARGUMENT); UtAssert_INT32_EQ(CFE_ES_GetAppName(AppNameBuf, CFE_ES_APPID_UNDEFINED, sizeof(AppNameBuf)), @@ -205,7 +205,7 @@ void TestLibInfo(void) UtAssert_INT32_EQ(LibInfo.ExceptionAction, 0); UtAssert_True(LibInfo.Priority == 0, "Lib Info -> Priority = %d", (int)LibInfo.Priority); - UtAssert_ResourceID_Undifeined(LibInfo.MainTaskId); + UtAssert_ResourceID_Undefined(LibInfo.MainTaskId); UtAssert_True(LibInfo.ExecutionCounter == 0, "Lib Info -> ExecutionCounter = %d", (int)LibInfo.ExecutionCounter); UtAssert_True(strlen(LibInfo.MainTaskName) == 0, "Lib Info -> Task Name = %s", LibInfo.MainTaskName); UtAssert_True(LibInfo.NumOfChildTasks == 0, "Lib Info -> Child Tasks = %d", (int)LibInfo.NumOfChildTasks); diff --git a/modules/cfe_testcase/src/es_task_test.c b/modules/cfe_testcase/src/es_task_test.c new file mode 100644 index 000000000..cfca4e61e --- /dev/null +++ b/modules/cfe_testcase/src/es_task_test.c @@ -0,0 +1,115 @@ +/************************************************************************* +** +** GSC-18128-1, "Core Flight Executive Version 6.7" +** +** Copyright (c) 2006-2019 United States Government as represented by +** the Administrator of the National Aeronautics and Space Administration. +** All Rights Reserved. +** +** Licensed under the Apache License, Version 2.0 (the "License"); +** you may not use this file except in compliance with the License. +** You may obtain a copy of the License at +** +** http://www.apache.org/licenses/LICENSE-2.0 +** +** Unless required by applicable law or agreed to in writing, software +** distributed under the License is distributed on an "AS IS" BASIS, +** WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +** See the License for the specific language governing permissions and +** limitations under the License. +** +** File: es_task_test.c +** +** Purpose: +** Functional test of basic ES Child Tasks APIs +** +** Demonstration of how to register and use the UT assert functions. +** +*************************************************************************/ + +/* + * Includes + */ + +#include "cfe_test.h" + +uint32 count = 0; + +void TaskFunction(void) +{ + while (count < 200) + { + count += 1; + OS_TaskDelay(100); + } + return; +} + +void TaskExitFunction(void) +{ + while (count < 200) + { + count += 1; + CFE_ES_ExitChildTask(); + } + return; +} + +void TestCreateChild(void) +{ + UtPrintf("Testing: CFE_ES_CreateChildTask, CFE_ES_GetTaskIDByName, CFE_ES_GetTaskName, CFE_ES_DeleteChildTask"); + + CFE_ES_TaskId_t TaskId; + const char * TaskName = "CHILD_TASK_1"; + CFE_ES_TaskId_t TaskIdByName; + char TaskNameBuf[OS_MAX_API_NAME + 4]; + CFE_ES_StackPointer_t StackPointer = CFE_ES_TASK_STACK_ALLOCATE; + size_t StackSize = CFE_PLATFORM_ES_PERF_CHILD_STACK_SIZE; + CFE_ES_TaskPriority_Atom_t Priority = CFE_PLATFORM_ES_PERF_CHILD_PRIORITY; + uint32 Flags = 0; + int countCopy; + + UtAssert_INT32_EQ(CFE_ES_CreateChildTask(&TaskId, TaskName, TaskFunction, StackPointer, StackSize, Priority, Flags), + CFE_SUCCESS); + + UtAssert_INT32_EQ(CFE_ES_GetTaskIDByName(&TaskIdByName, TaskName), CFE_SUCCESS); + UtAssert_ResourceID_EQ(TaskIdByName, TaskId); + + UtAssert_INT32_EQ(CFE_ES_GetTaskName(TaskNameBuf, TaskId, sizeof(TaskNameBuf)), CFE_SUCCESS); + UtAssert_StrCmp(TaskNameBuf, TaskName, "CFE_ES_GetTaskName() = %s", TaskNameBuf); + + OS_TaskDelay(500); + + countCopy = count; + + UtAssert_INT32_EQ(CFE_ES_DeleteChildTask(TaskId), CFE_SUCCESS); + + OS_TaskDelay(500); + + UtAssert_True(countCopy == count || countCopy == count + 1, "countCopy (%d) == count (%d)", countCopy, count); +} + +void TestExitChild(void) +{ + UtPrintf("Testing: CFE_ES_ExitChildTask"); + + CFE_ES_TaskId_t TaskId; + const char * TaskName = "CHILD_TASK_1"; + CFE_ES_StackPointer_t StackPointer = CFE_ES_TASK_STACK_ALLOCATE; + size_t StackSize = CFE_PLATFORM_ES_PERF_CHILD_STACK_SIZE; + CFE_ES_TaskPriority_Atom_t Priority = CFE_PLATFORM_ES_PERF_CHILD_PRIORITY; + uint32 Flags = 0; + count = 0; + + UtAssert_INT32_EQ( + CFE_ES_CreateChildTask(&TaskId, TaskName, TaskExitFunction, StackPointer, StackSize, Priority, Flags), + CFE_SUCCESS); + OS_TaskDelay(500); + UtAssert_INT32_EQ(count, 1); +} + +void ESTaskTestSetup(void) +{ + UtTest_Add(TestCreateChild, NULL, NULL, "Test Create Child"); + UtTest_Add(TestExitChild, NULL, NULL, "Test Exit Child"); +} From c9735e5d0e36345b42a62cfe9b7d967c9a346c5b Mon Sep 17 00:00:00 2001 From: "Gerardo E. Cruz-Ortiz" <59618057+astrogeco@users.noreply.github.com> Date: Wed, 28 Apr 2021 17:56:57 -0400 Subject: [PATCH 16/16] IC:2021-04-28, Bump to v6.8.0-rc1+dev540 --- README.md | 21 +++++++++++++++++++++ modules/core_api/fsw/inc/cfe_version.h | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 4c923550b..e077aaa54 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,27 @@ The detailed cFE user's guide can be viewed at and + + ### Development Build: v6.8.0-rc1+dev509 - Separates the list of CFE core interface modules (e.g. core_api) from the list of CFE core implementation modules (e.g. msg). This allows the content of core_api to be expanded to locally include any additional modules the user has added to cFE core via the `MISSION_CORE_MODULES` list. diff --git a/modules/core_api/fsw/inc/cfe_version.h b/modules/core_api/fsw/inc/cfe_version.h index 1909939db..0f02cf719 100644 --- a/modules/core_api/fsw/inc/cfe_version.h +++ b/modules/core_api/fsw/inc/cfe_version.h @@ -28,7 +28,7 @@ #define CFE_VERSION_H /* Development Build Macro Definitions */ -#define CFE_BUILD_NUMBER 509 /*!< Development Build: Number of commits since baseline */ +#define CFE_BUILD_NUMBER 540 /*!< 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 \ */