-
Notifications
You must be signed in to change notification settings - Fork 91
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
Address some Static Analysis Issues #519 #521
Conversation
Signed-off-by: Norman Ashley <[email protected]>
Signed-off-by: Norman Ashley <[email protected]>
Signed-off-by: Norman Ashley <[email protected]>
Signed-off-by: Norman Ashley <[email protected]>
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.
These changes all make sense and improve code safety. Thanks @ashman-p !
Thanks @baentsch, i am not sure if i needed another approval, so adding a few more reviewers. |
No, 1 review is sufficient in a project with as many maintainers :-) Do you have permissions to merge? |
@ashman-p Is it possible that you merged this without squashing? The commit history at least got substantially longer that I'd figure it should be. If no, please do so in the future to keep things together that belong together. Thanks! |
I think that was the case. I will remember to squash be the merge. |
Addresses some static analysis errors reported by Coverity tool.