Fix storage related integer overflow #5616
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I affirm:
What does this pull request do?
This pull request fixes an interger overflow which can occur when a player accumulates a high amount (>255) of storage in their mog house.
When a player reaches >255 - they will overflow the variables resulting in their total storage size getting capped at an unxpectedly low value.
Why does this happen?
The previous code
std::clamp<uint8>((uint8)m_buff, 0, 80)
was first casting m_buff to uint8, then applying the clamp. When m_buff exceeds the limits of a uint8, it would first overflow, then be clamped.The fix:
Since the clamp is forcing values between 0 and 80 there is no explicit need (at this time) to cast downwards, I have removed the inner cast to unit8 and changed the clamp type to int. Note that clamp type of unit8 will cause a cast prior ro clamping and result in the same integer overlow.
Steps to test these changes
Prior to bug fix:
!additem well
With the bug fix:
At step 6 - you will have 1/80 storage. This will continue until enough items are removed to pull the total storage below 80.