Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Light initialization logic refactor + remove multiple returns #179

Closed
2 tasks done
thnkslprpt opened this issue Dec 11, 2023 · 0 comments · Fixed by #180
Closed
2 tasks done

Light initialization logic refactor + remove multiple returns #179

thnkslprpt opened this issue Dec 11, 2023 · 0 comments · Fixed by #180

Comments

@thnkslprpt
Copy link
Contributor

thnkslprpt commented Dec 11, 2023

Checklist

  • I reviewed the Contributing Guide.
  • I performed a cursory search to see if the bug report is relevant, not redundant, nor in conflict with other tickets.

Describe the bug
TO_LAB_init contains 5 exit points - where possible this should be reduced to 1 (or maximum 2 if needed, for early exit after input-validation failure).

to_lab/fsw/src/to_lab_app.c

Lines 117 to 155 in ec09026

status = CFE_EVS_Register(NULL, 0, CFE_EVS_EventFilter_BINARY);
if (status != CFE_SUCCESS)
{
CFE_ES_WriteToSysLog("TO_LAB: Error registering for Event Services, RC = 0x%08X\n", (unsigned int)status);
return status;
}
/*
** Initialize housekeeping packet (clear user data area)...
*/
CFE_MSG_Init(CFE_MSG_PTR(TO_LAB_Global.HkTlm.TelemetryHeader), CFE_SB_ValueToMsgId(TO_LAB_HK_TLM_MID),
sizeof(TO_LAB_Global.HkTlm));
status =
CFE_TBL_Register(&TO_LAB_Global.SubsTblHandle, "TO_LAB_Subs", sizeof(TO_LAB_Subs_t), CFE_TBL_OPT_DEFAULT, NULL);
if (status != CFE_SUCCESS)
{
CFE_EVS_SendEvent(TO_LAB_TBL_ERR_EID, CFE_EVS_EventType_ERROR, "L%d TO Can't register table status %i",
__LINE__, (int)status);
return status;
}
status = CFE_TBL_Load(TO_LAB_Global.SubsTblHandle, CFE_TBL_SRC_FILE, "/cf/to_lab_sub.tbl");
if (status != CFE_SUCCESS)
{
CFE_EVS_SendEvent(TO_LAB_TBL_ERR_EID, CFE_EVS_EventType_ERROR, "L%d TO Can't load table status %i", __LINE__,
(int)status);
return status;
}
status = CFE_TBL_GetAddress((void **)&TblPtr, TO_LAB_Global.SubsTblHandle);
if (status != CFE_SUCCESS && status != CFE_TBL_INFO_UPDATED)
{
CFE_EVS_SendEvent(TO_LAB_TBL_ERR_EID, CFE_EVS_EventType_ERROR, "L%d TO Can't get table addr status %i",
__LINE__, (int)status);
return status;

Also, the current logic doesn't contain early returns or status-guards after the first call to CFE_SB_CreatePipe, so subsequent initialization steps are attempted even if previous steps failed, and successful initialization is reported even if no pipes were created:

to_lab/fsw/src/to_lab_app.c

Lines 161 to 206 in ec09026

status = CFE_SB_CreatePipe(&TO_LAB_Global.Cmd_pipe, PipeDepth, PipeName);
if (status == CFE_SUCCESS)
{
CFE_SB_Subscribe(CFE_SB_ValueToMsgId(TO_LAB_CMD_MID), TO_LAB_Global.Cmd_pipe);
CFE_SB_Subscribe(CFE_SB_ValueToMsgId(TO_LAB_SEND_HK_MID), TO_LAB_Global.Cmd_pipe);
}
else
CFE_EVS_SendEvent(TO_LAB_CR_PIPE_ERR_EID, CFE_EVS_EventType_ERROR, "L%d TO Can't create cmd pipe status %i",
__LINE__, (int)status);
/* Create TO TLM pipe */
status = CFE_SB_CreatePipe(&TO_LAB_Global.Tlm_pipe, ToTlmPipeDepth, ToTlmPipeName);
if (status != CFE_SUCCESS)
{
CFE_EVS_SendEvent(TO_LAB_TLMPIPE_ERR_EID, CFE_EVS_EventType_ERROR, "L%d TO Can't create Tlm pipe status %i",
__LINE__, (int)status);
}
/* Subscriptions for TLM pipe*/
SubEntry = TO_LAB_Global.SubsTblPtr->Subs;
for (i = 0; i < TO_LAB_MAX_SUBSCRIPTIONS; i++)
{
if (!CFE_SB_IsValidMsgId(SubEntry->Stream))
{
/* Only process until invalid MsgId is found*/
break;
}
status = CFE_SB_SubscribeEx(SubEntry->Stream, TO_LAB_Global.Tlm_pipe, SubEntry->Flags, SubEntry->BufLimit);
if (status != CFE_SUCCESS)
{
CFE_EVS_SendEvent(TO_LAB_SUBSCRIBE_ERR_EID, CFE_EVS_EventType_ERROR,
"L%d TO Can't subscribe to stream 0x%x status %i", __LINE__,
(unsigned int)CFE_SB_MsgIdToValue(SubEntry->Stream), (int)status);
}
++SubEntry;
}
/*
** Install the delete handler
*/
OS_TaskInstallDeleteHandler(&TO_LAB_delete_callback);
CFE_EVS_SendEvent(TO_LAB_INIT_INF_EID, CFE_EVS_EventType_INFORMATION,
"TO Lab Initialized.%s, Awaiting enable command.", TO_LAB_VERSION_STRING);

Expected behavior
Single exit point.

Reporter Info
Avi Weiss   @thnkslprpt

thnkslprpt added a commit to thnkslprpt/to_lab that referenced this issue Dec 11, 2023
thnkslprpt added a commit to thnkslprpt/to_lab that referenced this issue Dec 12, 2023
dzbaker added a commit that referenced this issue Oct 3, 2024
…improvements

Fix #179, Light initialization logic refactor + remove multiple returns
@dzbaker dzbaker closed this as completed in e8f3dcf Oct 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant