Skip to content

Coverity Unused values Issues #5763

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

JohnAFernandez
Copy link
Contributor

Some coverity issues in the "Unused Values" category. In draft because I need to see this patch's effect on targeting large ship subsystems.

@JohnAFernandez JohnAFernandez added coverity An issue flagged by Coverity fix A fix for bugs, not-a-bugs, and/or regressions. labels Nov 4, 2023
@JohnAFernandez JohnAFernandez self-assigned this Nov 4, 2023
@JohnAFernandez JohnAFernandez added the sanitizer/stability Issues caught by Address Sanitizer or other third-party tools. label Nov 4, 2023
Since we are explicitly setting the exact flag values.

For coverity 1320526
Since it would be overwritten next line anyway.

For coverity 1320527
For coverity 1523417
This was originally

`glBlendFunc(GL_ONE, GL_ZERO);`
`blendfunc_Value[0] = GL_ONE;`
`blendfunc_Value[1] = GL_ZERO;`

Marked by coverity 1523247
So that linters and coverity don't complain about the initial value being overwritten.

Coverity 1320531
In hud_lock_acquire_current_target, because we didn't need a pointer here, we needed a pointer of a pointer.

For coverity 1522999
This is just another cleanup pointed out by coverity 1523154
This cleans up the code and keeps us from thinking that target_ship_obj is needed.  This is a retail issue.

Marked by coverity 1320528
@JohnAFernandez JohnAFernandez marked this pull request as ready for review December 11, 2023 22:45
@JohnAFernandez
Copy link
Contributor Author

Game seems unaffected (as expected) from tests.

@JohnAFernandez JohnAFernandez added the Waiting for Stable Marks a pull request that is to be merged after the next stable release, due to a release cycle label Jan 24, 2024
@Goober5000 Goober5000 removed the Waiting for Stable Marks a pull request that is to be merged after the next stable release, due to a release cycle label Feb 26, 2024
Goober5000
Goober5000 previously approved these changes Apr 12, 2025
@@ -719,7 +719,7 @@ void hud_do_lock_indicator(float frametime)
}
}

void hud_lock_acquire_current_target(object *target_objp, ship_subsys *target_subsys)
void hud_lock_acquire_current_target(object *target_objp, ship_subsys **target_subsys)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will now modify the source target subsys, which is obviously the intent of the function but didn't happen before now. Have the repercussions been checked?

@Goober5000 Goober5000 dismissed their stale review April 12, 2025 22:54

Putting this on hold until we have a more complete understanding of the hud_lock_acquire_current_target consequences

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
coverity An issue flagged by Coverity fix A fix for bugs, not-a-bugs, and/or regressions. sanitizer/stability Issues caught by Address Sanitizer or other third-party tools.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants