<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/4363/">http://reviewboard.kde.org/r/4363/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By obby.</div>
<p style="color: grey;"><i>Updated 2010-06-21 14:52:36.388199</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">The resubmitted diff actually subtly changed the semantics of the game for cells which were on the right or left borders of the game. My initial idea that the odd loop bounds were to prevent odd behavior resulting from the (only actual) bug was incorrect. The loop bounds were meant to keep a border of 0'ed cells around the (active) game board. This allowed the calculation of neighbors to be very succinct.
With this updated patch, there would no longer be a padded border of 0's around the active game board. This should make the loop bounds simpler to read and understand, and will prevent the implicit display of an extra cell's width and height border around the game board. It does so at the cost of a succinct neighbors() function.
If it is better to stick with old approach, I would like to update the display code to avoid displaying the 0'ed border of the game board (any border around the game board will be explicit.)
At any rate, I believe the original bug should be fixed (displaying the state of memory one step outside of the cells array.) That fix is the smallest part of the current patch.
-Obby, 21 June 2010</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">Note: This review request replaces former request http://reviewboard.kde.org/r/4361/ which was discarded after several attempts to upload an updated diff failed. I apologize for the extra mail. The original fix contained another bug which would cause a crash if the game board was not symmetrical.
This patch fixes an off-by-one error which led to accessing memory just outside of the cells array. It also kept the first character in the cells array from ever being displayed on the board properly, and a bunch of crazy math to keep the rules of the game intact. (The last cell displayed was reflecting the state of memory outside of the cells array.)
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">Testing Done:
Aside from the initial test cases which were used to confirm the bug (hard coding one char in the cells array to always be 1 and moving it around to find the bounds of the display,) I also generated a few other board states manually using examples found here:
http://en.wikipedia.org/wiki/Conway%27s_Game_of_Life
"Blinker" and "Toad" were two that were used to test.
Note: The bug in the original fix (diff in previous review request mentioned above) led to a crash when the gameboard was resized to be non-square. After fixing that bug, I tried several random board sizes, and they all worked.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> (updated)</h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdeplasma-addons/applets/life/life.h <span style="color: grey">(1138875)</span></li>
<li>/trunk/KDE/kdeplasma-addons/applets/life/life.cpp <span style="color: grey">(1138875)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4363/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>