Skip to content

Commit

Permalink
Fix nasa#44, instantiate properly sized and aligned buffers
Browse files Browse the repository at this point in the history
For unit unit tests that invoke CF PDU processing functions
on either input or output, ensure that the locally instantiated
"dummy" PDU is both sized sufficiently and aligned correctly.

This removes quite a bit of questionable casting between the
buffer types, and fixes a number of stack-smashing issues.

For completeness, this also clears (memset to 0) all instantiated
buffers, before setting values in the test.  This ensures that the
entire message structure has predictable/repeatable content.
  • Loading branch information
jphickey committed Dec 1, 2021
1 parent 7b99b91 commit 95612b0
Show file tree
Hide file tree
Showing 6 changed files with 954 additions and 729 deletions.
17 changes: 0 additions & 17 deletions fsw/src/cf_cfdp_pdu.h
Original file line number Diff line number Diff line change
Expand Up @@ -232,36 +232,19 @@ typedef struct
pdu_fd_data_t fdd;
} CF_PACK pdu_fd_t;

#if 0
typedef union {
uint8 data[CF_MAX_PDU_SIZE];
pdu_header_t ph;
pdu_file_directive_header_t fdh;

pdu_fd_t fd;
pdu_md_t md;
pdu_eof_t eof;
pdu_ack_t ack;
pdu_fin_t fin;
pdu_nak_t nak;
} CF_PACK pdu_t;
#endif

/* NOTE: the use of pdu_header_t below is correct, but the pdu_r_msg_t and pdu_s_msg_t
* structures are both longer than these definitions. They are always backed by a buffer
* of size CF_MAX_PDU_SIZE */
typedef struct
{
CFE_MSG_CommandHeader_t hdr;
pdu_header_t ph;
// pdu_t pdu;
} CF_PACK pdu_r_msg_t;

typedef struct
{
CFE_MSG_TelemetryHeader_t hdr;
pdu_header_t ph;
// pdu_t pdu;
} CF_PACK pdu_s_msg_t;

