Multiple return statements in LC_CreateTaskCDS and LC_TableInit

[skliper]

Checklist (Please check before submitting)

• I reviewed the Contributing Guide.
• I reviewed the README file to see if the feature is in the major future work.
• I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
Multiple returns are a coding style violation, and the implementation is challenging to follow (see #12 and others).

LC/fsw/src/lc_app.c

Lines 332 to 493 in bb91036

	int32 LC_TableInit(void)
	{
	int32 Result;
	/*
	** LC task use of Critical Data Store (CDS)
	**
	** Global application data (LC_AppData)
	** Watchpint results dump only table data
	** Actionpoint results dump only table data
	**
	** cFE Table Services use of CDS for LC task
	**
	** Watchpint definition loadable table data
	** Actionpoint definition loadable table data
	**
	** LC table initialization logic re CDS
	**
	** If LC cannot create all the CDS storage at startup, then LC
	** will disable LC use of CDS and continue.
	**
	** If LC cannot register definition tables as critical, then LC
	** will disable LC use of CDS and re-register tables as non-critical.
	**
	** If LC cannot register definition and results tables at startup,
	** then LC will terminate - table use is a required function.
	**
	** If LC can create all the CDS storage and register definition
	** tables as critical, then LC will write to CDS regardless of
	** whether LC was able to read from CDS at startup.
	**
	** If LC cannot restore everything from CDS at startup, then LC
	** will initialize everything - load default definition tables,
	** init results table contents, init global application data.
	*/
	/* lc_platform_cfg.h */
	#ifdef LC_SAVE_TO_CDS
	LC_OperData.HaveActiveCDS = true;
	#endif
	/*
	** Maintain a detailed record of table initialization results
	*/
	if (LC_OperData.HaveActiveCDS)
	{
	LC_OperData.TableResults |= LC_CDS_ENABLED;
	}
	/*
	** Create watchpoint and actionpoint result tables
	*/
	if ((Result = LC_CreateResultTables()) != CFE_SUCCESS)
	{
	return (Result);
	}
	/*
	** If CDS is enabled - create the 3 CDS areas managed by the LC task
	** (continue with init, but disable CDS if unable to create all 3)
	*/
	if (LC_OperData.HaveActiveCDS)
	{
	if (LC_CreateTaskCDS() != CFE_SUCCESS)
	{
	LC_OperData.HaveActiveCDS = false;
	}
	}
	/*
	** Create wp/ap definition tables - critical if CDS enabled
	*/
	if ((Result = LC_CreateDefinitionTables()) != CFE_SUCCESS)
	{
	return (Result);
	}
	/*
	** CDS still active only if we created 3 CDS areas and 2 critical tables
	*/
	if (LC_OperData.HaveActiveCDS)
	{
	LC_OperData.TableResults |= LC_CDS_CREATED;
	}
	/*
	** If any CDS area or critical table is not restored - initialize everything.
	** (might be due to reset type, CDS disabled or corrupt, table restore error)
	*/
	if (((LC_OperData.TableResults & LC_WRT_CDS_RESTORED) == LC_WRT_CDS_RESTORED) &&
	((LC_OperData.TableResults & LC_ART_CDS_RESTORED) == LC_ART_CDS_RESTORED) &&
	((LC_OperData.TableResults & LC_APP_CDS_RESTORED) == LC_APP_CDS_RESTORED) &&
	((LC_OperData.TableResults & LC_WDT_TBL_RESTORED) == LC_WDT_TBL_RESTORED) &&
	((LC_OperData.TableResults & LC_ADT_TBL_RESTORED) == LC_ADT_TBL_RESTORED))
	{
	LC_OperData.TableResults |= LC_CDS_RESTORED;
	/*
	** Get a pointer to the watchpoint definition table data...
	*/
	Result = CFE_TBL_GetAddress((void *)&LC_OperData.WDTPtr, LC_OperData.WDTHandle);
	if ((Result != CFE_SUCCESS) && (Result != CFE_TBL_INFO_UPDATED))
	{
	CFE_EVS_SendEvent(LC_WDT_GETADDR_ERR_EID, CFE_EVS_EventType_ERROR, "Error getting WDT address, RC=0x%08X",
	(unsigned int)Result);
	return (Result);
	}
	/*
	** Get a pointer to the actionpoint definition table data
	*/
	Result = CFE_TBL_GetAddress((void *)&LC_OperData.ADTPtr, LC_OperData.ADTHandle);
	if ((Result != CFE_SUCCESS) && (Result != CFE_TBL_INFO_UPDATED))
	{
	CFE_EVS_SendEvent(LC_ADT_GETADDR_ERR_EID, CFE_EVS_EventType_ERROR, "Error getting ADT address, RC=0x%08X",
	(unsigned int)Result);
	return (Result);
	}
	}
	else
	{
	Result = LC_LoadDefaultTables();
	if ((Result != CFE_SUCCESS) && (Result != CFE_TBL_INFO_UPDATED))
	{
	return (Result);
	}
	}
	/*
	** Create watchpoint hash tables -- also subscribes to watchpoint packets
	*/
	LC_CreateHashTable();
	/*
	** Display results of CDS initialization (if enabled at startup)
	*/
	if ((LC_OperData.TableResults & LC_CDS_ENABLED) == LC_CDS_ENABLED)
	{
	if ((LC_OperData.TableResults & LC_CDS_RESTORED) == LC_CDS_RESTORED)
	{
	CFE_EVS_SendEvent(LC_CDS_RESTORED_INF_EID, CFE_EVS_EventType_INFORMATION,
	"Previous state restored from Critical Data Store");
	}
	else if ((LC_OperData.TableResults & LC_CDS_UPDATED) == LC_CDS_UPDATED)
	{
	CFE_EVS_SendEvent(LC_CDS_UPDATED_INF_EID, CFE_EVS_EventType_INFORMATION,
	"Default state loaded and written to CDS, activity mask = 0x%08X",
	(unsigned int)LC_OperData.TableResults);
	}
	}
	else
	{
	CFE_EVS_SendEvent(LC_CDS_DISABLED_INF_EID, CFE_EVS_EventType_INFORMATION,
	"LC use of Critical Data Store disabled, activity mask = 0x%08X",
	(unsigned int)LC_OperData.TableResults);
	}
	return (CFE_SUCCESS);
	} /* LC_TableInit() */

LC/fsw/src/lc_app.c

Lines 735 to 855 in bb91036

	int32 LC_CreateTaskCDS(void)
	{
	int32 Result;
	uint32 DataSize;
	/*
	** Create CDS and try to restore Watchpoint Results Table (WRT) data
	*/
	DataSize = LC_MAX_WATCHPOINTS * sizeof(LC_WRTEntry_t);
	Result = CFE_ES_RegisterCDS(&LC_OperData.WRTDataCDSHandle, DataSize, LC_WRT_CDSNAME);
	if (Result == CFE_SUCCESS)
	{
	/*
	** Normal result after a power on reset (cold boot) - continue with next CDS area
	*/
	LC_OperData.TableResults |= LC_WRT_CDS_CREATED;
	}
	else if (Result == CFE_ES_CDS_ALREADY_EXISTS)
	{
	/*
	** Normal result after a processor reset (warm boot) - try to restore previous data
	*/
	LC_OperData.TableResults |= LC_WRT_CDS_CREATED;
	Result = CFE_ES_RestoreFromCDS(LC_OperData.WRTPtr, LC_OperData.WRTDataCDSHandle);
	if (Result == CFE_SUCCESS)
	{
	LC_OperData.TableResults |= LC_WRT_CDS_RESTORED;
	}
	}
	else
	{
	CFE_EVS_SendEvent(LC_WRT_CDS_REGISTER_ERR_EID, CFE_EVS_EventType_ERROR,
	"Error registering WRT CDS Area, RC=0x%08X", (unsigned int)Result);
	return (Result);
	}
	/*
	** Create CDS and try to restore Actionpoint Results Table (ART) data
	*/
	DataSize = LC_MAX_ACTIONPOINTS * sizeof(LC_ARTEntry_t);
	Result = CFE_ES_RegisterCDS(&LC_OperData.ARTDataCDSHandle, DataSize, LC_ART_CDSNAME);
	if (Result == CFE_SUCCESS)
	{
	/*
	** Normal result after a power on reset (cold boot) - continue with next CDS area
	*/
	LC_OperData.TableResults |= LC_ART_CDS_CREATED;
	}
	else if (Result == CFE_ES_CDS_ALREADY_EXISTS)
	{
	/*
	** Normal result after a processor reset (warm boot) - try to restore previous data
	*/
	LC_OperData.TableResults |= LC_ART_CDS_CREATED;
	Result = CFE_ES_RestoreFromCDS(LC_OperData.ARTPtr, LC_OperData.ARTDataCDSHandle);
	if (Result == CFE_SUCCESS)
	{
	LC_OperData.TableResults |= LC_ART_CDS_RESTORED;
	}
	}
	else
	{
	CFE_EVS_SendEvent(LC_ART_CDS_REGISTER_ERR_EID, CFE_EVS_EventType_ERROR,
	"Error registering ART CDS Area, RC=0x%08X", (unsigned int)Result);
	return (Result);
	}
	/*
	** Create CDS and try to restore Application (APP) data
	*/
	DataSize = sizeof(LC_AppData_t);
	Result = CFE_ES_RegisterCDS(&LC_OperData.AppDataCDSHandle, DataSize, LC_APPDATA_CDSNAME);
	if (Result == CFE_SUCCESS)
	{
	/*
	** Normal result after a power on reset (cold boot) - continue with next CDS area
	*/
	LC_OperData.TableResults |= LC_APP_CDS_CREATED;
	}
	else if (Result == CFE_ES_CDS_ALREADY_EXISTS)
	{
	/*
	** Normal result after a processor reset (warm boot) - try to restore previous data
	*/
	LC_OperData.TableResults |= LC_APP_CDS_CREATED;
	Result = CFE_ES_RestoreFromCDS(&LC_AppData, LC_OperData.AppDataCDSHandle);
	if ((Result == CFE_SUCCESS) && (LC_AppData.CDSSavedOnExit == LC_CDS_SAVED))
	{
	/*
	** Success - only if previous session saved CDS data at least once
	*/
	LC_OperData.TableResults |= LC_APP_CDS_RESTORED;
	/*
	** May need to override the restored application state
	*/
	#if LC_STATE_WHEN_CDS_RESTORED != LC_STATE_FROM_CDS
	LC_AppData.CurrentLCState = LC_STATE_WHEN_CDS_RESTORED;
	#endif
	}
	}
	else
	{
	CFE_EVS_SendEvent(LC_APP_CDS_REGISTER_ERR_EID, CFE_EVS_EventType_ERROR,
	"Error registering application data CDS Area, RC=0x%08X", (unsigned int)Result);
	return (Result);
	}
	return (CFE_SUCCESS);
	} /* LC_CreateTaskCDS() */

Describe the solution you'd like
Single entry/exit point from functions. Refactor to simplify flow.

Describe alternatives you've considered
None

Additional context
Could group w/ multiple issues or tackle by file.

Requester Info
Jacob Hageman - NASA/GSFC

[skliper]

As part of LC_TableInit, consider reducing duplicated logic and EID use for getting the WDTPtr and ADTPtr:

LC/fsw/src/lc_app.c

Lines 429 to 450 in bb91036

	/*
	** Get a pointer to the watchpoint definition table data...
	*/
	Result = CFE_TBL_GetAddress((void *)&LC_OperData.WDTPtr, LC_OperData.WDTHandle);
	if ((Result != CFE_SUCCESS) && (Result != CFE_TBL_INFO_UPDATED))
	{
	CFE_EVS_SendEvent(LC_WDT_GETADDR_ERR_EID, CFE_EVS_EventType_ERROR, "Error getting WDT address, RC=0x%08X",
	(unsigned int)Result);
	return (Result);
	}
	/*
	** Get a pointer to the actionpoint definition table data
	*/
	Result = CFE_TBL_GetAddress((void *)&LC_OperData.ADTPtr, LC_OperData.ADTHandle);
	if ((Result != CFE_SUCCESS) && (Result != CFE_TBL_INFO_UPDATED))
	{
	CFE_EVS_SendEvent(LC_ADT_GETADDR_ERR_EID, CFE_EVS_EventType_ERROR, "Error getting ADT address, RC=0x%08X",
	(unsigned int)Result);
	return (Result);

LC/fsw/src/lc_app.c

Lines 890 to 896 in bb91036

	Result = CFE_TBL_GetAddress((void *)&LC_OperData.WDTPtr, LC_OperData.WDTHandle);
	if ((Result != CFE_SUCCESS) && (Result != CFE_TBL_INFO_UPDATED))
	{
	CFE_EVS_SendEvent(LC_WDT_GETADDR_ERR_EID, CFE_EVS_EventType_ERROR, "Error getting WDT address, RC=0x%08X",
	(unsigned int)Result);
	}

LC/fsw/src/lc_app.c

Lines 925 to 931 in bb91036

	Result = CFE_TBL_GetAddress((void *)&LC_OperData.ADTPtr, LC_OperData.ADTHandle);
	if ((Result != CFE_SUCCESS) && (Result != CFE_TBL_INFO_UPDATED))
	{
	CFE_EVS_SendEvent(LC_ADT_GETADDR_ERR_EID, CFE_EVS_EventType_ERROR, "Error getting ADT address, RC=0x%08X",
	(unsigned int)Result);
	}

[thnkslprpt]

Submitted a PR just focusing on LC_TableInit().
If that is approved/merged I will open a new issue for LC_CreateTaskCDS().