Drop dead code found by Coverity - #1268
Open
ejohnstown wants to merge 2 commits into
Open
ejohnstown wants to merge 2 commits into
ejohnstown wants to merge 2 commits into
Conversation
wolfSSH_ProcessBuffer() rejects any format outside ASN1, PEM, RAW, and OPENSSH before the decode chain runs, so the chain's trailing else could never be reached. PEM becomes the chain's default arm, which keeps derSz definitely assigned on every path into the decrypt step. Issue: CID-653252
The done: block in test_SendChannelTerminalRequestNoTty() is only entered with a descriptor still open on a path that already set an error code, so the result checks guarding -1494 and -1495 could never be true. Restore the descriptors and keep the original code.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes remove confirmed unreachable/dead code without altering reachable behavior, and the surrounding validation/cleanup logic remains consistent.
Review effort: Lite
Findings: None
What changed in this PR
Removes two Coverity-identified dead-code paths: an unreachable decode-chain “reject” branch in wolfSSH_ProcessBuffer() and dead stores in a unit test cleanup path.
Changes:
src/internal.c: Treat PEM as the implicit remaining format after the existing upfront format validation, and drop the unreachableWS_UNIMPLEMENTED_Ereturn.tests/unit.c: Remove ineffectivedup2()failure-to-result assignments in adone:cleanup path whereresult == 0cannot occur.
| File | Description |
|---|---|
| tests/unit.c | Simplifies cleanup error handling by removing dead stores while preserving descriptor restoration behavior. |
| src/internal.c | Removes an unreachable return by relying on existing format validation and making PEM the decode-chain default arm. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
wolfSSH_ProcessBuffer() validates its format argument before the decode chain runs, so the chain's trailing reject could never be reached. An unrelated dead store in the terminal-request test goes along with it.