Skip to content

Conversation

@thnkslprpt
Copy link
Contributor

Checklist

Describe 2the contribution

Testing performed
GitHub CI actions all passing successfully (incl. Build + Run, Unit/Functional Tests etc.).
Note: Most of the added default case lines are not tested (impossible to test given that all cases are handled explicitly) so the number of missed lines has grown larger.

Expected behavior changes
Effectively no change to current behavior.
Ensuring switch statements are well-formed is good defensive coding for future code changes and improves consistency across cFE.

System(s) tested on
Debian GNU/Linux 11 (bullseye)
Current main branch of cFS bundle.

Contributor Info
Avi Weiss   @thnkslprpt

@thnkslprpt
Copy link
Contributor Author

I can also add a comment in for each default if it's worthwhile - something like:
/* Not currently implemented - should never get here as all cases are handled explicitly. */

@thnkslprpt thnkslprpt changed the title Fix #2478, Add missing deafult/break to switch statements Fix #2478, Add missing default/break to switch statements Jan 25, 2024
@thnkslprpt thnkslprpt force-pushed the fix-2478-add-missing-break-default-to-switches branch 2 times, most recently from 0f24968 to b292fe6 Compare January 25, 2024 22:12
@thnkslprpt thnkslprpt force-pushed the fix-2478-add-missing-break-default-to-switches branch 2 times, most recently from 5a464f5 to a7654a2 Compare March 23, 2024 10:51
Copy link
Contributor

@jphickey jphickey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should defer this one until we also add coverage tests to get the default cases. Otherwise it makes the missed lines metric go up. (agreed that there is no actual change or new missed code there, it is just now being reported as such)

Some of them might be unreachable, which brings up the question of which of the opposing coding standards we want to violate (i.e. no unreachable lines or having the default case always in the switch)

@thnkslprpt thnkslprpt force-pushed the fix-2478-add-missing-break-default-to-switches branch 3 times, most recently from c9d4a3e to b775219 Compare November 15, 2025 11:44
@thnkslprpt thnkslprpt force-pushed the fix-2478-add-missing-break-default-to-switches branch from b775219 to 6becb5f Compare November 15, 2025 11:52
@thnkslprpt
Copy link
Contributor Author

@jphickey in the end it was only 2 problematic cases where the default is unreachable - namely CFE_SB_EnableRouteCmd() and CFE_SB_DisableRouteCmd().

So I rebased this to main and it looks good now. Lots of nice typo fixes as well.

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 this pull request may close these issues.

Several switch statements in cFE are missing a default and/or break

2 participants