[Kde-games-devel] Review Request 126919: Fix crash when placing bonuses in the arena

[Frederik Schwarzer]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126919/#review91767
-----------------------------------------------------------



In line 354 of the same file the author already tries to handle the 4th quarter.
```cpp
    nQuarterSize = (nQuarter < 3 ? nFullSize / 4 : nFullSize - 3 * nFullSize / 4);
```
Can you fix the issue in that line instead?
I mean, if I divide by 4, I am always at the lower bound of possibilities. E.g. with a size of 75, I get 18.75 mathematically, which is 18 with the '/'.
If I then take three fourth of the size (size*3/4), it's 56,25, resulting in 56 being subtracted from the 75, which is 19. Boom?

So I would suggest something like:
```cpp
-        nQuarterSize = (nQuarter < 3 ? nFullSize / 4 : nFullSize - 3 * nFullSize / 4);
+        if (nQuarter < 3) {
+            nQuarterSize = nFullSize / 4;
+        } else {
+            if ((nFullSize * 3 % 4) == 0) {
+                nQuarterSize = nFullSize - nFullSize * 3 / 4;
+            } else {
+                nQuarterSize = nFullSize - nFullSize * 3 / 4 - 1;
+            }
+        }
```
around line 354 so nQuarterSize is correct throughout the whole rest of that method.

Or am I barking up the wrong tree here?

- Frederik Schwarzer


On Jan. 28, 2016, 1:30 p.m., Julian Helfferich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126919/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2016, 1:30 p.m.)
> 
> 
> Review request for KDE Games.
> 
> 
> Repository: granatier
> 
> 
> Description
> -------
> 
> The crash takes place in game.cpp - Game::createBonus(). The total amount of blocks is nFullSize. To place the bonuses, the game iterates over 4 quarters, the first three containing nFullSize/4 blocks and the last containing the remaining blocks. Thus, nQuarterSize can be larger for the last quarter than for the previous three. Now, when a bonus is assigned to a block, the index of the block is calculated as
> 
> nIndex = nQuarter * nQuarterSize + i
> 
> where i iterates from zero to nQuarterSize. The idea is that nQuarter * nQuarterSize is the number of blocks of the previous quarters. However, since nQuarterSize can be larger for the last quarter, this can lead to an index out of bounds when a bonus is to be placed in one of the last blocks. The fixed version is
> 
> nIndex = nQuarter * (nFullSize/4) + i
> 
> where nFullSize/4 is the size of the first three quarters.
> 
> 
> Diffs
> -----
> 
>   src/game.cpp 371fac9 
> 
> Diff: https://git.reviewboard.kde.org/r/126919/diff/
> 
> 
> Testing
> -------
> 
> Started the game a lot of times. Crash did not happen.
> 
> 
> Thanks,
> 
> Julian Helfferich
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20160129/8f52e297/attachment-0001.html>