DECLARE_FIELD(PDU_MD_SEGMENTATION_CONTROL, 1, 7)
Expand Down
48 changes: 30 additions & 18 deletions unit-test/cf_cfdp_helpers_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -775,10 +775,12 @@ void Test_CF_GetEIDSize_When_ret_Is_LessThan_size_of_cf_entity_id_t_Add_1_AndRet
void Test_CF_GetVariableHeader_When_eid_l_AND_tsn_l_AreNotGreaterThan_0_DoesNotCallAnyMemCopiesReturn_neg1(void)
{
/* Arrange */
pdu_r_msg_t dummy_ph;
int local_result;
CF_UT_inmsg_buffer_t dummy_ph;
int local_result;

CF_AppData.engine.in.msg = (CFE_SB_Buffer_t *)&dummy_ph;
memset(&dummy_ph, 0, sizeof(dummy_ph));

CF_AppData.engine.in.msg = &dummy_ph.cfe_sb_buffer;

/* Arrange for CF_GetEIDSize */
uint32 forced_return_FGV_from_EID = sizeof(cf_entity_id_t); /* unstubbable code adds +1 to this value */
Expand All @@ -804,10 +806,12 @@ void Test_CF_GetVariableHeader_When_eid_l_AND_tsn_l_AreNotGreaterThan_0_DoesNotC
void Test_CF_GetVariableHeader_WhenOnly_eid_l_IsGreaterThan_0_DoesNotCallAnyMemCopiesReturn_neg1(void)
{
/* Arrange */
pdu_r_msg_t dummy_ph;
int local_result;
CF_UT_inmsg_buffer_t dummy_ph;
int local_result;

memset(&dummy_ph, 0, sizeof(dummy_ph));

CF_AppData.engine.in.msg = (CFE_SB_Buffer_t *)&dummy_ph;
CF_AppData.engine.in.msg = &dummy_ph.cfe_sb_buffer;

/* Arrange for CF_GetEIDSize */
uint32 forced_return_FGV_from_EID = sizeof(cf_entity_id_t) - 1; /* unstubbable code adds +1 to this value */
Expand Down Expand Up @@ -835,10 +839,12 @@ void Test_CF_GetVariableHeader_WhenOnly_eid_l_IsGreaterThan_0_DoesNotCallAnyMemC
void Test_CF_GetVariableHeader_WhenOnly_tsn_l_IsGreaterThan_0_DoesNotCallAnyMemCopiesReturn_neg1(void)
{
/* Arrange */
pdu_r_msg_t dummy_ph;
int local_result;
CF_UT_inmsg_buffer_t dummy_ph;
int local_result;

memset(&dummy_ph, 0, sizeof(dummy_ph));

CF_AppData.engine.in.msg = (CFE_SB_Buffer_t *)&dummy_ph;
CF_AppData.engine.in.msg = &dummy_ph.cfe_sb_buffer;

/* Arrange for CF_GetEIDSize */
uint32 forced_return_FGV_from_EID = sizeof(cf_entity_id_t); /* unstubbable code adds +1 to this value */
Expand Down Expand Up @@ -866,10 +872,12 @@ void Test_CF_GetVariableHeader_WhenOnly_tsn_l_IsGreaterThan_0_DoesNotCallAnyMemC
void Test_CF_GetVariableHeader_GetsAllThreeVariableLengthItemsOutOfHeaderAndReturn_0(void)
{
/* Arrange */
pdu_r_msg_t dummy_ph;
int local_result;
CF_UT_inmsg_buffer_t dummy_ph;
int local_result;

CF_AppData.engine.in.msg = (CFE_SB_Buffer_t *)&dummy_ph;
memset(&dummy_ph, 0, sizeof(dummy_ph));

CF_AppData.engine.in.msg = &dummy_ph.cfe_sb_buffer;

/* Arrange for CF_GetEIDSize */
uint32 forced_return_FGV_from_EID =
Expand Down Expand Up @@ -903,12 +911,14 @@ void Test_CF_GetVariableHeader_GetsAllThreeVariableLengthItemsOutOfHeaderAndRetu
void Test_CF_SetVariableHeader_Call_FSV_Twice(void)
{
/* Arrange */
cf_entity_id_t arg_src_eid = 1;
cf_entity_id_t arg_dst_eid = 1;
cf_transaction_seq_t arg_tsn = 1;
pdu_s_msg_t dummy_msg;
cf_entity_id_t arg_src_eid = 1;
cf_entity_id_t arg_dst_eid = 1;
cf_transaction_seq_t arg_tsn = 1;
CF_UT_outmsg_buffer_t dummy_msg;

memset(&dummy_msg, 0, sizeof(dummy_msg));

CF_AppData.engine.out.msg = (CFE_SB_Buffer_t *)&dummy_msg;
CF_AppData.engine.out.msg = &dummy_msg.cfe_sb_buffer;

/* Act */
CF_SetVariableHeader(arg_src_eid, arg_dst_eid, arg_tsn);
Expand Down Expand Up @@ -974,6 +984,8 @@ void Test_CF_HeaderSize_Return_sizeof_pdu_header_t_Plus_2_Times_eid_l_Plus_tsn_l
(forced_return_FGV_for_tsn_l + 1); /* each +1 added by CUT */
int local_result;

memset(&dummy_ph, 0, sizeof(dummy_ph));

UT_SetDeferredRetcode(UT_KEY(FGV), 1, forced_return_FGV_for_eid_l);
UT_SetDeferredRetcode(UT_KEY(FGV), 1, forced_return_FGV_for_tsn_l);

Expand Down Expand Up @@ -1210,4 +1222,4 @@ void UtTest_Setup(void)

} /* end UtTest_Setup for cf_cfdp_helpers_tests.c */

/* end cf_cfdp_helpers_tests.c */
/* end cf_cfdp_helpers_tests.c */
Loading

0 comments on commit 95612b0

Please sign in to comment.