From 168181f20be0c4fb5f02f8701baa022294b18f7f Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Fri, 10 Sep 2021 17:01:26 -0400 Subject: [PATCH] Fix #1945, add CFE_SB_ValueToMsgId/MsgIdToValue wrappers Correct code that was not correctly using the CFE_SB_ValueToMsgId or CFE_SB_MsgIdToValue conversion wrappers where required to do so. This should be used whenever the value is intentionally converted to/from an integer. The CFE_SB_MsgId_t type should not be assumed to be an integer in nature. --- modules/cfe_testcase/src/message_id_test.c | 4 ++-- modules/cfe_testcase/src/msg_api_test.c | 4 ++-- modules/cfe_testcase/src/sb_subscription_test.c | 7 +------ modules/cfe_testcase/src/tbl_information_test.c | 2 +- .../cfe_testcase/src/tbl_registration_test.c | 2 +- modules/core_api/fsw/inc/cfe_sb.h | 2 +- modules/core_api/fsw/inc/cfe_sb_api_typedefs.h | 17 +++++++++++++++-- modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c | 11 ++++++----- 8 files changed, 29 insertions(+), 20 deletions(-) diff --git a/modules/cfe_testcase/src/message_id_test.c b/modules/cfe_testcase/src/message_id_test.c index 235d1cf8d..2956c2895 100644 --- a/modules/cfe_testcase/src/message_id_test.c +++ b/modules/cfe_testcase/src/message_id_test.c @@ -42,7 +42,7 @@ void TestMsgId(void) UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, expectedmsgid), CFE_SUCCESS); UtAssert_INT32_EQ(CFE_MSG_GetMsgId(&msg, &msgid), CFE_SUCCESS); - UtAssert_UINT32_EQ(msgid, expectedmsgid); + CFE_Assert_MSGID_EQ(msgid, expectedmsgid); UtAssert_INT32_EQ(CFE_MSG_SetMsgId(NULL, msgid), CFE_MSG_BAD_ARGUMENT); @@ -93,4 +93,4 @@ void MessageIdTestSetup(void) { UtTest_Add(TestMsgId, NULL, NULL, "Test Set/Get Message ID"); UtTest_Add(TestGetTypeFromMsgId, NULL, NULL, "Test Get Type From Message ID"); -} \ No newline at end of file +} diff --git a/modules/cfe_testcase/src/msg_api_test.c b/modules/cfe_testcase/src/msg_api_test.c index 11e12f40d..ddeb29b6a 100644 --- a/modules/cfe_testcase/src/msg_api_test.c +++ b/modules/cfe_testcase/src/msg_api_test.c @@ -53,9 +53,9 @@ void TestMsgApiBasic(void) msgId = CFE_SB_ValueToMsgId(0); /* test msg-init */ - UtAssert_INT32_EQ(CFE_MSG_Init(NULL, CFE_SB_ValueToMsgId(0), sizeof(cmd)), CFE_MSG_BAD_ARGUMENT); + UtAssert_INT32_EQ(CFE_MSG_Init(NULL, msgId, sizeof(cmd)), CFE_MSG_BAD_ARGUMENT); UtAssert_INT32_EQ(CFE_MSG_Init(&cmd.Msg, msgId, 0), CFE_MSG_BAD_ARGUMENT); - UtAssert_INT32_EQ(CFE_MSG_Init(&cmd.Msg, CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1, sizeof(cmd)), + UtAssert_INT32_EQ(CFE_MSG_Init(&cmd.Msg, CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1), sizeof(cmd)), CFE_MSG_BAD_ARGUMENT); UtAssert_INT32_EQ(CFE_MSG_Init(&cmd.Msg, msgId, sizeof(cmd)), CFE_SUCCESS); diff --git a/modules/cfe_testcase/src/sb_subscription_test.c b/modules/cfe_testcase/src/sb_subscription_test.c index 8e1745272..628360445 100644 --- a/modules/cfe_testcase/src/sb_subscription_test.c +++ b/modules/cfe_testcase/src/sb_subscription_test.c @@ -54,7 +54,6 @@ void TestSubscribeUnsubscribe(void) /* Subscribe - Confirm Bad MsgId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_Subscribe(CFE_SB_INVALID_MSG_ID, PipeId1), CFE_SB_BAD_ARGUMENT); - UtAssert_INT32_EQ(CFE_SB_Subscribe(CFE_SB_MSGID_RESERVED, PipeId2), CFE_SB_BAD_ARGUMENT); /* Subscribe - Confirm Bad PipeId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_Subscribe(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE), CFE_SB_BAD_ARGUMENT); @@ -73,7 +72,6 @@ void TestSubscribeUnsubscribe(void) /* Unsubscribe - Confirm Bad MsgId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_Unsubscribe(CFE_SB_INVALID_MSG_ID, PipeId1), CFE_SB_BAD_ARGUMENT); - UtAssert_INT32_EQ(CFE_SB_Unsubscribe(CFE_SB_MSGID_RESERVED, PipeId2), CFE_SB_BAD_ARGUMENT); /* Unsubscribe - Confirm Bad PipeId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_Unsubscribe(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE), CFE_SB_BAD_ARGUMENT); @@ -108,7 +106,6 @@ void TestSubscribeUnsubscribeLocal(void) /* Subscribe - Confirm Bad MsgId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_SubscribeLocal(CFE_SB_INVALID_MSG_ID, PipeId1, 2), CFE_SB_BAD_ARGUMENT); - UtAssert_INT32_EQ(CFE_SB_SubscribeLocal(CFE_SB_MSGID_RESERVED, PipeId2, 2), CFE_SB_BAD_ARGUMENT); /* Subscribe - Confirm Bad PipeId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_SubscribeLocal(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE, 2), CFE_SB_BAD_ARGUMENT); @@ -127,7 +124,6 @@ void TestSubscribeUnsubscribeLocal(void) /* Unsubscribe - Confirm Bad MsgId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_UnsubscribeLocal(CFE_SB_INVALID_MSG_ID, PipeId1), CFE_SB_BAD_ARGUMENT); - UtAssert_INT32_EQ(CFE_SB_UnsubscribeLocal(CFE_SB_MSGID_RESERVED, PipeId2), CFE_SB_BAD_ARGUMENT); /* Unsubscribe - Confirm Bad PipeId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_UnsubscribeLocal(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE), CFE_SB_BAD_ARGUMENT); @@ -174,7 +170,6 @@ void TestSubscribeEx(void) /* Subscribe - Confirm Bad MsgId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_SubscribeEx(CFE_SB_INVALID_MSG_ID, PipeId1, CFE_SB_DEFAULT_QOS, 2), CFE_SB_BAD_ARGUMENT); - UtAssert_INT32_EQ(CFE_SB_SubscribeEx(CFE_SB_MSGID_RESERVED, PipeId2, CFE_SB_DEFAULT_QOS, 2), CFE_SB_BAD_ARGUMENT); /* Subscribe - Confirm Bad PipeId Arg Rejection */ UtAssert_INT32_EQ(CFE_SB_SubscribeEx(CFE_FT_CMD_MSGID, CFE_SB_INVALID_PIPE, CFE_SB_DEFAULT_QOS, 2), @@ -214,7 +209,7 @@ void TestSBMaxSubscriptions(void) while (NumSubs <= CFE_PLATFORM_SB_MAX_MSG_IDS) { /* fabricate a msgid to subscribe to (this may overlap real msgids) */ - TestMsgId = CFE_SB_MSGID_WRAP_VALUE(CFE_PLATFORM_CMD_MID_BASE + NumSubs); + TestMsgId = CFE_SB_ValueToMsgId(CFE_PLATFORM_CMD_MID_BASE + NumSubs); Status = CFE_SB_Subscribe(TestMsgId, PipeId); if (Status != CFE_SUCCESS) diff --git a/modules/cfe_testcase/src/tbl_information_test.c b/modules/cfe_testcase/src/tbl_information_test.c index 813aa4ef0..bb44c31dc 100644 --- a/modules/cfe_testcase/src/tbl_information_test.c +++ b/modules/cfe_testcase/src/tbl_information_test.c @@ -78,7 +78,7 @@ void TestNotifyByMessage(void) UtPrintf("Testing: CFE_TBL_NotifyByMessage"); CFE_TBL_Handle_t SharedTblHandle; const char * SharedTblName = "SAMPLE_APP.SampleAppTable"; - CFE_SB_MsgId_t TestMsgId = CFE_TEST_CMD_MID; + CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(CFE_TEST_CMD_MID); CFE_MSG_FcnCode_t TestCmdCode = 0; uint32 TestParameter = 0; diff --git a/modules/cfe_testcase/src/tbl_registration_test.c b/modules/cfe_testcase/src/tbl_registration_test.c index ed300ece3..dcbf74885 100644 --- a/modules/cfe_testcase/src/tbl_registration_test.c +++ b/modules/cfe_testcase/src/tbl_registration_test.c @@ -220,7 +220,7 @@ void TestTblNonAppContext(void) CFE_ES_ERR_RESOURCEID_NOT_VALID); UtAssert_INT32_EQ(CFE_TBL_Manage(CFE_FT_Global.TblHandle), CFE_ES_ERR_RESOURCEID_NOT_VALID); UtAssert_INT32_EQ(CFE_TBL_Modified(CFE_FT_Global.TblHandle), CFE_ES_ERR_RESOURCEID_NOT_VALID); - UtAssert_INT32_EQ(CFE_TBL_NotifyByMessage(CFE_FT_Global.TblHandle, CFE_TEST_CMD_MID, 0, 0), + UtAssert_INT32_EQ(CFE_TBL_NotifyByMessage(CFE_FT_Global.TblHandle, CFE_SB_ValueToMsgId(CFE_TEST_CMD_MID), 0, 0), CFE_ES_ERR_RESOURCEID_NOT_VALID); UtAssert_INT32_EQ(CFE_TBL_ReleaseAddress(CFE_FT_Global.TblHandle), CFE_ES_ERR_RESOURCEID_NOT_VALID); UtAssert_INT32_EQ(CFE_TBL_Share(&Handle, CFE_FT_Global.TblName), CFE_ES_ERR_RESOURCEID_NOT_VALID); diff --git a/modules/core_api/fsw/inc/cfe_sb.h b/modules/core_api/fsw/inc/cfe_sb.h index 8f1eeff8f..0133eb06c 100644 --- a/modules/core_api/fsw/inc/cfe_sb.h +++ b/modules/core_api/fsw/inc/cfe_sb.h @@ -827,7 +827,7 @@ static inline CFE_SB_MsgId_Atom_t CFE_SB_MsgIdToValue(CFE_SB_MsgId_t MsgId) */ static inline CFE_SB_MsgId_t CFE_SB_ValueToMsgId(CFE_SB_MsgId_Atom_t MsgIdValue) { - CFE_SB_MsgId_t Result = CFE_SB_MSGID_WRAP_VALUE(MsgIdValue); + CFE_SB_MsgId_t Result = CFE_SB_MSGID_C(MsgIdValue); return Result; } /** @} */ diff --git a/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h b/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h index 64d800062..95a92fecc 100644 --- a/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h +++ b/modules/core_api/fsw/inc/cfe_sb_api_typedefs.h @@ -63,7 +63,20 @@ * * \sa CFE_SB_ValueToMsgId() */ -#define CFE_SB_MSGID_WRAP_VALUE(val) ((CFE_SB_MsgId_t)(val)) +#define CFE_SB_MSGID_WRAP_VALUE(val) (val) + +/** + * \brief Translation macro to convert to MsgId integer values from a literal + * + * This ensures that the literal is interpreted as the CFE_SB_MsgId_t type, rather + * than the default type associated with that literal (e.g. int/unsigned int). + * + * \note Due to constraints in C99 this style of initializer can only be used + * at runtime, not for static/compile-time initializers. + * + * \sa CFE_SB_ValueToMsgId() + */ +#define CFE_SB_MSGID_C(val) ((CFE_SB_MsgId_t)CFE_SB_MSGID_WRAP_VALUE(val)) /** * \brief Translation macro to convert to MsgId integer values from opaque/abstract API values @@ -96,7 +109,7 @@ * purposes (rvalue), #CFE_SB_MSGID_RESERVED should be used instead. * However, in the current implementation, they are equivalent. */ -#define CFE_SB_INVALID_MSG_ID CFE_SB_MSGID_RESERVED +#define CFE_SB_INVALID_MSG_ID CFE_SB_MSGID_C(-1) /** * \brief Cast/Convert a generic CFE_ResourceId_t to a CFE_SB_PipeId_t diff --git a/modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c b/modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c index b7616a27b..9728d1cf7 100644 --- a/modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c +++ b/modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c @@ -48,18 +48,19 @@ void Test_MSG_MsgId(void) UtAssert_INT32_EQ(CFE_MSG_GetMsgId(&msg, NULL), CFE_MSG_BAD_ARGUMENT); UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0); UtAssert_INT32_EQ(CFE_MSG_SetMsgId(NULL, msgid), CFE_MSG_BAD_ARGUMENT); - UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_SB_INVALID_MSG_ID), CFE_MSG_BAD_ARGUMENT); + UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(-1)), CFE_MSG_BAD_ARGUMENT); UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0); - UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1), CFE_MSG_BAD_ARGUMENT); + UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1)), + CFE_MSG_BAD_ARGUMENT); UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0); - UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, 0xFFFF), CFE_MSG_BAD_ARGUMENT); + UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(0xFFFF)), CFE_MSG_BAD_ARGUMENT); UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0); UtPrintf("Set msg to all F's, set msgid to 0 and verify"); memset(&msg, 0xFF, sizeof(msg)); CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid)); UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 0xFFFF); - CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, 0)); + CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(0))); UT_DisplayPkt(&msg, sizeof(msg)); CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid)); UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 0); @@ -69,7 +70,7 @@ void Test_MSG_MsgId(void) memset(&msg, 0, sizeof(msg)); CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid)); UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 0); - CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, CFE_PLATFORM_SB_HIGHEST_VALID_MSGID)); + CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID))); UT_DisplayPkt(&msg, sizeof(msg)); CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid)); UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), CFE_PLATFORM_SB_HIGHEST_VALID_MSGID);