Sam Russell wrote:
> > Bug: code reads one more word from memory than it should.
>
> This is a good idea. The nice thing with CRC its that it's pseudo-random so
> having the extra trailing randomb bytes helps us catch this but also even
> uninitialised memory at the end of the string is going to
> Bug: code reads one more word from memory than it should.
This is a good idea. The nice thing with CRC its that it's pseudo-random so
having the extra trailing randomb bytes helps us catch this but also even
uninitialised memory at the end of the string is going to make the tests
fail if we rea
Sam Russell wrote:
> I think we are massively overengineering this. We shouldn't need to
> future-proof tests for changes that *might* be added in the future, and
> having 32x32 hashes in a table at the start of the test is not a big deal
> (it took less than a second to generate and it'll take the
Thanks, my mistake on the dropped CC
On Tue, 22 Oct 2024 at 22:24, Simon Josefsson wrote:
> Thanks, pushed in your name -- next time please attach git-format-patch
> output that includes the commit message, and keep cc to the list.
>
> /Simon
>
> Sam Russell writes:
>
> >> Why '__'? I think
Thanks, pushed in your name -- next time please attach git-format-patch
output that includes the commit message, and keep cc to the list.
/Simon
Sam Russell writes:
>> Why '__'? I think they should be dropped.
>
> thanks, bad habit, other codebases use them as they work better with
> standard
Sam Russell writes:
> +__uint32_t trailing_unaligned_byte_hashes[32] = {
> +__uint32_t leading_unaligned_byte_hashes[32] = {
> +__uint32_t variable_alignment_variable_length_hashes[] = {
Why '__'? I think they should be dropped.
> main (int argc, char *argv[])
A detail, but if the variables
> I'd opt for reducing this set of expected values to 32 values,
> by reintroducing the memcpy() trick from your previous patch:
Attached is a patch with this implementation
On Tue, 22 Oct 2024 at 14:34, Sam Russell wrote:
> I think we are massively overengineering this. We shouldn't need to
>
I think we are massively overengineering this. We shouldn't need to
future-proof tests for changes that *might* be added in the future, and
having 32x32 hashes in a table at the start of the test is not a big deal
(it took less than a second to generate and it'll take the same amount of
time if we
Hi Simon,
> How about an aggregated trick, incrementally hashing each output crc value
> and at the end just compare the hash against an expected value? Instead of
> comparing each incremental crc value against a table.
While this needs fewer expected values, it makes debugging harder
in case o
How about an aggregated trick, incrementally hashing each output crc value and
at the end just compare the hash against an expected value? Instead of
comparing each incremental crc value against a table. This trick works better
for many polynomials too. The hash doesn’t have to be strong so cou
Sam Russell wrote:
> 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
Yes, tha
> 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 al
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"
+
Copyright paperwork is done, is there anymore feedback on this? IMO the
main things are as follows:
1) correctness
2) consistency with coding standard
3) solves a problem (new approach reads 8 bytes at a time so we need
something that will break if the code does this wrong e.g. alignment,
endianne
Thanks this looks better. Let's keep it around for more feedback/review
until your copyright papers arrive.
/Simon
Sam Russell writes:
>> Or repeat "Gnulib crc test string" many times.
>
> done
>
>> This isn't correct until the new implementation is added
>
> done
>
>> I would make it test mu
> Or repeat "Gnulib crc test string" many times.
done
> This isn't correct until the new implementation is added
done
> I would make it test much larger
> mis-alignment too, how about a ~700 byte string and test up to ~300 byte
> mis-alignment?
I'm not sure what problem this solves. I've used
Sam Russell writes:
> + char plaintext[] = "This file is free software: you can redistribu"
> + "te it and/or modify it under the terms of the "
> + "GNU Lesser General Public License as published"
> + " by th";
Using license texts lik
Patch file attached.
As far as I can tell, the main concern with the new optimisations is that
we're moving from processing byte-by-byte to processing in blocks of up to
128 bytes, and these will be read in either 8 or 16 byte chunks. This new
test calls the inner CRC update function with data ali
18 matches
Mail list logo
