FNV value calculation is incorrect #4
Comments
|
The pyfasthash library appears to use the reference implementation created by FNV's co-author at http://www.isthe.com/chongo/tech/comp/fnv/#FNV-reference-source The reference implementation on that site does not perform an explicit modulo operation either. I believe the modulo operation is implicit in fnv1a_32 due to the types being unsigned 32 bit integers. If you believe this implementation is incorrect, can you provide the test data and results from your implementation to be tested directly against the reference implementation? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I ran a comparison of your implementation of fnv1a_32 against my own implementation, and yours has a bug. The specification calls for all multiplication to be performed modulo the size of the result. You don't do the modulus operation until after the full calculation is finished. If you run this on a 64-bit machine, the result is wrong.
You need to do something like "hash %= (2 ** 32);" after every multiplication operation to correct this problem.
I considered submitting a push request but the 64-bit code looks pretty hairy, especially the stuff that is meant to calculate the 64-bit checksum on a 32-bit machine. So I didn't want to attempt to patch it.
See here:
http://www.isthe.com/chongo/tech/comp/fnv/#FNV-param
Specifically, this bullet point:
o The multiplication is performed modulo 2^n where n is the bit length of hash.
Your implementation is incorrect.
The text was updated successfully, but these errors were encountered: