From a9989fbbb5cd7975b763a9b4a06292ec483a5bdf Mon Sep 17 00:00:00 2001 From: Joseph Hickey Date: Tue, 19 Dec 2023 14:09:16 -0500 Subject: [PATCH] Fix #2488, separate bad argument test 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. --- modules/cfe_testcase/src/msg_api_test.c | 8 +++++--- modules/msg/fsw/src/cfe_msg_msgid_v1.c | 2 +- modules/msg/fsw/src/cfe_msg_msgid_v2.c | 2 +- modules/msg/ut-coverage/test_cfe_msg_init.c | 15 +++++++++------ .../msg/ut-coverage/test_cfe_msg_msgid_shared.c | 3 +++ modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c | 12 +++++------- modules/msg/ut-coverage/test_cfe_msg_msgid_v2.c | 11 +++++------ 7 files changed, 29 insertions(+), 24 deletions(-) diff --git a/modules/cfe_testcase/src/msg_api_test.c b/modules/cfe_testcase/src/msg_api_test.c index 623e60973..5eaa91599 100644 --- a/modules/cfe_testcase/src/msg_api_test.c +++ b/modules/cfe_testcase/src/msg_api_test.c @@ -26,6 +26,7 @@ */ #include "cfe_test.h" +#include "cfe_test_msgids.h" #include void TestMsgApiBasic(void) @@ -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)), @@ -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); diff --git a/modules/msg/fsw/src/cfe_msg_msgid_v1.c b/modules/msg/fsw/src/cfe_msg_msgid_v1.c index e4574e2fe..b205f18ac 100644 --- a/modules/msg/fsw/src/cfe_msg_msgid_v1.c +++ b/modules/msg/fsw/src/cfe_msg_msgid_v1.c @@ -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)) { return CFE_MSG_BAD_ARGUMENT; } diff --git a/modules/msg/fsw/src/cfe_msg_msgid_v2.c b/modules/msg/fsw/src/cfe_msg_msgid_v2.c index 8c8876fae..f402a729d 100644 --- a/modules/msg/fsw/src/cfe_msg_msgid_v2.c +++ b/modules/msg/fsw/src/cfe_msg_msgid_v2.c @@ -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; } diff --git a/modules/msg/ut-coverage/test_cfe_msg_init.c b/modules/msg/ut-coverage/test_cfe_msg_init.c index a072566ae..c63a8dfcf 100644 --- a/modules/msg/ut-coverage/test_cfe_msg_init.c +++ b/modules/msg/ut-coverage/test_cfe_msg_init.c @@ -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; diff --git a/modules/msg/ut-coverage/test_cfe_msg_msgid_shared.c b/modules/msg/ut-coverage/test_cfe_msg_msgid_shared.c index 32e962954..0ab6b3310 100644 --- a/modules/msg/ut-coverage/test_cfe_msg_msgid_shared.c +++ b/modules/msg/ut-coverage/test_cfe_msg_msgid_shared.c @@ -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)); 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 7b56a845b..da0f127e7 100644 --- a/modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c +++ b/modules/msg/ut-coverage/test_cfe_msg_msgid_v1.c @@ -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); diff --git a/modules/msg/ut-coverage/test_cfe_msg_msgid_v2.c b/modules/msg/ut-coverage/test_cfe_msg_msgid_v2.c index 6a07cbe62..6e293cd1c 100644 --- a/modules/msg/ut-coverage/test_cfe_msg_msgid_v2.c +++ b/modules/msg/ut-coverage/test_cfe_msg_msgid_v2.c @@ -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);