Skip to content
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

A couple of coverity fixes #13064

Merged
merged 3 commits into from
Oct 17, 2019
Merged

A couple of coverity fixes #13064

merged 3 commits into from
Oct 17, 2019

Conversation

julianoes
Copy link
Contributor

Coverity sends me these emails about potential problems. In order to get to inbox-zero I had to attempt to fix some of them.

This might fix a warning about argv being a TAINTED_SCALAR further down.
@julianoes julianoes requested a review from sfuhrer October 1, 2019 11:54

select_airspeed_and_publish();
select_airspeed_and_publish();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

select_airspeed_and_publish() should also be executed if no airspeed sensor is connected, so with this change I think it wouldn't work.
Inside select_airspeed_and_publish, _airspeed_validator is used yes, but only if the number of airspeed sensors is >0, in which case _airspeed_validator is != nullptr. Probably not the nicest way to program it, but should be safe, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha. However, there is still one catch: if new fails to allocate because you're out of RAM _airspeed_validator is also null even though _number_of_airspeed_sensors is > 0. We need some kind of mechanism to check for that failure case and notify that airspeed is broken and we must not fly.

@dagar
Copy link
Member

dagar commented Oct 14, 2019

Perhaps @sfuhrer could account for the airspeed selector coverity fixes in #12887 and get the others in here?

@sfuhrer
Copy link
Contributor

sfuhrer commented Oct 15, 2019

Perhaps @sfuhrer could account for the airspeed selector coverity fixes in #12887 and get the others in here?

Yes, I'll do that. @julianoes you can remove the 4 commits concerning the airspeed selector, I'll handle it in #12887.

@julianoes
Copy link
Contributor Author

@sfuhrer you can change this branch and do interactive rebase as you want.

@sfuhrer
Copy link
Contributor

sfuhrer commented Oct 17, 2019

@sfuhrer you can change this branch and do interactive rebase as you want.

Done. This PR is then ready to be merged I'd say.

@sfuhrer sfuhrer self-requested a review October 17, 2019 12:18
@julianoes
Copy link
Contributor Author

Thanks @sfuhrer!

@dagar dagar merged commit 2fcddd9 into master Oct 17, 2019
@dagar dagar deleted the pr-coverity-fixes branch October 17, 2019 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants