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

Fix signed integer overflow in pixel_minimap::prepare_cache_for_updates #36873

Merged
merged 2 commits into from
Jan 10, 2020

Conversation

hexagonrecursion
Copy link
Contributor

Summary

SUMMARY: Bugfixes "fix an integer overflow in pixel_minimap::prepare_cache_for_updates"

Purpose of change

Fixes #36807 - Initial value of cached_center_sm was causing an integer overflow

Describe the solution

  1. cached_center_sm is only used in prepare_cache_for_updates
  2. The first time prepare_cache_for_updates is called the cache is empty, which means both branches of if are no-op and it does not matter which one it executed.
    if( std::abs( center_sm_diff.x ) > 1 ||
    std::abs( center_sm_diff.y ) > 1 ||
    std::abs( center_sm_diff.z ) > 0 ) {
    cache.clear();
    } else {
    for( auto &mcp : cache ) {
    mcp.second.touched = false;
    }
    }
  3. Every time prepare_cache_for_updates is executed it updates the value of cached_center_sm

Therefore any initial value that does not cause an overflow is fine.
I think tripoint_zero will work here.

Describe alternatives you've considered

Testing

Additional context

Assumptions:
1. cached_center_sm is only used in prepare_cache_for_updates
2. The first time prepare_cache_for_updates is called cache is empty, which means both branches of if are no-op and it does not matter which one it executed.
Apparently tripoint is zero-initialized by default. Thanks travis-ci for running clang-tidy.
@kevingranade kevingranade merged commit 4d2187e into CleverRaven:master Jan 10, 2020
@hexagonrecursion hexagonrecursion deleted the patch-2 branch January 11, 2020 08:30
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.

Signed integer overflow in pixel_minimap::prepare_cache_for_updates
2 participants