> It would be better to use the randomb[] array as input done
> Please put a space between a function-like identifier and an opening > parenthesis (GNU Coding Style). done > sizeof(unsigned long int) = 8 does not reflect the maximum possible > alignment. Already nowadays, for some things the alignment is 16. > For being future-proof, better use 32: done, #define at top of file so this can be changed in the future > It would be good to make sure that the contents of data[] beyond > the first i + sizeof(plaintext) bytes is not used. To this effect, > store a couple of random bytes after the data[i + sizeof(plaintext) - 1]. using randomb directly so there are extra characters after what we read > Also, one of the proposed algorithms will dissect the input into three > pieces: > 1. a few unaligned bytes at the beginning, > 2. a block of aligned words, > 3. a few extra bytes at the end. > It will be useful to have a test where the number of bytes in part 1 and > in part 3 are independent. So, write a double loop > > for (i = 0; i < 32; i++) > for (k = 0; k < 32; k++) > process (i + 32 * j + k bytes, aligned at 32-i mod 32) did this as two separate loops, one for leading bytes, one for trailing bytes > Additionally, the case where the input is so short that part 1, 2, 3 are > all together is worth testing separately, so: > > for (n = 0; n < 32; n++) > for (i = 0; i < 32; i++) > process (n bytes, aligned at i mod 32) done ran the tests just printing out the values from the current implementation and then put them into arrays that we now check against, the idea being that the current implementation is correct and we expect future implementations to match the current implementation On Tue, 22 Oct 2024 at 12:06, Bruno Haible <br...@clisp.org> wrote: > Sam Russell wrote: > > is there anymore feedback on this? > > + char plaintext[] = "Gnulib crc testsGnulib crc tests" > + "Gnulib crc testsGnulib crc tests" > + "Gnulib crc testsGnulib crc tests" > + "Gnulib crc testsGnulib crc tests" > + "Gnulib crc testsGnulib crc tests"; > > ASCII uses only 7 out of 8 bits. I.e. this input has bit 7 set to zero > everywhere. It would be better to use the randomb[] array as input, > that is now part of the 'crc-tests' module. > > + char data[sizeof(plaintext) + sizeof(unsigned long int)]; > > Please put a space between a function-like identifier and an opening > parenthesis (GNU Coding Style). > > + for (i = 0; i < sizeof(unsigned long int); i++) > > sizeof(unsigned long int) = 8 does not reflect the maximum possible > alignment. Already nowadays, for some things the alignment is 16. > For being future-proof, better use 32: > for (i = 0; i < 32; i++) > > + memcpy(data + i, plaintext, sizeof(plaintext)); > > It would be good to make sure that the contents of data[] beyond > the first i + sizeof(plaintext) bytes is not used. To this effect, > store a couple of random bytes after the data[i + sizeof(plaintext) - 1]. > > Also, one of the proposed algorithms will dissect the input into three > pieces: > 1. a few unaligned bytes at the beginning, > 2. a block of aligned words, > 3. a few extra bytes at the end. > It will be useful to have a test where the number of bytes in part 1 and > in part 3 are independent. So, write a double loop > > for (i = 0; i < 32; i++) > for (k = 0; k < 32; k++) > process (i + 32 * j + k bytes, aligned at 32-i mod 32) > > Additionally, the case where the input is so short that part 1, 2, 3 are > all together is worth testing separately, so: > > for (n = 0; n < 32; n++) > for (i = 0; i < 32; i++) > process (n bytes, aligned at i mod 32) > > Bruno > > > >
gnulib_new_tests_2.patch
Description: Binary data
