fix: cppcheck identicalInnerCondition and uninitvar findings - #1204
Conversation
Four sites re-tested an unchanged ret after a WOLFSSH_SMALL_STACK allocation, which cppcheck reports as identicalInnerCondition because the allocation that can change ret is compiled out in the default build. Test the allocated pointer instead; that is what the check is guarding. - certman.c, keygen.c and wolfsshd/auth.c: check the DecodedCert, MlDsaKey and DecodedCert pointers. - internal.c CompositeEccSign: give the fixed-buffer build pointer aliases so both builds have the same shape, then check the r/s pointers. - port.c: initialize fileHandle, which is only assigned when mbstowcs_s succeeds. - wolfsftp.c: zero localTime before WLOCALTIME, which is a per-port macro.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1204
Scan targets checked: wolfssh-bugs, wolfssh-src
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Fixes new and existing cppcheck findings (identicalInnerCondition + uninitvar) by adjusting small-stack allocation guards, initializing an uninitialized Windows handle, and making time conversion use a zeroed struct to avoid stale reads across ports.
Changes:
- Replace
ret-based inner guards with pointer-based guards for small-stack allocations in several call sites. - Initialize
fileHandlein Windows-onlyport.cto avoid potential use of an uninitialized handle. - Zero
localTimebeforeWLOCALTIME()to avoid stale reads when port macros don’t fully populate the struct.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/wolfsftp.c | Zero-initialize localTime before calling per-port WLOCALTIME() macro. |
| src/port.c | Initialize Windows HANDLE fileHandle to INVALID_HANDLE_VALUE. |
| src/keygen.c | Switch from ret-guard to pointer-guard around MlDsaKey usage after small-stack allocation. |
| src/internal.c | Make fixed-buffer build use pointer aliases and switch guard to pointer checks for r/s buffers. |
| src/certman.c | Switch from ret-guard to pointer-guard around DecodedCert usage after small-stack allocation. |
| apps/wolfsshd/auth.c | Switch from ret-guard to pointer-guard around DecodedCert usage after small-stack allocation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The cppcheck CI job reports three identicalInnerCondition warnings and one
uninitvar error. A newer cppcheck (2.21) adds two more of the same two
kinds; both sets are fixed here.
Four sites re-tested an unchanged ret after a WOLFSSH_SMALL_STACK
allocation. cppcheck calls the inner check dead because the allocation
that can change ret is compiled out in the default build. They now test
the allocated pointer, which is what the check guards.
and DecodedCert pointers
so both builds have the same shape, then checks the r/s pointers
succeeds
cppcheck cannot see through with only std.cfg loaded