Skip to content

Commit

Permalink
Fix #1945, add CFE_SB_ValueToMsgId/MsgIdToValue wrappers
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jphickey committed Sep 22, 2021
1 parent 98f78e8 commit 2347d7c
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 18 deletions.
4 changes: 2 additions & 2 deletions modules/cfe_testcase/src/message_id_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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");
}
}
2 changes: 1 addition & 1 deletion modules/cfe_testcase/src/msg_api_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ void TestMsgApiBasic(void)
/* 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(&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);
Expand Down
7 changes: 1 addition & 6 deletions modules/cfe_testcase/src/sb_subscription_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion modules/cfe_testcase/src/tbl_information_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion modules/cfe_testcase/src/tbl_registration_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion modules/core_api/fsw/inc/cfe_sb.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
/** @} */
Expand Down
17 changes: 15 additions & 2 deletions modules/core_api/fsw/inc/cfe_sb_api_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,17 @@ void Test_MSG_MsgId(void)
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(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);
Expand All @@ -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);
Expand Down

0 comments on commit 2347d7c

Please sign in to comment.