-
Notifications
You must be signed in to change notification settings - Fork 25
Fixes scan-build issues #174
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
Conversation
JacobBarthelmeh
left a comment
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.
Overall looks great. Only thing that stood out to me is the loss of capturing return values. I think we should act on the return values if scan-build is complaining about them not being used, or the internal function should be changed to a void return if no return values are expected.
|
@miyazakh looks like there are merge conflicts. Can you please address my comments as well as resolve them. I also would like to see |
6713d08 to
bb24508
Compare
1e376cb to
d49d232
Compare
|
|
@miyazakh Should be fixed if you rebase on main, sorry about that! |
d49d232 to
20d3f1a
Compare
billphipps
left a comment
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.
Looks good! A few minor suggestions and a couple of questions. This should be merged prior to #158.
52f9be3 to
6e5cac2
Compare
806a491 to
3ea1f22
Compare
billphipps
left a comment
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.
Lots of excellent fixes and finds! Thank you!
I recommend we ignore some of the error returns as in the original code. Let me know if you agree or if we should find an alternate path on how to handle secondary errors.
I think exiting early during reading the NVM directory objects is an error and must be corrected.
6c01197 to
52dfdb3
Compare
52dfdb3 to
1bbcd35
Compare
|
@billphipps please merge when you are ok with it |
billphipps
left a comment
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.
Looks great! Thanks!
Fixes issues on scan-build LLVM-19
There is one issue, which is
warning: Value stored to 'ret' is never readinwolfcrypt\src\random.cagainsttools/whnvmtool/, remaining. That will be separately fixed.Github action addition for changes in the future will be submitted as separate PR.