Skip to content

Commit

Permalink
Fix #2488, separate bad argument test
Browse files Browse the repository at this point in the history 
Pass in CFE_SB_INVALID_MSG_ID separately from a NULL pointer.
This also required updates to the way the MSG module checks the
MsgID, as it was not using the IsValidMsgID check.
  • Loading branch information
@jphickey
jphickey committed Dec 19, 2023
1 parent 7f5ebcd commit a9989fb
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 24 deletions.
8 changes: 5 additions & 3 deletions modules/cfe_testcase/src/msg_api_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
*/

#include "cfe_test.h"
#include "cfe_test_msgids.h"
#include <string.h>

void TestMsgApiBasic(void)
Expand All @@ -46,10 +47,11 @@ void TestMsgApiBasic(void)
bool _returned = false;

memset(&cmd, 0xFF, sizeof(cmd));
msgId = CFE_SB_ValueToMsgId(1);
msgId = CFE_SB_ValueToMsgId(CFE_TEST_CMD_MID);

/* test msg-init */
UtAssert_INT32_EQ(CFE_MSG_Init(NULL, CFE_SB_INVALID_MSG_ID, 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(CFE_MSG_PTR(cmd), CFE_SB_INVALID_MSG_ID, sizeof(cmd)), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(CFE_MSG_Init(CFE_MSG_PTR(cmd), msgId, 0), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(
CFE_MSG_Init(CFE_MSG_PTR(cmd), CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1), sizeof(cmd)),
Expand Down Expand Up @@ -135,7 +137,7 @@ void TestMsgApiAdvanced(void)
CFE_MSG_SequenceCount_t seqCnt;

memset(&cmd, 0xFF, sizeof(cmd));
msgId = CFE_SB_INVALID_MSG_ID;
msgId = CFE_SB_ValueToMsgId(CFE_TEST_CMD_MID);

UtAssert_INT32_EQ(CFE_MSG_Init(CFE_MSG_PTR(cmd), msgId, sizeof(cmd)), CFE_SUCCESS);

Expand Down
2 changes: 1 addition & 1 deletion modules/msg/fsw/src/cfe_msg_msgid_v1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ CFE_Status_t CFE_MSG_SetMsgId(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId)
{
CFE_SB_MsgId_Atom_t msgidval = CFE_SB_MsgIdToValue(MsgId);

if (MsgPtr == NULL || msgidval > CFE_PLATFORM_SB_HIGHEST_VALID_MSGID)
if (MsgPtr == NULL || !CFE_SB_IsValidMsgId(MsgId))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression Warning

This Boolean expression is not side-effect free.
{
return CFE_MSG_BAD_ARGUMENT;
}
Expand Down
2 changes: 1 addition & 1 deletion modules/msg/fsw/src/cfe_msg_msgid_v2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ CFE_Status_t CFE_MSG_SetMsgId(CFE_MSG_Message_t *MsgPtr, CFE_SB_MsgId_t MsgId)
{
CFE_SB_MsgId_Atom_t msgidval = CFE_SB_MsgIdToValue(MsgId);

if (MsgPtr == NULL || msgidval > CFE_PLATFORM_SB_HIGHEST_VALID_MSGID)
if (MsgPtr == NULL || !CFE_SB_IsValidMsgId(MsgId))
{
return CFE_MSG_BAD_ARGUMENT;
}
Expand Down
15 changes: 9 additions & 6 deletions modules/msg/ut-coverage/test_cfe_msg_init.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,18 @@ void Test_MSG_Init(void)
bool hassec;
bool is_v1;

msgidval_exp = 1;
msgid_act = CFE_SB_ValueToMsgId(msgidval_exp);

UtPrintf("Bad parameter tests, Null pointer, invalid size, invalid msgid");
UtAssert_INT32_EQ(CFE_MSG_Init(NULL, CFE_SB_INVALID_MSG_ID, sizeof(cmd)), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(CFE_MSG_Init(CFE_MSG_PTR(cmd), CFE_SB_INVALID_MSG_ID, 0), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(
CFE_MSG_Init(CFE_MSG_PTR(cmd), CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID + 1), sizeof(cmd)),
CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(CFE_MSG_Init(CFE_MSG_PTR(cmd), CFE_SB_ValueToMsgId(-1), sizeof(cmd)), CFE_MSG_BAD_ARGUMENT);
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), true);
UtAssert_INT32_EQ(CFE_MSG_Init(NULL, msgid_act, sizeof(cmd)), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(CFE_MSG_Init(CFE_MSG_PTR(cmd), msgid_act, 0), CFE_MSG_BAD_ARGUMENT);
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), false);
UtAssert_INT32_EQ(CFE_MSG_Init(CFE_MSG_PTR(cmd), msgid_act, sizeof(cmd)), CFE_MSG_BAD_ARGUMENT);

UtPrintf("Set to all F's, msgid value = 0");
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), true);
memset(&cmd, 0xFF, sizeof(cmd));
msgidval_exp = 0;

Expand Down
3 changes: 3 additions & 0 deletions modules/msg/ut-coverage/test_cfe_msg_msgid_shared.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,17 @@ void Test_MSG_GetTypeFromMsgId(void)
CFE_MSG_Type_t actual = CFE_MSG_Type_Invalid;

UtPrintf("Bad parameter tests, Null pointer");
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), true);
memset(&msg, 0, sizeof(msg));
UtAssert_INT32_EQ(CFE_MSG_GetTypeFromMsgId(msgid, NULL), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0);

