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

Fix #793, Remove unreachable code in OS_SocketOpen_Impl for BSD socket #802

Merged

Conversation

skliper
Copy link
Contributor

@skliper skliper commented Feb 11, 2021

Describe the contribution
Fix #793, Just simplifies switch statements based on previous checks. Can always be expanded again if additional cases are implemented.

Testing performed
Build and execute unit tests, passed

Expected behavior changes
No impact to behavior

System(s) tested on

  • Hardware: cFS Dev Server
  • OS: Ubuntu 18.04
  • Versions: cFS Bundle main + this commit

Additional context
Static analysis warning fix

Third party code
None

Contributor Info - All information REQUIRED for consideration of pull request
Jacob Hageman - NASA/GSFC

@skliper skliper added this to the 6.0.0 milestone Feb 11, 2021
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.

I'm a little reluctant on this one - a switch() with an empty default case is a common paradigm where code is anticipated to be extended in the future.

Is this saying that the tool sees an empty default case as unreachable code?

Although I agree this doesn't change current behavior - it does make it more likely that the next time someone updates this it gets broken - the switch was there for future proofing.

@skliper
Copy link
Contributor Author

skliper commented Feb 12, 2021

Yeah, unreachable case. I get the future proof... but I'd prefer to add that back in if/when needed. Unless we can refactor to avoid dead code.

In my book dead code avoidance trumps "default case for all switches".

@astrogeco astrogeco changed the base branch from main to integration-candidate February 12, 2021 20:35
@astrogeco astrogeco merged commit c878647 into nasa:integration-candidate Feb 12, 2021
@skliper skliper deleted the fix793-socket-dead-code branch April 1, 2021 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unreachable code in os-impl-bsd-sockets.c:
3 participants