From e4374200a932fb42e82770e64c76d2675a1f4802 Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Thu, 28 Sep 2023 14:57:49 -0400 Subject: [PATCH 1/4] Fix #2449, crc calculation refactor Move the current CRC-16 algorithm to a separate source file and better structure the code to support future CRC algorithm alternatives. Improve documentation to better indicate what the current algorithm is and what to expect going forward. Also corrects for issues in the CRC functional test: - Use the standard check input and compare against the standard check value for CRC-16/ARC. - Change cases from MIR to a normal test case - given a specific algorithm with specific input, the return value should be the same. --- docs/cFE Application Developers Guide.md | 32 +++-- modules/cfe_testcase/src/es_misc_test.c | 44 ++++--- modules/core_api/fsw/inc/cfe_es.h | 9 +- .../core_api/fsw/inc/cfe_es_api_typedefs.h | 48 ++++++- modules/es/CMakeLists.txt | 1 + .../es/config/default_cfe_es_interface_cfg.h | 10 +- modules/es/fsw/src/cfe_es_api.c | 70 ++-------- modules/es/fsw/src/cfe_es_crc.c | 124 ++++++++++++++++++ modules/es/fsw/src/cfe_es_crc.h | 53 ++++++++ modules/es/fsw/src/cfe_es_module_all.h | 1 + 10 files changed, 288 insertions(+), 104 deletions(-) create mode 100644 modules/es/fsw/src/cfe_es_crc.c create mode 100644 modules/es/fsw/src/cfe_es_crc.h diff --git a/docs/cFE Application Developers Guide.md b/docs/cFE Application Developers Guide.md index ebf9e6f0c..083cb5685 100644 --- a/docs/cFE Application Developers Guide.md +++ b/docs/cFE Application Developers Guide.md @@ -1375,22 +1375,30 @@ uint32 CFE_ES_CalculateCRC(const void *DataPtr, size_t DataLength, uint32 InputC where DataPtr points to the first byte of an array of bytes that are to have the CRC calculated on, DataLength specifies the number of sequential bytes to include in the calculation, InputCRC is the initial value of the CRC and -TypeCRC identifies which of the standard CRC polynomials to be used. Currently, -there are the following types available: +TypeCRC identifies which of the standard CRC polynomials to be used. -``` -CFE_ES_CrcType_CRC_8 – an 8-bit additive checksum calculation that returns a 32-bit value -CFE_ES_CrcType_CRC_16 – a 16-bit additive checksum calculation that returns a 32-bit value -CFE_ES_CrcType_CRC_32 – a 32-bit additive checksum calculation that returns a 32-bit value -CFE_MISSION_ES_DEFAULT_CRC – the mission specified default CRC calculation (currently - this is set to CFE_ES_CrcType_CRC_16 in sample_mission_cfg.h) -``` +The set of available CRC algorithms for the TypeCRC parameter depends on the version +of CFE and compile-time options selected. Historically, CFE only has +implemented a single algorithm which is CRC-16/ARC: + + - Polynomial: 0x8005 + - Initial Value: 0x0000 + - Reflect In/Out: true + - Final XOR: 0x0000 + - Check Value: 0xbb3d + +See the definition of the `CFE_ES_CrcType_Enum_t` type for complete documentation of all +available values for this paramater and details of the algorithm used for each. Note that +this enumeration may be extended in future versions of CFE, as mission needs evolve. + +For application compatibility, the `CFE_MISSION_ES_DEFAULT_CRC` macro is defined as part of +the CFE ES API configuration, which maps to the recommended CRC algorithm for applications +to use. By default this is set to CRC-16/ARC, but it can be configured based on mission +preference and the set of available CRC algorithms. Unless there is a specific interface with a specified CRC calculation, applications -must use the CFE_MISSION_ES_DEFAULT_CRC type. +should use the `CFE_MISSION_ES_DEFAULT_CRC` value for TypeCRC when invoking this API. -Currently only CFE_ES_CrcType_CRC_16 is supported. CFE_ES_CrcType_CRC_8 and CFE_ES_CrcType_CRC_32 are yet -to be implemented. ## 5.11 File System Functions diff --git a/modules/cfe_testcase/src/es_misc_test.c b/modules/cfe_testcase/src/es_misc_test.c index 86a4b92e4..46df2ab4b 100644 --- a/modules/cfe_testcase/src/es_misc_test.c +++ b/modules/cfe_testcase/src/es_misc_test.c @@ -31,29 +31,43 @@ void TestCalculateCRC(void) { - const char *Data = "Random Stuff"; - uint32 inputCrc = 345353; - uint32 Result; + static const char CRC_CHECK_INPUT[] = "123456789"; + static const char CRC_OTHER_INPUT[] = "Random Stuff"; + + uint32 inputCrc = 345353; UtPrintf("Testing: CFE_ES_CalculateCRC"); - /* CRC is implementation specific, functional just checks that a result is produced and reports */ - UtAssert_VOIDCALL(Result = CFE_ES_CalculateCRC(Data, sizeof(Data), 0, CFE_MISSION_ES_DEFAULT_CRC)); - UtAssert_MIR("Confirm mission default CRC of \"%s\" is %lu", Data, (unsigned long)Result); + /* The CFE_ES_CrcType_NONE algorithm always returns 0. */ + UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(CRC_CHECK_INPUT, sizeof(CRC_CHECK_INPUT) - 1, 1, CFE_ES_CrcType_NONE), 0); + + /* Calling with an invalid type value should get aliased to NONE. */ + UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(CRC_CHECK_INPUT, sizeof(CRC_CHECK_INPUT) - 1, 1, CFE_ES_CrcType_MAX + 1), 0); + + /* The CFE_ES_CrcType_16_ARC algorithm uses an input value of 0 and the check value result is 0xbb3d */ + UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(CRC_CHECK_INPUT, sizeof(CRC_CHECK_INPUT) - 1, 0, CFE_ES_CrcType_16_ARC), + 0xBB3D); - UtAssert_VOIDCALL(Result = CFE_ES_CalculateCRC(Data, sizeof(Data), inputCrc, CFE_ES_CrcType_CRC_16)); - UtAssert_MIR("Confirm CRC16 of \"%s\" with input CRC of %lu is %lu", Data, (unsigned long)inputCrc, - (unsigned long)Result); + /* + * In this version of CFE, only CRC-16/ARC is defined, so the default must be the same. In a future version of CFE, + * if more than one option is available, this can go back to an "MIR" case, and the user must compare the result + * to the check value for what they have actully configured as the default CRC. + */ + UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(CRC_CHECK_INPUT, sizeof(CRC_CHECK_INPUT) - 1, 0, CFE_MISSION_ES_DEFAULT_CRC), + 0xBB3D); - UtAssert_VOIDCALL(Result = CFE_ES_CalculateCRC(Data, sizeof(Data), 0, CFE_ES_CrcType_CRC_8)); - UtAssert_MIR("Confirm CRC8 of \"%s\" is %lu", Data, (unsigned long)Result); + /* Compute a secondary string - the reference value obtained using 3rd party tool implementing same algorithm */ + UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(CRC_OTHER_INPUT, sizeof(CRC_OTHER_INPUT) - 1, 0, CFE_ES_CrcType_16_ARC), + 0x11E3); - UtAssert_VOIDCALL(Result = CFE_ES_CalculateCRC(Data, sizeof(Data), 0, CFE_ES_CrcType_CRC_32)); - UtAssert_MIR("Confirm CRC32 of \"%s\" is %lu", Data, (unsigned long)Result); + /* Test of nonzero initial value, this is used for checking CRC in across non-contiguous chunks or across multiple + * cycles */ + UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(CRC_CHECK_INPUT, sizeof(CRC_CHECK_INPUT) - 1, 345353, CFE_ES_CrcType_16_ARC), + 0xE493); /* NULL input or 0 size returns input crc */ - UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(NULL, sizeof(Data), inputCrc, CFE_ES_CrcType_CRC_16), inputCrc); - UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(Data, 0, inputCrc, CFE_ES_CrcType_CRC_16), inputCrc); + UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(NULL, sizeof(CRC_OTHER_INPUT), inputCrc, CFE_ES_CrcType_16_ARC), inputCrc); + UtAssert_UINT32_EQ(CFE_ES_CalculateCRC(CRC_OTHER_INPUT, 0, inputCrc, CFE_ES_CrcType_16_ARC), inputCrc); } void TestWriteToSysLog(void) diff --git a/modules/core_api/fsw/inc/cfe_es.h b/modules/core_api/fsw/inc/cfe_es.h index cac2c8f03..fc765ee68 100644 --- a/modules/core_api/fsw/inc/cfe_es.h +++ b/modules/core_api/fsw/inc/cfe_es.h @@ -998,14 +998,7 @@ CFE_Status_t CFE_ES_WriteToSysLog(const char *SpecStringPtr, ...) OS_PRINTF(1, 2 ** allows the user to calculate the CRC of non-contiguous blocks as ** a single value. Nominally, the user should set this value to zero. ** -** \param[in] TypeCRC One of the following CRC algorithm selections: -** \arg \c CFE_ES_CrcType_CRC_8 - (Not currently implemented) -** \arg \c CFE_ES_CrcType_CRC_16 - CRC-16/ARC
-** Polynomial: 0x8005
-** Initialization: 0x0000
-** Reflect Input/Output: true
-** XorOut: 0x0000 -** \arg \c CFE_ES_CrcType_CRC_32 - (not currently implemented) +** \param[in] TypeCRC One of the following CRC algorithm selections defined in CFE_ES_CrcType_Enum_t ** ** \return The result of the CRC calculation on the specified memory block. ** If the TypeCRC is unimplemented will return 0. diff --git a/modules/core_api/fsw/inc/cfe_es_api_typedefs.h b/modules/core_api/fsw/inc/cfe_es_api_typedefs.h index 3b1ec6758..9c94ec4e7 100644 --- a/modules/core_api/fsw/inc/cfe_es_api_typedefs.h +++ b/modules/core_api/fsw/inc/cfe_es_api_typedefs.h @@ -86,13 +86,53 @@ typedef void *CFE_ES_StackPointer_t; /* aka osal_stackptr_t in proposed OSAL cha /** * \brief Checksum/CRC algorithm identifiers * - * Currently only CFE_ES_CrcType_CRC_16 is supported. + * Currently only CFE_ES_CrcType_16_ARC is supported. + * + * All defined CRC algorithms should include a check value, which is the accepted + * result of computing the CRC across the fixed string "123456789" */ typedef enum CFE_ES_CrcType_Enum { - CFE_ES_CrcType_CRC_8 = 1, /**< \brief CRC ( 8 bit additive - returns 32 bit total) (Not currently implemented) */ - CFE_ES_CrcType_CRC_16 = 2, /**< \brief CRC (16 bit additive - returns 32 bit total) */ - CFE_ES_CrcType_CRC_32 = 3 /**< \brief CRC (32 bit additive - returns 32 bit total) (Not currently implemented) */ + /** + * \brief Reserved placeholder value + * + * No computation is performed, always returns 0 as a result. + */ + CFE_ES_CrcType_NONE = 0, + + /** + * \brief Implementation of CRC-16/ARC + * + * \par + * Polynomial: 0x8005
+ * Initialization: 0x0000
+ * Reflect Input/Output: true
+ * Check value: 0xbb3d
+ * XorOut: 0x0000
+ */ + CFE_ES_CrcType_16_ARC = 1, + + /** + * Placeholder for end of normal enumeration list + * This should reflect the number of algorithms defined. + */ + CFE_ES_CrcType_MAX = 2, + + /* + * Backward compatibility values. + * For compatibility with apps, these simplified symbols need to be defined, + * and they also must map to nonzero values. + */ + + /** CRC-16 historically implied CRC-16/ARC */ + CFE_ES_CrcType_CRC_16 = CFE_ES_CrcType_16_ARC, + + /**< CRC-8 historically defined but not implemented, value must not be 0 */ + CFE_ES_CrcType_CRC_8 = CFE_ES_CrcType_MAX + 1, + + /**< CRC-32 historically defined but not implemented, value must not be 0 */ + CFE_ES_CrcType_CRC_32 = CFE_ES_CrcType_MAX + 2, + } CFE_ES_CrcType_Enum_t; /** diff --git a/modules/es/CMakeLists.txt b/modules/es/CMakeLists.txt index 9d7d50824..d15a94431 100644 --- a/modules/es/CMakeLists.txt +++ b/modules/es/CMakeLists.txt @@ -13,6 +13,7 @@ set(es_SOURCES fsw/src/cfe_es_backgroundtask.c fsw/src/cfe_es_cds.c fsw/src/cfe_es_cds_mempool.c + fsw/src/cfe_es_crc.c fsw/src/cfe_es_dispatch.c fsw/src/cfe_es_erlog.c fsw/src/cfe_es_generic_pool.c diff --git a/modules/es/config/default_cfe_es_interface_cfg.h b/modules/es/config/default_cfe_es_interface_cfg.h index 04f833187..8189075d4 100644 --- a/modules/es/config/default_cfe_es_interface_cfg.h +++ b/modules/es/config/default_cfe_es_interface_cfg.h @@ -112,10 +112,10 @@ ** Table Image data integrity values. ** ** \par Limits -** Currently only CFE_ES_CrcType_CRC_16 is supported (see brief in CFE_ES_CrcType_Enum +** Currently only CFE_ES_CrcType_16_ARC is supported (see brief in CFE_ES_CrcType_Enum ** definition in cfe_es_api_typedefs.h) */ -#define CFE_MISSION_ES_DEFAULT_CRC CFE_ES_CrcType_CRC_16 +#define CFE_MISSION_ES_DEFAULT_CRC CFE_ES_CrcType_16_ARC /** ** \cfeescfg Maximum Length of Full CDS Name in messages @@ -142,9 +142,9 @@ /** \name Checksum/CRC algorithm identifiers */ -#define CFE_MISSION_ES_CRC_8 CFE_ES_CrcType_CRC_8 /* 1 */ -#define CFE_MISSION_ES_CRC_16 CFE_ES_CrcType_CRC_16 /* 2 */ -#define CFE_MISSION_ES_CRC_32 CFE_ES_CrcType_CRC_32 /* 3 */ +#define CFE_MISSION_ES_CRC_8 CFE_ES_CrcType_CRC_8 +#define CFE_MISSION_ES_CRC_16 CFE_ES_CrcType_CRC_16 +#define CFE_MISSION_ES_CRC_32 CFE_ES_CrcType_CRC_32 #endif diff --git a/modules/es/fsw/src/cfe_es_api.c b/modules/es/fsw/src/cfe_es_api.c index 2992581f9..6b460ec6b 100644 --- a/modules/es/fsw/src/cfe_es_api.c +++ b/modules/es/fsw/src/cfe_es_api.c @@ -1573,72 +1573,22 @@ CFE_Status_t CFE_ES_WriteToSysLog(const char *SpecStringPtr, ...) *-----------------------------------------------------------------*/ uint32 CFE_ES_CalculateCRC(const void *DataPtr, size_t DataLength, uint32 InputCRC, CFE_ES_CrcType_Enum_t TypeCRC) { - uint32 i; - int16 Index; - int16 Crc = 0; - const uint8 *BufPtr; - uint8 ByteValue; - - static const uint16 CrcTable[256] = { - - 0x0000, 0xC0C1, 0xC181, 0x0140, 0xC301, 0x03C0, 0x0280, 0xC241, 0xC601, 0x06C0, 0x0780, 0xC741, 0x0500, 0xC5C1, - 0xC481, 0x0440, 0xCC01, 0x0CC0, 0x0D80, 0xCD41, 0x0F00, 0xCFC1, 0xCE81, 0x0E40, 0x0A00, 0xCAC1, 0xCB81, 0x0B40, - 0xC901, 0x09C0, 0x0880, 0xC841, 0xD801, 0x18C0, 0x1980, 0xD941, 0x1B00, 0xDBC1, 0xDA81, 0x1A40, 0x1E00, 0xDEC1, - 0xDF81, 0x1F40, 0xDD01, 0x1DC0, 0x1C80, 0xDC41, 0x1400, 0xD4C1, 0xD581, 0x1540, 0xD701, 0x17C0, 0x1680, 0xD641, - 0xD201, 0x12C0, 0x1380, 0xD341, 0x1100, 0xD1C1, 0xD081, 0x1040, 0xF001, 0x30C0, 0x3180, 0xF141, 0x3300, 0xF3C1, - 0xF281, 0x3240, 0x3600, 0xF6C1, 0xF781, 0x3740, 0xF501, 0x35C0, 0x3480, 0xF441, 0x3C00, 0xFCC1, 0xFD81, 0x3D40, - 0xFF01, 0x3FC0, 0x3E80, 0xFE41, 0xFA01, 0x3AC0, 0x3B80, 0xFB41, 0x3900, 0xF9C1, 0xF881, 0x3840, 0x2800, 0xE8C1, - 0xE981, 0x2940, 0xEB01, 0x2BC0, 0x2A80, 0xEA41, 0xEE01, 0x2EC0, 0x2F80, 0xEF41, 0x2D00, 0xEDC1, 0xEC81, 0x2C40, - 0xE401, 0x24C0, 0x2580, 0xE541, 0x2700, 0xE7C1, 0xE681, 0x2640, 0x2200, 0xE2C1, 0xE381, 0x2340, 0xE101, 0x21C0, - 0x2080, 0xE041, 0xA001, 0x60C0, 0x6180, 0xA141, 0x6300, 0xA3C1, 0xA281, 0x6240, 0x6600, 0xA6C1, 0xA781, 0x6740, - 0xA501, 0x65C0, 0x6480, 0xA441, 0x6C00, 0xACC1, 0xAD81, 0x6D40, 0xAF01, 0x6FC0, 0x6E80, 0xAE41, 0xAA01, 0x6AC0, - 0x6B80, 0xAB41, 0x6900, 0xA9C1, 0xA881, 0x6840, 0x7800, 0xB8C1, 0xB981, 0x7940, 0xBB01, 0x7BC0, 0x7A80, 0xBA41, - 0xBE01, 0x7EC0, 0x7F80, 0xBF41, 0x7D00, 0xBDC1, 0xBC81, 0x7C40, 0xB401, 0x74C0, 0x7580, 0xB541, 0x7700, 0xB7C1, - 0xB681, 0x7640, 0x7200, 0xB2C1, 0xB381, 0x7340, 0xB101, 0x71C0, 0x7080, 0xB041, 0x5000, 0x90C1, 0x9181, 0x5140, - 0x9301, 0x53C0, 0x5280, 0x9241, 0x9601, 0x56C0, 0x5780, 0x9741, 0x5500, 0x95C1, 0x9481, 0x5440, 0x9C01, 0x5CC0, - 0x5D80, 0x9D41, 0x5F00, 0x9FC1, 0x9E81, 0x5E40, 0x5A00, 0x9AC1, 0x9B81, 0x5B40, 0x9901, 0x59C0, 0x5880, 0x9841, - 0x8801, 0x48C0, 0x4980, 0x8941, 0x4B00, 0x8BC1, 0x8A81, 0x4A40, 0x4E00, 0x8EC1, 0x8F81, 0x4F40, 0x8D01, 0x4DC0, - 0x4C80, 0x8C41, 0x4400, 0x84C1, 0x8581, 0x4540, 0x8701, 0x47C0, 0x4680, 0x8641, 0x8201, 0x42C0, 0x4380, 0x8341, - 0x4100, 0x81C1, 0x8081, 0x4040 - - }; + const CFE_ES_ComputeCRC_Params_t *CrcParams; + uint32 CrcResult; + /* Protect against bad pointer input - historical behavior is to just return the input */ if (DataPtr == NULL || DataLength == 0) { - return InputCRC; + CrcResult = InputCRC; } - - switch (TypeCRC) + else { - case CFE_ES_CrcType_CRC_32: - CFE_ES_WriteToSysLog("%s: Calculate CRC32 not Implemented\n", __func__); - break; - - case CFE_ES_CrcType_CRC_16: - Crc = (int16)(0xFFFF & InputCRC); - BufPtr = (const uint8 *)DataPtr; - - for (i = 0; i < DataLength; i++, BufPtr++) - { - /* - * It is assumed that the supplied buffer is in a - * directly-accessible memory space that does not - * require special logic to access - */ - ByteValue = *BufPtr; - Index = ((Crc ^ ByteValue) & 0x00FF); - Crc = ((Crc >> 8) & 0x00FF) ^ CrcTable[Index]; - } - break; - - case CFE_ES_CrcType_CRC_8: - CFE_ES_WriteToSysLog("%s: Calculate CRC8 not Implemented\n", __func__); - break; - - default: - break; + /* This always returns a valid object, even if it is not implemented */ + CrcParams = CFE_ES_ComputeCRC_GetParams(TypeCRC); + CrcResult = CrcParams->Algorithm(DataPtr, DataLength, InputCRC); } - return Crc; + + return CrcResult; } /*---------------------------------------------------------------- diff --git a/modules/es/fsw/src/cfe_es_crc.c b/modules/es/fsw/src/cfe_es_crc.c new file mode 100644 index 000000000..ff868b5f0 --- /dev/null +++ b/modules/es/fsw/src/cfe_es_crc.c @@ -0,0 +1,124 @@ +/************************************************************************ + * NASA Docket No. GSC-18,719-1, and identified as “core Flight System: Bootes” + * + * Copyright (c) 2020 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 + * + * Defines underlying algorithm(s) and parameters for CRC calculation + * + * References: + * Flight Software Branch C Coding Standard Version 1.0a + * cFE Flight Software Application Developers Guide + * + * Notes: + * + */ + +#include "cfe_es_crc.h" + +#include + +uint32 CFE_ES_ComputeCRC_Algo_NONE(const void *DataPtr, size_t DataLength, uint32 InputCRC) +{ + /* This is a placeholder algorithm that always returns 0, no matter what the input was */ + return 0; +} + +uint32 CFE_ES_ComputeCRC_Algo_16_ARC(const void *DataPtr, size_t DataLength, uint32 InputCRC) +{ + size_t i; + uint16 Index; + uint16 Crc; + const uint8 *BufPtr; + + static const uint16 CrcTable[256] = { + + 0x0000, 0xC0C1, 0xC181, 0x0140, 0xC301, 0x03C0, 0x0280, 0xC241, 0xC601, 0x06C0, 0x0780, 0xC741, 0x0500, 0xC5C1, + 0xC481, 0x0440, 0xCC01, 0x0CC0, 0x0D80, 0xCD41, 0x0F00, 0xCFC1, 0xCE81, 0x0E40, 0x0A00, 0xCAC1, 0xCB81, 0x0B40, + 0xC901, 0x09C0, 0x0880, 0xC841, 0xD801, 0x18C0, 0x1980, 0xD941, 0x1B00, 0xDBC1, 0xDA81, 0x1A40, 0x1E00, 0xDEC1, + 0xDF81, 0x1F40, 0xDD01, 0x1DC0, 0x1C80, 0xDC41, 0x1400, 0xD4C1, 0xD581, 0x1540, 0xD701, 0x17C0, 0x1680, 0xD641, + 0xD201, 0x12C0, 0x1380, 0xD341, 0x1100, 0xD1C1, 0xD081, 0x1040, 0xF001, 0x30C0, 0x3180, 0xF141, 0x3300, 0xF3C1, + 0xF281, 0x3240, 0x3600, 0xF6C1, 0xF781, 0x3740, 0xF501, 0x35C0, 0x3480, 0xF441, 0x3C00, 0xFCC1, 0xFD81, 0x3D40, + 0xFF01, 0x3FC0, 0x3E80, 0xFE41, 0xFA01, 0x3AC0, 0x3B80, 0xFB41, 0x3900, 0xF9C1, 0xF881, 0x3840, 0x2800, 0xE8C1, + 0xE981, 0x2940, 0xEB01, 0x2BC0, 0x2A80, 0xEA41, 0xEE01, 0x2EC0, 0x2F80, 0xEF41, 0x2D00, 0xEDC1, 0xEC81, 0x2C40, + 0xE401, 0x24C0, 0x2580, 0xE541, 0x2700, 0xE7C1, 0xE681, 0x2640, 0x2200, 0xE2C1, 0xE381, 0x2340, 0xE101, 0x21C0, + 0x2080, 0xE041, 0xA001, 0x60C0, 0x6180, 0xA141, 0x6300, 0xA3C1, 0xA281, 0x6240, 0x6600, 0xA6C1, 0xA781, 0x6740, + 0xA501, 0x65C0, 0x6480, 0xA441, 0x6C00, 0xACC1, 0xAD81, 0x6D40, 0xAF01, 0x6FC0, 0x6E80, 0xAE41, 0xAA01, 0x6AC0, + 0x6B80, 0xAB41, 0x6900, 0xA9C1, 0xA881, 0x6840, 0x7800, 0xB8C1, 0xB981, 0x7940, 0xBB01, 0x7BC0, 0x7A80, 0xBA41, + 0xBE01, 0x7EC0, 0x7F80, 0xBF41, 0x7D00, 0xBDC1, 0xBC81, 0x7C40, 0xB401, 0x74C0, 0x7580, 0xB541, 0x7700, 0xB7C1, + 0xB681, 0x7640, 0x7200, 0xB2C1, 0xB381, 0x7340, 0xB101, 0x71C0, 0x7080, 0xB041, 0x5000, 0x90C1, 0x9181, 0x5140, + 0x9301, 0x53C0, 0x5280, 0x9241, 0x9601, 0x56C0, 0x5780, 0x9741, 0x5500, 0x95C1, 0x9481, 0x5440, 0x9C01, 0x5CC0, + 0x5D80, 0x9D41, 0x5F00, 0x9FC1, 0x9E81, 0x5E40, 0x5A00, 0x9AC1, 0x9B81, 0x5B40, 0x9901, 0x59C0, 0x5880, 0x9841, + 0x8801, 0x48C0, 0x4980, 0x8941, 0x4B00, 0x8BC1, 0x8A81, 0x4A40, 0x4E00, 0x8EC1, 0x8F81, 0x4F40, 0x8D01, 0x4DC0, + 0x4C80, 0x8C41, 0x4400, 0x84C1, 0x8581, 0x4540, 0x8701, 0x47C0, 0x4680, 0x8641, 0x8201, 0x42C0, 0x4380, 0x8341, + 0x4100, 0x81C1, 0x8081, 0x4040 + + }; + + i = DataLength; + BufPtr = (const uint8 *)DataPtr; + Crc = InputCRC; + + while (i > 0) + { + /* + * It is assumed that the supplied buffer is in a + * directly-accessible memory space that does not + * require special logic to access + */ + Index = *BufPtr; + Index = ((Crc ^ Index) & 0x00FF); + Crc = ((Crc >> 8) & 0x00FF) ^ CrcTable[Index]; + + --i; + ++BufPtr; + } + + return Crc; +} + +CFE_ES_ComputeCRC_Params_t *CFE_ES_ComputeCRC_GetParams(CFE_ES_CrcType_Enum_t CrcType) +{ + CFE_ES_ComputeCRC_Params_t *ParamPtr; + + /* The "NONE" type must be always defined, it is used as a catch-all */ + static CFE_ES_ComputeCRC_Params_t CRC_PARAM_NONE = { + .InitialValue = 0, .FinalXOR = 0, .Algorithm = CFE_ES_ComputeCRC_Algo_NONE}; + + /* Lookup table for types that are implemented in this version of CFE */ + static CFE_ES_ComputeCRC_Params_t CRC_PARAM_TABLE[CFE_ES_CrcType_MAX] = { + [CFE_ES_CrcType_16_ARC] = {0, 0, CFE_ES_ComputeCRC_Algo_16_ARC}, + }; + + if (CrcType < CFE_ES_CrcType_MAX) + { + ParamPtr = &CRC_PARAM_TABLE[CrcType]; + } + else + { + ParamPtr = NULL; + } + + /* Catch-all: Fall back to NONE if not implemented */ + if (ParamPtr == NULL || ParamPtr->Algorithm == NULL) + { + ParamPtr = &CRC_PARAM_NONE; + } + + return ParamPtr; +} diff --git a/modules/es/fsw/src/cfe_es_crc.h b/modules/es/fsw/src/cfe_es_crc.h new file mode 100644 index 000000000..923776f24 --- /dev/null +++ b/modules/es/fsw/src/cfe_es_crc.h @@ -0,0 +1,53 @@ +/************************************************************************ + * NASA Docket No. GSC-18,719-1, and identified as “core Flight System: Bootes” + * + * Copyright (c) 2020 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 + * Implemention of the CRC algorithm(s) for CFE ES + * + * Notes: + * Historically only CRC-16/ARC is defined, but this can be expanded. + * + */ + +#ifndef CFE_ES_CRC_H +#define CFE_ES_CRC_H + +/* +** Include Files +*/ +#include "common_types.h" +#include "cfe_es.h" + +typedef uint32 (*const CFE_ES_ComputeCRC_Algo_t)(const void *DataPtr, size_t DataLength, uint32 InputCRC); + +typedef const struct CFE_ES_ComputeCRC_Params +{ + uint32 InitialValue; + uint32 FinalXOR; + + /** + * Function that implments the processing for this algorithm + */ + CFE_ES_ComputeCRC_Algo_t Algorithm; + +} CFE_ES_ComputeCRC_Params_t; + +CFE_ES_ComputeCRC_Params_t *CFE_ES_ComputeCRC_GetParams(CFE_ES_CrcType_Enum_t CrcType); + +#endif /* CFE_ES_CRC_H */ diff --git a/modules/es/fsw/src/cfe_es_module_all.h b/modules/es/fsw/src/cfe_es_module_all.h index 9bc4d997d..783e0f7cd 100644 --- a/modules/es/fsw/src/cfe_es_module_all.h +++ b/modules/es/fsw/src/cfe_es_module_all.h @@ -41,6 +41,7 @@ #include "cfe_es_core_internal.h" #include "cfe_es_apps.h" #include "cfe_es_cds.h" +#include "cfe_es_crc.h" #include "cfe_es_perf.h" #include "cfe_es_generic_pool.h" #include "cfe_es_mempool.h" From 27e6b24f1e57e5e9d67f7fbbbeeecf1491a43e85 Mon Sep 17 00:00:00 2001 From: jdfiguer Date: Wed, 6 Sep 2023 14:54:04 -0400 Subject: [PATCH 2/4] Fix #2436, Adds an empty string or null pointer check for pipe creation --- modules/cfe_testcase/src/sb_pipe_mang_test.c | 1 + modules/sb/fsw/src/cfe_sb_api.c | 2 +- modules/sb/ut-coverage/sb_UT.c | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/modules/cfe_testcase/src/sb_pipe_mang_test.c b/modules/cfe_testcase/src/sb_pipe_mang_test.c index 431ac8eed..0fe7b5f2c 100644 --- a/modules/cfe_testcase/src/sb_pipe_mang_test.c +++ b/modules/cfe_testcase/src/sb_pipe_mang_test.c @@ -47,6 +47,7 @@ void TestPipeCreate(void) UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId1, OS_QUEUE_MAX_DEPTH + 5, PipeName), CFE_SB_BAD_ARGUMENT); UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId1, 0, PipeName), CFE_SB_BAD_ARGUMENT); UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId1, PipeDepth, NULL), CFE_SB_PIPE_CR_ERR); + UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId1, PipeDepth, ""), CFE_SB_BAD_ARGUMENT); } void TestPipeCreateMax(void) diff --git a/modules/sb/fsw/src/cfe_sb_api.c b/modules/sb/fsw/src/cfe_sb_api.c index 1b9574260..b4747f452 100644 --- a/modules/sb/fsw/src/cfe_sb_api.c +++ b/modules/sb/fsw/src/cfe_sb_api.c @@ -123,7 +123,7 @@ CFE_Status_t CFE_SB_CreatePipe(CFE_SB_PipeId_t *PipeIdPtr, uint16 Depth, const c CFE_ES_GetTaskID(&TskId); /* check input parameters */ - if ((PipeIdPtr == NULL) || (Depth > OS_QUEUE_MAX_DEPTH) || (Depth == 0)) + if ((PipeIdPtr == NULL) || (Depth > OS_QUEUE_MAX_DEPTH) || (Depth == 0) || (PipeName != NULL && PipeName[0] == '\0')) { PendingEventId = CFE_SB_CR_PIPE_BAD_ARG_EID; Status = CFE_SB_BAD_ARGUMENT; diff --git a/modules/sb/ut-coverage/sb_UT.c b/modules/sb/ut-coverage/sb_UT.c index 78c89bab5..fa002161b 100644 --- a/modules/sb/ut-coverage/sb_UT.c +++ b/modules/sb/ut-coverage/sb_UT.c @@ -1645,6 +1645,7 @@ void Test_CreatePipe_API(void) SB_UT_ADD_SUBTEST(Test_CreatePipe_InvalPipeDepth); SB_UT_ADD_SUBTEST(Test_CreatePipe_MaxPipes); SB_UT_ADD_SUBTEST(Test_CreatePipe_SamePipeName); + SB_UT_ADD_SUBTEST(Test_CreatePipe_EmptyPipeName); } /* @@ -1798,6 +1799,21 @@ void Test_CreatePipe_SamePipeName(void) CFE_UtAssert_TEARDOWN(CFE_SB_DeletePipe(PipeId)); } +/* +** Test create pipe response to empty pipe name +*/ +void Test_CreatePipe_EmptyPipeName(void) +{ + CFE_SB_PipeId_t PipeId = SB_UT_PIPEID_0; + uint16 PipeDepth = 1; + char PipeName[] = ""; + + /* Call to CFE_SB_CreatePipe with empty PipeName should fail */ + UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId, PipeDepth, PipeName), CFE_SB_BAD_ARGUMENT); + + UtAssert_INT32_EQ(CFE_SB_Global.HKTlmMsg.Payload.CreatePipeErrorCounter, 1); +} + /* ** Function for calling SB delete pipe API test functions */ From f5ce0f68535486793fe610f4130f965227776610 Mon Sep 17 00:00:00 2001 From: Dylan Date: Thu, 5 Oct 2023 14:58:38 -0400 Subject: [PATCH 3/4] Updating documentation and version numbers for v7.0.0-rc4+dev395 --- CHANGELOG.md | 5 +++++ modules/core_api/fsw/inc/cfe_version.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad62c6340..e450053d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## Development Build:v7.0.0-rc4+dev395 +- Adds an empty string or null pointer check for pipe creation +- crc calculation refactor +- See and + ## Development Build: v7.0.0-rc4+dev389 - Adds a cast to the negation of unsigned expression - See diff --git a/modules/core_api/fsw/inc/cfe_version.h b/modules/core_api/fsw/inc/cfe_version.h index ee749b969..ec98c3c4b 100644 --- a/modules/core_api/fsw/inc/cfe_version.h +++ b/modules/core_api/fsw/inc/cfe_version.h @@ -26,7 +26,7 @@ #define CFE_VERSION_H /* Development Build Macro Definitions */ -#define CFE_BUILD_NUMBER 389 /**< @brief Development: Number of development git commits since CFE_BUILD_BASELINE */ +#define CFE_BUILD_NUMBER 395 /**< @brief Development: Number of development git commits since CFE_BUILD_BASELINE */ #define CFE_BUILD_BASELINE "v7.0.0-rc4" /**< @brief Development: Reference git tag for build number */ /* See \ref cfsversions for definitions */ From 58551ac4cf5dda2abbf36916a3e9a874615b6c0e Mon Sep 17 00:00:00 2001 From: jdfiguer Date: Wed, 6 Sep 2023 14:54:04 -0400 Subject: [PATCH 4/4] Fix #2436, Adds an empty string or null pointer check for pipe creation --- modules/cfe_testcase/src/sb_pipe_mang_test.c | 1 + modules/sb/fsw/src/cfe_sb_api.c | 2 +- modules/sb/ut-coverage/sb_UT.c | 36 ++++++++++++++++++++ modules/sb/ut-coverage/sb_UT.h | 15 ++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) diff --git a/modules/cfe_testcase/src/sb_pipe_mang_test.c b/modules/cfe_testcase/src/sb_pipe_mang_test.c index 431ac8eed..0fe7b5f2c 100644 --- a/modules/cfe_testcase/src/sb_pipe_mang_test.c +++ b/modules/cfe_testcase/src/sb_pipe_mang_test.c @@ -47,6 +47,7 @@ void TestPipeCreate(void) UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId1, OS_QUEUE_MAX_DEPTH + 5, PipeName), CFE_SB_BAD_ARGUMENT); UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId1, 0, PipeName), CFE_SB_BAD_ARGUMENT); UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId1, PipeDepth, NULL), CFE_SB_PIPE_CR_ERR); + UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId1, PipeDepth, ""), CFE_SB_BAD_ARGUMENT); } void TestPipeCreateMax(void) diff --git a/modules/sb/fsw/src/cfe_sb_api.c b/modules/sb/fsw/src/cfe_sb_api.c index 1b9574260..b4747f452 100644 --- a/modules/sb/fsw/src/cfe_sb_api.c +++ b/modules/sb/fsw/src/cfe_sb_api.c @@ -123,7 +123,7 @@ CFE_Status_t CFE_SB_CreatePipe(CFE_SB_PipeId_t *PipeIdPtr, uint16 Depth, const c CFE_ES_GetTaskID(&TskId); /* check input parameters */ - if ((PipeIdPtr == NULL) || (Depth > OS_QUEUE_MAX_DEPTH) || (Depth == 0)) + if ((PipeIdPtr == NULL) || (Depth > OS_QUEUE_MAX_DEPTH) || (Depth == 0) || (PipeName != NULL && PipeName[0] == '\0')) { PendingEventId = CFE_SB_CR_PIPE_BAD_ARG_EID; Status = CFE_SB_BAD_ARGUMENT; diff --git a/modules/sb/ut-coverage/sb_UT.c b/modules/sb/ut-coverage/sb_UT.c index 78c89bab5..bd065742d 100644 --- a/modules/sb/ut-coverage/sb_UT.c +++ b/modules/sb/ut-coverage/sb_UT.c @@ -1645,6 +1645,8 @@ void Test_CreatePipe_API(void) SB_UT_ADD_SUBTEST(Test_CreatePipe_InvalPipeDepth); SB_UT_ADD_SUBTEST(Test_CreatePipe_MaxPipes); SB_UT_ADD_SUBTEST(Test_CreatePipe_SamePipeName); + SB_UT_ADD_SUBTEST(Test_CreatePipe_EmptyPipeName); + SB_UT_ADD_SUBTEST(Test_CreatePipe_PipeName_NullPtr); } /* @@ -1798,6 +1800,40 @@ void Test_CreatePipe_SamePipeName(void) CFE_UtAssert_TEARDOWN(CFE_SB_DeletePipe(PipeId)); } +/* +** Test create pipe response to empty pipe name +*/ +void Test_CreatePipe_EmptyPipeName(void) +{ + CFE_SB_PipeId_t PipeId = SB_UT_PIPEID_0; + uint16 PipeDepth = 1; + char PipeName[] = ""; + + /* Call to CFE_SB_CreatePipe with empty PipeName should fail */ + UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId, PipeDepth, PipeName), CFE_SB_BAD_ARGUMENT); + + UtAssert_INT32_EQ(CFE_SB_Global.HKTlmMsg.Payload.CreatePipeErrorCounter, 1); +} + +/* +** Test create pipe response to a null pipe name pointer. +*/ +void Test_CreatePipe_PipeName_NullPtr(void) +{ + CFE_SB_PipeId_t PipeId = SB_UT_PIPEID_0; + uint16 PipeDepth = 1; + + /* To fail the pipe create, force the OS_QueueCreate call to return some + * type of error code. + */ + UT_SetDeferredRetcode(UT_KEY(OS_QueueCreate), 1, OS_ERR_NO_FREE_IDS); + + /* Call to CFE_SB_CreatePipe with empty PipeName should fail */ + UtAssert_INT32_EQ(CFE_SB_CreatePipe(&PipeId, PipeDepth, NULL), CFE_SB_PIPE_CR_ERR); + + UtAssert_INT32_EQ(CFE_SB_Global.HKTlmMsg.Payload.CreatePipeErrorCounter, 1); +} + /* ** Function for calling SB delete pipe API test functions */ diff --git a/modules/sb/ut-coverage/sb_UT.h b/modules/sb/ut-coverage/sb_UT.h index 6a4f59270..45e15fcf3 100644 --- a/modules/sb/ut-coverage/sb_UT.h +++ b/modules/sb/ut-coverage/sb_UT.h @@ -1198,6 +1198,21 @@ void Test_CreatePipe_InvalPipeDepth(void); ******************************************************************************/ void Test_CreatePipe_EmptyPipeName(void); +/*****************************************************************************/ +/** +** \brief Test create pipe response to a NULL pipe name +** +** \par Description +** This function tests the create pipe response to a null pipe name pointer. +** +** \par Assumptions, External Events, and Notes: +** None +** +** \returns +** This function does not return a value. +******************************************************************************/ +void Test_CreatePipe_PipeName_NullPtr(void); + /*****************************************************************************/ /** ** \brief Test create pipe response to a pipe name longer than allowed