Merged
Conversation
Contributor
ejohnstown
commented
Apr 21, 2026
ejohnstown
commented
- Fix potential double-free: Null out a pointer after freeing in wolfSSH_CTX_UseCert_buffer() test to prevent a double-free if the second cert load fails.
- Fix scan-build errno warnings: Replace rewind() with fseek() in both the WREWIND macro and api.c test helper to avoid scan-build flagging the implicit errno clear, and remove a redundant errno check after execv().
- Fix unchecked return value warning: Handle the return value of dup() in regress.c to silence the static analysis warning.
- 7-bit clean ASCII: Scrub non-ASCII characters (curly quotes, n-dashes) from the main source files to ensure all text is 7-bit clean.
Static analysis indicated the potential to double-free a pointer in the `wolfSSH_CTX_UseCert_buffer()` test. Loading the second cert after freeing the first cert could fail and the test cleanup would free the pointer again. Just set pointer to null after freeing.
Clean up the main set of files to be 7-bit clean ASCII. There were many single and double-quotes and n-dashes.
Remove the errno check in the condition after execv(). If execv() returns at all, it has failed, so checking ret alone is sufficient. The errno check was flagged by clang scan-build as potentially reading an undefined value (unix.Errno).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a set of static-analysis findings and consistency issues across the wolfSSH codebase, primarily around file handling, error checks, and ensuring 7-bit clean ASCII in source/test text.
Changes:
- Replaces
rewind()usage withfseek()in the portability layer and a test helper to avoid scan-build errno warnings. - Tightens test robustness by preventing potential double-free patterns and asserting
dup()success. - Scrubs non-ASCII punctuation from source/comments/docs and removes a redundant
errnocheck afterexecv().
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| wolfssh/port.h | Changes WREWIND implementation from rewind() to fseek(..., SEEK_SET) |
| tests/regress.c | Adds an assertion for dup(STDIN_FILENO) return value |
| tests/auth.c | Replaces non-ASCII dashes in comments with ASCII hyphens |
| tests/api.c | Replaces rewind() with fseek() in load_file() and nulls freed cert pointer in a test |
| scripts/fwd.test.expect | Replaces non-ASCII em-dash with ASCII hyphen in expected output |
| examples/sftpclient/sftpclient.c | Replaces non-ASCII punctuation in comments |
| apps/wolfsshd/wolfsshd.c | Removes redundant && errno check after execv() |
| apps/wolfsshd/test/test_configuration.c | Replaces non-ASCII punctuation in comments |
| ChangeLog.md | Replaces curly quotes / non-ASCII punctuation with ASCII equivalents |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace rewind() with fseek(s, 0, SEEK_SET) in the WREWIND macro. rewind() internally clears errno, which scan-build flags as overwriting a previously unchecked errno value. fseek() to offset 0 is functionally equivalent without the implicit errno reset.
Replace rewind() with fseek(f, 0, SEEK_SET) in the test helper function. rewind() internally clears errno, which scan-build flags as overwriting a previously unchecked errno value. fseek() to offset 0 is functionally equivalent without the implicit errno reset.
padelsbach
approved these changes
Apr 22, 2026
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.