-
-
Notifications
You must be signed in to change notification settings - Fork 441
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
Coverity fixes #1296
Coverity fixes #1296
Conversation
ab43e0e
to
99b5b40
Compare
I remember you argued about "non-null-pointer-assert" being just "line noise in the code". |
Yes, that's correct. That's not what this change is doing. By NULL-pointer-assert we refer to the following ...
In this scenario, the assert line is not useful (IMO) because the very next line of code performs the same function, for both debug and non-debug builds. |
@natoscott Well, I mean while some assert lines are not useful by themselves, they might convey the intent of the programmers. This way I can see when the lack of null pointer check is intentional, and not a developer's oversight. I know the proper way to mark the pointer as being non-null is to use |
Sure, I understand your POV. A comment can often be a better way to go in that situation though. |
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.
LGTM. Some minor changes still pending.
Coverity scanning shows we end up passing an integer into the Row_setPidColumnWidth routine which requires a pid_t - update each platform to return the correct type (and never return -1 as a failure code, this was being ignored).
Coverity scanning shows a couple of locations where a NULL pointer dereference could happen; updated code to ensure it cannot.
Coverity scanning struggles to follow our 'super' handling under certain circumstances. Adjust the code to ensure we keep good coverage even though this is a false positive.
Coverity scanning rightly warns that a usually-checked return status was being ignored in two locations. These two are in cases where it's safe to ignore, so I've adjusted the code to maintain coverage even though they are false positives here.
99b5b40
to
eb86bdf
Compare
No description provided.