-
Notifications
You must be signed in to change notification settings - Fork 332
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 an unused variable issue in nxt_term_parse() as found by clang-analyzer and coverity #1441
Merged
Conversation
This file contains 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
Member
ac000
commented
Sep 23, 2024
•
edited
Loading
edited
hongzhidao
approved these changes
Sep 24, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job.
I feel the first commit looks good and sufficient, and the subsequent changes can be discarded.
ac000
changed the title
Do some cleanups in nxt_term_parse() (fixing a clang-analyzer (and coverity) issue)
Fix an unused variable issue in nxt_term_parse() as found by clang-analyzer and coverity
Sep 24, 2024
|
hongzhidao
approved these changes
Sep 24, 2024
Thanks @hongzhidao ! |
Both clang-analyzer and coverity flagged an issue in nxt_term_parse() that we set 'state = st_letter' but then set it to 'state = st_space' before using it. While we could simply remove the first assignment and placate the analyzers, upon further analysis it seems that there is some more cleanup that could be done in this function. This commit addresses the above issue, subsequent commits will continue the cleanup. To solve the unused assignment issue we can get rid of the 'state == st_letter' assignment and unconditionally execute the code that was behind the if (state != st_letter) { guard. If we're not handling a space then we should have either a digit or letter. Also, perhaps more importantly, this if () statement would never be false at this point as state would never == st_letter. We may as well also remove the st_letter enum value. The src/test/nxt_term_parse_test.c still passes tests: [notice] term parse test passed NOTE: Although this function is not currently used in Unit (only by src/test/nxt_term_parse_test.c), it is probably worth cleaning it up and solving one of the open clang-analyzer (and coverity) issues. Signed-off-by: Andrew Clayton <[email protected]>
The function nxt_term_parse() is able to take strings with trailing whitespace e.g. "1w1d ", add a test case to cover such things. Signed-off-by: Andrew Clayton <[email protected]>
Rebased with master
|
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.