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

LC_TableInit has an if/else if without an else clause and its behavior is undefined #12

Closed
skliper opened this issue Apr 22, 2022 · 1 comment
Labels

Comments

@skliper
Copy link
Contributor

skliper commented Apr 22, 2022

So at the end of the LC_TableInit function there is an odd branching

if (LC_CDS_ENABLED)
{
if (LC_CDS_RESTORED)
{
restored event
}
else if (LC_CDS_UPDATED)
{
default event
}
// nothing else here !!!
}
else
{
CDS disabled event
}

So, the question is: Can we have a scenario where LC_CDS_ENABLED is TRUE, but both LC_CDS_RESTORED and LC_CDS_UPDATED are FALSE? And IF SO: What is the desired behavior here?? This is a situation where the lack of an else clause on an "else if" most definitively is cause for concern. If that scenario cannot exist, then it would seem an "else if" is not required. Unfortunately, due to the convoluted nature of this function and those that it calls, it is would be difficult to determine if the above scenario is a possibility.

Imported from GSFCCFS-1104

@skliper
Copy link
Contributor Author

skliper commented Jun 14, 2022

Code link:

LC/fsw/src/lc_app.c

Lines 467 to 489 in bb91036

/*
** 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);
}

Behavior is not "undefined", it's behaving as intended (send an event if any of the cases match). The convoluted nature of the routine is a separate issue (#36)

@skliper skliper closed this as completed Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant