Skip to content

Conversation

ramon-bernardo
Copy link
Contributor

Pull Request Prelude

Changes Proposed

  • Fix int_fast16_t overflow

@ghost ghost requested review from ranisalt, nekiro and DSpeichert October 16, 2021 04:11
soul4soul
soul4soul previously approved these changes Oct 16, 2021
@DSpeichert
Copy link
Member

From the commit message, PR message and the code, I still have no idea what the problem is and what the fix fixes.

@ranisalt
Copy link
Member

ranisalt commented Oct 16, 2021

From the commit message, PR message and the code, I still have no idea what the problem is and what the fix fixes.

The range arguments are int32, but the result variable is int16, therefore overflow may occur if the range does not fit 16 bits. It doesn't usually happen, because the range is usually small (this function is heavy af, it usually runs with the viewport dimensions), but the conversion MAY hurt performance.

I would just use int everywhere.

@DSpeichert
Copy link
Member

@ranisalt int or int32_t to keep it the same?

@ranisalt
Copy link
Member

ranisalt commented Dec 16, 2021

auto to avoid conversions, but int32_t between those (also avoids conversion)

@DSpeichert DSpeichert merged commit 17bf638 into otland:master Dec 21, 2021
@ramon-bernardo ramon-bernardo deleted the i16-overflow branch December 21, 2021 23:06
Znote pushed a commit to Znote/forgottenserver that referenced this pull request Jan 30, 2022
DSpeichert pushed a commit that referenced this pull request May 9, 2022
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.

4 participants