From 6b453be2de988bac473ebb84315f6c058b63d7b5 Mon Sep 17 00:00:00 2001 From: Jacob Hageman Date: Wed, 28 Oct 2020 17:02:24 -0400 Subject: [PATCH] Fix #973, CFE_MSG_Init clear option removed --- fsw/cfe-core/src/inc/cfe_msg_api.h | 9 ++- fsw/cfe-core/src/sb/cfe_sb_util.c | 2 +- fsw/cfe-core/ut-stubs/ut_msg_stubs.c | 3 +- modules/msg/src/cfe_msg_init.c | 11 ++-- .../test_cfe_msg_ccsdsext.c | 59 +++++++++++------- .../unit-test-coverage/test_cfe_msg_init.c | 60 +++++++------------ 6 files changed, 68 insertions(+), 76 deletions(-) diff --git a/fsw/cfe-core/src/inc/cfe_msg_api.h b/fsw/cfe-core/src/inc/cfe_msg_api.h index a2a5d1e92..3dfe24b4f 100644 --- a/fsw/cfe-core/src/inc/cfe_msg_api.h +++ b/fsw/cfe-core/src/inc/cfe_msg_api.h @@ -44,20 +44,19 @@ * \brief Initialize a message * * \par Description - * This routine initialize a message. If Clear is true the - * message is cleard and constant header defaults are set. - * The bits from MsgId and message size are always set. + * This routine initialize a message. The entire message is + * set to zero (based on size), defaults are set, then the + * size and bits from MsgId are set. * * \param[in, out] MsgPtr A pointer to the buffer that contains the message. * \param[in] MsgId MsgId that corresponds to message * \param[in] Size Total size of the mesage (used to set length field) - * \param[in] Clear Boolean to clear and set defaults * * \return Execution status, see \ref CFEReturnCodes * \retval #CFE_SUCCESS \copybrief CFE_SUCCESS * \retval #CFE_MSG_BAD_ARGUMENT \copybrief CFE_MSG_BAD_ARGUMENT */ -CFE_Status_t CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size_t Size, bool Clear); +CFE_Status_t CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size_t Size); /*****************************************************************************/ /** diff --git a/fsw/cfe-core/src/sb/cfe_sb_util.c b/fsw/cfe-core/src/sb/cfe_sb_util.c index 1cc61fbc5..6086f22b1 100644 --- a/fsw/cfe-core/src/sb/cfe_sb_util.c +++ b/fsw/cfe-core/src/sb/cfe_sb_util.c @@ -51,7 +51,7 @@ void CFE_SB_InitMsg(void *MsgPtr, bool Clear ) { - CFE_MSG_Init((CFE_MSG_Message_t *)MsgPtr, MsgId, Length, Clear); + CFE_MSG_Init((CFE_MSG_Message_t *)MsgPtr, MsgId, Length); } /* end CFE_SB_InitMsg */ diff --git a/fsw/cfe-core/ut-stubs/ut_msg_stubs.c b/fsw/cfe-core/ut-stubs/ut_msg_stubs.c index 77e14b03e..c7d4cc8a0 100644 --- a/fsw/cfe-core/ut-stubs/ut_msg_stubs.c +++ b/fsw/cfe-core/ut-stubs/ut_msg_stubs.c @@ -414,12 +414,11 @@ int32 CFE_MSG_GetTypeFromMsgId(CFE_SB_MsgId_t MsgId, CFE_MSG_Type_t *Type) * Stub implementation of CFE_MSG_Init * ----------------------------------------------------------- */ -int32 CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size_t Size, bool Clear) +int32 CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size_t Size) { UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_MSG_Init), MsgPtr); UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_MSG_Init), MsgId); UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_MSG_Init), Size); - UT_Stub_RegisterContextGenericArg(UT_KEY(CFE_MSG_Init), Clear); int32 status; diff --git a/modules/msg/src/cfe_msg_init.c b/modules/msg/src/cfe_msg_init.c index b64e2d0f6..664b8c3dc 100644 --- a/modules/msg/src/cfe_msg_init.c +++ b/modules/msg/src/cfe_msg_init.c @@ -29,7 +29,7 @@ /****************************************************************************** * Top level message initialization - See API and header file for details */ -int32 CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size_t Size, bool Clear) +int32 CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size_t Size) { int32 status; @@ -39,12 +39,9 @@ int32 CFE_MSG_Init(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId, CFE_MSG_Size return CFE_MSG_BAD_ARGUMENT; } - /* Clear and set defaults if request */ - if (Clear) - { - memset(MsgPtr, 0, Size); - CFE_MSG_InitDefaultHdr(MsgPtr); - } + /* Clear and set defaults */ + memset(MsgPtr, 0, Size); + CFE_MSG_InitDefaultHdr(MsgPtr); /* Set values input */ status = CFE_MSG_SetMsgId(MsgPtr, MsgId); diff --git a/modules/msg/unit-test-coverage/test_cfe_msg_ccsdsext.c b/modules/msg/unit-test-coverage/test_cfe_msg_ccsdsext.c index f8bfe369c..05ee2438f 100644 --- a/modules/msg/unit-test-coverage/test_cfe_msg_ccsdsext.c +++ b/modules/msg/unit-test-coverage/test_cfe_msg_ccsdsext.c @@ -49,29 +49,29 @@ /* Extended header initialization specific coverage */ void Test_MSG_Init_Ext(void) { - CFE_MSG_Message_t msg; - CFE_SB_MsgId_Atom_t msgidval_exp; - CFE_MSG_HeaderVersion_t hdrver; - CFE_MSG_Subsystem_t subsys; - CFE_MSG_EDSVersion_t edsver; - CFE_MSG_System_t system; - CFE_MSG_Endian_t endian; - bool is_v1; - int sc_id = 0xab; - - /* Get msgid version by checking if msgid sets header version */ + CFE_MSG_Message_t msg; + CFE_SB_MsgId_Atom_t msgidval_exp; + CFE_MSG_Subsystem_t subsys; + CFE_MSG_EDSVersion_t edsver; + CFE_MSG_System_t system; + CFE_MSG_Endian_t endian; + bool hassec; + bool is_v1; + int sc_id = 0xab; + + /* Get msgid version by checking if msgid sets "has secondary" field*/ memset(&msg, 0xFF, sizeof(msg)); ASSERT_EQ(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(0)), CFE_SUCCESS); - ASSERT_EQ(CFE_MSG_GetHeaderVersion(&msg, &hdrver), CFE_SUCCESS); - is_v1 = (hdrver == 0); + ASSERT_EQ(CFE_MSG_GetHasSecondaryHeader(&msg, &hassec), CFE_SUCCESS); + is_v1 = !hassec; /* Set up return */ - UT_SetDeferredRetcode(UT_KEY(CFE_PSP_GetSpacecraftId), 1, sc_id); + UT_SetForceFail(UT_KEY(CFE_PSP_GetSpacecraftId), sc_id); - UtPrintf("Set to all F's, msgid value = 0, and run with clearing"); + UtPrintf("Set to all F's, msgid value = 0"); memset(&msg, 0xFF, sizeof(msg)); msgidval_exp = 0; - ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg), true), CFE_SUCCESS); + ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg)), CFE_SUCCESS); Test_MSG_PrintMsg(&msg, 0); /* Default EDS version check */ @@ -100,21 +100,38 @@ void Test_MSG_Init_Ext(void) /* Confirm the rest of the fields not already explicitly checked */ ASSERT_EQ(Test_MSG_Ext_NotZero(&msg) & ~(MSG_EDSVER_FLAG | MSG_ENDIAN_FLAG | MSG_SUBSYS_FLAG | MSG_SYSTEM_FLAG), 0); - UtPrintf("Set to all 0, max msgid value, and run without clearing"); + UtPrintf("Set to all 0, max msgid value"); memset(&msg, 0, sizeof(msg)); msgidval_exp = CFE_PLATFORM_SB_HIGHEST_VALID_MSGID; - ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg), false), CFE_SUCCESS); + ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg)), CFE_SUCCESS); Test_MSG_PrintMsg(&msg, 0); + /* Default EDS version check */ + ASSERT_EQ(CFE_MSG_GetEDSVersion(&msg, &edsver), CFE_SUCCESS); + ASSERT_EQ(edsver, CFE_PLATFORM_EDSVER); + + /* Default system check */ + ASSERT_EQ(CFE_MSG_GetSystem(&msg, &system), CFE_SUCCESS); + ASSERT_EQ(system, sc_id); + + /* Default endian check */ + ASSERT_EQ(CFE_MSG_GetEndian(&msg, &endian), CFE_SUCCESS); +#if (CFE_PLATFORM_ENDIAN == CCSDS_LITTLE_ENDIAN) + ASSERT_EQ(endian, CFE_MSG_Endian_Little); +#else + ASSERT_EQ(endian, CFE_MSG_Endian_Big); +#endif + /* Default subsystem check */ ASSERT_EQ(CFE_MSG_GetSubsystem(&msg, &subsys), CFE_SUCCESS); if (is_v1) - ASSERT_EQ(subsys, 0); + ASSERT_EQ(subsys, CFE_PLATFORM_DEFAULT_SUBSYS); else - ASSERT_EQ(subsys, 0xFF); + ASSERT_EQ(subsys, CFE_PLATFORM_DEFAULT_SUBSYS | ((msgidval_exp >> 8) & 0xFF)); /* Confirm the rest of the fields not already explicitly checked */ - ASSERT_EQ(Test_MSG_Ext_NotZero(&msg) & ~MSG_SUBSYS_FLAG, 0); + ASSERT_EQ(Test_MSG_Ext_NotZero(&msg) & ~(MSG_EDSVER_FLAG | MSG_ENDIAN_FLAG | MSG_SUBSYS_FLAG | MSG_SYSTEM_FLAG), 0); + } void Test_MSG_EDSVersion(void) diff --git a/modules/msg/unit-test-coverage/test_cfe_msg_init.c b/modules/msg/unit-test-coverage/test_cfe_msg_init.c index 972e82973..33964762e 100644 --- a/modules/msg/unit-test-coverage/test_cfe_msg_init.c +++ b/modules/msg/unit-test-coverage/test_cfe_msg_init.c @@ -50,44 +50,21 @@ void Test_MSG_Init(void) CFE_MSG_HeaderVersion_t hdrver; CFE_MSG_ApId_t apid; CFE_MSG_SegmentationFlag_t segflag; - unsigned int flag_exp; bool hassec; bool is_v1; UtPrintf("Bad parameter tests, Null pointer, invalid size, invalid msgid"); - memset(&msg, 0, sizeof(msg)); - ASSERT_EQ(CFE_MSG_Init(NULL, CFE_SB_ValueToMsgId(0), sizeof(msg), false), CFE_MSG_BAD_ARGUMENT); - ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(0), 0, false), CFE_MSG_BAD_ARGUMENT); - ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1), sizeof(msg), false), + ASSERT_EQ(CFE_MSG_Init(NULL, CFE_SB_ValueToMsgId(0), sizeof(msg)), CFE_MSG_BAD_ARGUMENT); + ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(0), 0), CFE_MSG_BAD_ARGUMENT); + ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1), sizeof(msg)), CFE_MSG_BAD_ARGUMENT); - ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(-1), sizeof(msg), false), CFE_MSG_BAD_ARGUMENT); - ASSERT_EQ(Test_MSG_NotZero(&msg), 0); + ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(-1), sizeof(msg)), CFE_MSG_BAD_ARGUMENT); - UtPrintf("Set to all F's, msgid value = 0, and run without clearing"); + UtPrintf("Set to all F's, msgid value = 0"); memset(&msg, 0xFF, sizeof(msg)); msgidval_exp = 0; - ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg), false), CFE_SUCCESS); - Test_MSG_PrintMsg(&msg, 0); - - /* Get msgid version by checking if header version was set */ - ASSERT_EQ(CFE_MSG_GetHeaderVersion(&msg, &hdrver), CFE_SUCCESS); - is_v1 = (hdrver == 0); - - flag_exp = MSG_TYPE_FLAG | MSG_LENGTH_FLAG | MSG_APID_FLAG; - if (is_v1) - flag_exp |= MSG_HDRVER_FLAG | MSG_HASSEC_FLAG; - - ASSERT_EQ(Test_MSG_Pri_NotF(&msg), flag_exp); - ASSERT_EQ(CFE_MSG_GetMsgId(&msg, &msgid_act), CFE_SUCCESS); - ASSERT_EQ(CFE_SB_MsgIdToValue(msgid_act), msgidval_exp); - ASSERT_EQ(CFE_MSG_GetSize(&msg, &size), CFE_SUCCESS); - ASSERT_EQ(size, sizeof(msg)); - UtPrintf("Set to all F's, msgid value = 0, and run with clearing"); - memset(&msg, 0xFF, sizeof(msg)); - msgidval_exp = 0; - - ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg), true), CFE_SUCCESS); + ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg)), CFE_SUCCESS); Test_MSG_PrintMsg(&msg, 0); ASSERT_EQ(CFE_MSG_GetMsgId(&msg, &msgid_act), CFE_SUCCESS); ASSERT_EQ(CFE_SB_MsgIdToValue(msgid_act), msgidval_exp); @@ -99,49 +76,52 @@ void Test_MSG_Init(void) ASSERT_EQ(CFE_MSG_GetApId(&msg, &apid), CFE_SUCCESS); ASSERT_EQ(CFE_MSG_GetHeaderVersion(&msg, &hdrver), CFE_SUCCESS); ASSERT_EQ(CFE_MSG_GetHasSecondaryHeader(&msg, &hassec), CFE_SUCCESS); + + /* A zero msgid will set hassec to false for v1 */ + is_v1 = !hassec; + if (!is_v1) { ASSERT_EQ(apid, CFE_PLATFORM_DEFAULT_APID & TEST_DEFAULT_APID_MASK); ASSERT_EQ(hdrver, CFE_MISSION_CCSDSVER); - ASSERT_EQ(hassec, true); } else { ASSERT_EQ(apid, 0); ASSERT_EQ(hdrver, 0); - ASSERT_EQ(hassec, false); } /* Confirm the rest of the fields not already explicitly checked */ ASSERT_EQ(Test_MSG_Pri_NotZero(&msg) & ~(MSG_APID_FLAG | MSG_HDRVER_FLAG | MSG_HASSEC_FLAG), MSG_LENGTH_FLAG | MSG_SEGMENT_FLAG); - UtPrintf("Set to all 0, max msgid value, and run without clearing"); + UtPrintf("Set to all 0, max msgid value"); memset(&msg, 0, sizeof(msg)); msgidval_exp = CFE_PLATFORM_SB_HIGHEST_VALID_MSGID; - ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg), false), CFE_SUCCESS); + + ASSERT_EQ(CFE_MSG_Init(&msg, CFE_SB_ValueToMsgId(msgidval_exp), sizeof(msg)), CFE_SUCCESS); Test_MSG_PrintMsg(&msg, 0); ASSERT_EQ(CFE_MSG_GetMsgId(&msg, &msgid_act), CFE_SUCCESS); ASSERT_EQ(CFE_SB_MsgIdToValue(msgid_act), msgidval_exp); ASSERT_EQ(CFE_MSG_GetSize(&msg, &size), CFE_SUCCESS); ASSERT_EQ(size, sizeof(msg)); + ASSERT_EQ(CFE_MSG_GetSegmentationFlag(&msg, &segflag), CFE_SUCCESS); + ASSERT_EQ(segflag, CFE_MSG_SegFlag_Unsegmented); ASSERT_EQ(CFE_MSG_GetApId(&msg, &apid), CFE_SUCCESS); + ASSERT_EQ(CFE_MSG_GetHeaderVersion(&msg, &hdrver), CFE_SUCCESS); ASSERT_EQ(CFE_MSG_GetHasSecondaryHeader(&msg, &hassec), CFE_SUCCESS); + ASSERT_EQ(hassec, true); if (!is_v1) { ASSERT_EQ(apid & TEST_DEFAULT_APID_MASK, CFE_PLATFORM_DEFAULT_APID & TEST_DEFAULT_APID_MASK); - ASSERT_EQ(hassec, false); + ASSERT_EQ(hdrver, CFE_MISSION_CCSDSVER); } else { ASSERT_EQ(apid, 0x7FF); - ASSERT_EQ(hassec, true); + ASSERT_EQ(hdrver, 0); } - ASSERT_EQ(Test_MSG_Pri_NotZero(&msg) & ~(MSG_APID_FLAG | MSG_HASSEC_FLAG), MSG_TYPE_FLAG | MSG_LENGTH_FLAG); - - /* Zero (no default) header version check */ - ASSERT_EQ(CFE_MSG_GetHeaderVersion(&msg, &hdrver), CFE_SUCCESS); - ASSERT_EQ(hdrver, 0); + ASSERT_EQ(Test_MSG_Pri_NotZero(&msg) & ~MSG_HDRVER_FLAG, MSG_APID_FLAG | MSG_HASSEC_FLAG | MSG_TYPE_FLAG | MSG_LENGTH_FLAG | MSG_SEGMENT_FLAG); }