UtPrintf("Bad parameter tests, Invalid message ID");
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), false);
UtAssert_INT32_EQ(CFE_MSG_GetTypeFromMsgId(CFE_SB_ValueToMsgId(-1), &actual), CFE_MSG_BAD_ARGUMENT);

UtPrintf("Set to all F's, test cmd and tlm");
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), true);
memset(&msg, 0xFF, sizeof(msg));
CFE_UtAssert_SUCCESS(CFE_MSG_SetMsgId(&msg, CFE_SB_ValueToMsgId(CFE_PLATFORM_SB_HIGHEST_VALID_MSGID)));
CFE_UtAssert_SUCCESS(CFE_MSG_SetType(&msg, CFE_MSG_Type_Tlm));
Expand Down
12 changes: 5 additions & 7 deletions modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,20 @@ void Test_MSG_MsgId(void)
CFE_MSG_ApId_t apid;
bool hassec;

UtPrintf("Bad parameter tests, Null pointers and invalid (max valid + 1)");
UtPrintf("Bad parameter tests, Null pointers and invalid msg ID");
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), true);
memset(&msg, 0, sizeof(msg));
UtAssert_INT32_EQ(CFE_MSG_GetMsgId(NULL, &msgid), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 1);
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_ValueToMsgId(-1)), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0);
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, CFE_SB_ValueToMsgId(0xFFFF)), CFE_MSG_BAD_ARGUMENT);
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), false);
UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, msgid), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(Test_MSG_NotZero(&msg), 0);

UtPrintf("Set msg to all F's, set msgid to 1 and verify");
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), true);
memset(&msg, 0xFF, sizeof(msg));
CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid));
UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 0xFFFF);
Expand Down
11 changes: 5 additions & 6 deletions modules/msg/ut-coverage/test_cfe_msg_msgid_v2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,20 @@ void Test_MSG_MsgId(void)
local_subsys_flag = MSG_SUBSYS_FLAG;
}

UtPrintf("Bad parameter tests, Null pointers and invalid (max valid + 1)");
UtPrintf("Bad parameter tests, Null pointers and invalid msg ID");
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), true);
memset(&msg, 0, sizeof(msg));
UtAssert_INT32_EQ(CFE_MSG_GetMsgId(NULL, &msgid), CFE_MSG_BAD_ARGUMENT);
UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 1);
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(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(Test_MSG_NotZero(&msg), 0);
UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, 0xFFFFFFFF), CFE_MSG_BAD_ARGUMENT);
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), false);
UtAssert_INT32_EQ(CFE_MSG_SetMsgId(&msg, msgid), 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");
UT_SetDefaultReturnValue(UT_KEY(CFE_SB_IsValidMsgId), true);
memset(&msg, 0xFF, sizeof(msg));
CFE_UtAssert_SUCCESS(CFE_MSG_GetMsgId(&msg, &msgid));
UtAssert_INT32_EQ(CFE_SB_MsgIdToValue(msgid), 0xFFFF);
Expand Down

0 comments on commit a9989fb

Please sign in to comment.