Cxbx-Reloaded / xbox_kernel_test_suite

Xbox kernel APIs tester written using nxdk
GNU General Public License v3.0
22 stars 6 forks source link

Fix missing randomness in some Rtl tests #65

Closed ergo720 closed 4 years ago

ergo720 commented 4 years ago

Closes https://github.com/Cxbx-Reloaded/xbox_kernel_test_suite/issues/62. First I tested on the Xbox by adding the following debug code (which was removed before submitting this PR) after the loop which selects the letters:

for(int k=0; k<100; k++){
    print("rnd_letters[%d] = %c", k, rnd_letters[k]);
}
Before PR ``` rnd_letters[0] = s rnd_letters[1] = s rnd_letters[2] = s rnd_letters[3] = s rnd_letters[4] = s rnd_letters[5] = s rnd_letters[6] = s rnd_letters[7] = s rnd_letters[8] = s rnd_letters[9] = s rnd_letters[10] = s rnd_letters[11] = s rnd_letters[12] = s rnd_letters[13] = s rnd_letters[14] = s rnd_letters[15] = s rnd_letters[16] = s rnd_letters[17] = s rnd_letters[18] = s rnd_letters[19] = s rnd_letters[20] = s rnd_letters[21] = s rnd_letters[22] = s rnd_letters[23] = s rnd_letters[24] = s rnd_letters[25] = s rnd_letters[26] = s rnd_letters[27] = s rnd_letters[28] = s rnd_letters[29] = s rnd_letters[30] = s rnd_letters[31] = s rnd_letters[32] = s rnd_letters[33] = s rnd_letters[34] = s rnd_letters[35] = s rnd_letters[36] = s rnd_letters[37] = s rnd_letters[38] = s rnd_letters[39] = s rnd_letters[40] = s rnd_letters[41] = s rnd_letters[42] = s rnd_letters[43] = s rnd_letters[44] = s rnd_letters[45] = s rnd_letters[46] = s rnd_letters[47] = s rnd_letters[48] = s rnd_letters[49] = s rnd_letters[50] = s rnd_letters[51] = s rnd_letters[52] = s rnd_letters[53] = s rnd_letters[54] = s rnd_letters[55] = s rnd_letters[56] = s rnd_letters[57] = s rnd_letters[58] = s rnd_letters[59] = s rnd_letters[60] = s rnd_letters[61] = s rnd_letters[62] = s rnd_letters[63] = s rnd_letters[64] = s rnd_letters[65] = s rnd_letters[66] = s rnd_letters[67] = s rnd_letters[68] = s rnd_letters[69] = s rnd_letters[70] = s rnd_letters[71] = s rnd_letters[72] = s rnd_letters[73] = s rnd_letters[74] = s rnd_letters[75] = s rnd_letters[76] = s rnd_letters[77] = s rnd_letters[78] = s rnd_letters[79] = s rnd_letters[80] = s rnd_letters[81] = s rnd_letters[82] = s rnd_letters[83] = s rnd_letters[84] = s rnd_letters[85] = s rnd_letters[86] = s rnd_letters[87] = s rnd_letters[88] = s rnd_letters[89] = s rnd_letters[90] = s rnd_letters[91] = s rnd_letters[92] = s rnd_letters[93] = s rnd_letters[94] = s rnd_letters[95] = s rnd_letters[96] = s rnd_letters[97] = s rnd_letters[98] = s rnd_letters[99] = s ```
After PR ``` rnd_letters[0] = h rnd_letters[1] = w rnd_letters[2] = x rnd_letters[3] = d rnd_letters[4] = e rnd_letters[5] = c rnd_letters[6] = f rnd_letters[7] = v rnd_letters[8] = l rnd_letters[9] = x rnd_letters[10] = a rnd_letters[11] = d rnd_letters[12] = w rnd_letters[13] = i rnd_letters[14] = y rnd_letters[15] = j rnd_letters[16] = d rnd_letters[17] = f rnd_letters[18] = o rnd_letters[19] = h rnd_letters[20] = a rnd_letters[21] = t rnd_letters[22] = u rnd_letters[23] = s rnd_letters[24] = z rnd_letters[25] = z rnd_letters[26] = z rnd_letters[27] = d rnd_letters[28] = o rnd_letters[29] = q rnd_letters[30] = s rnd_letters[31] = a rnd_letters[32] = m rnd_letters[33] = t rnd_letters[34] = c rnd_letters[35] = q rnd_letters[36] = r rnd_letters[37] = k rnd_letters[38] = q rnd_letters[39] = f rnd_letters[40] = j rnd_letters[41] = h rnd_letters[42] = j rnd_letters[43] = w rnd_letters[44] = d rnd_letters[45] = p rnd_letters[46] = q rnd_letters[47] = d rnd_letters[48] = u rnd_letters[49] = u rnd_letters[50] = r rnd_letters[51] = c rnd_letters[52] = d rnd_letters[53] = b rnd_letters[54] = r rnd_letters[55] = k rnd_letters[56] = k rnd_letters[57] = v rnd_letters[58] = k rnd_letters[59] = t rnd_letters[60] = r rnd_letters[61] = x rnd_letters[62] = b rnd_letters[63] = z rnd_letters[64] = v rnd_letters[65] = e rnd_letters[66] = w rnd_letters[67] = o rnd_letters[68] = z rnd_letters[69] = e rnd_letters[70] = x rnd_letters[71] = c rnd_letters[72] = w rnd_letters[73] = d rnd_letters[74] = h rnd_letters[75] = f rnd_letters[76] = t rnd_letters[77] = k rnd_letters[78] = i rnd_letters[79] = m rnd_letters[80] = o rnd_letters[81] = d rnd_letters[82] = x rnd_letters[83] = p rnd_letters[84] = h rnd_letters[85] = a rnd_letters[86] = t rnd_letters[87] = d rnd_letters[88] = x rnd_letters[89] = o rnd_letters[90] = j rnd_letters[91] = p rnd_letters[92] = o rnd_letters[93] = t rnd_letters[94] = i rnd_letters[95] = p rnd_letters[96] = s rnd_letters[97] = l rnd_letters[98] = m rnd_letters[99] = y ```

The previous code was indeed selecting the same letter, thus confirming the issue. After the fix, pseudo-random letters are selected, as desired. I repeated the test with https://github.com/Cxbx-Reloaded/Cxbx-Reloaded/commit/08691c24e17f48e7cecc839fb4a7fd6543d332c5 and got the same behaviour as well. Also, the kernel_tests.log file reported the RtlUpperString test as successful on both the Xbox and Cxbx-R so this didn’t break the test either. Finally, I applied the same fix to the RtlMoveMemory test, since I noticed it used the same code to select the random letters. I only tested that with Cxbx-R however, where I got the same result of above (that is, it selects pseudo-random letters).

PS: the results of above were obtained with a seed based on time(NULL), and thus not constant like https://github.com/Cxbx-Reloaded/xbox_kernel_test_suite/issues/62 suggested. This was rectified in a later commit, but the seed constness was only tested and confirmed with Cxbx-R.

RadWolfie commented 4 years ago

LGTM, though it would be better to squash all those commits into one.