Previous version of Murmur3 patch had several issues. Dag-Egling graciously stepped forward with his own implmentation.
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
You replaced the original diff with a new diff relative to the old one. This makes it impossible to comment on the complete diff.
Comment Actions
That's not very useful. I don't want to see the original diff, I want to see the complete diff.
| sys/libkern/murmur3_32.c | ||
|---|---|---|
| 28–31 | <sys/types.h> should always come first, and <sys/stdint.h> is redundant. | |
| 85 | <string.h> isn't needed. | |
| sys/sys/hash.h | ||
| 123–130 | The comment should note that this is intended for aligned data in multiples of four bytes, and / or we should have separate aligned and unaligned versions of the code. | |
| sys/libkern/murmur3_32.c | ||
|---|---|---|
| 38 | Should we have a different name for this function, due to the multiple-of-4 issue? (I.e., other implementations called murmur3_32 handle the 0-3 byte tail.) | |
Comment Actions
The comment should note that this is intended for aligned data in multiples of four bytes, and / or we should have separate aligned and unaligned versions of the code.
I strongly believe we should provide the same api (alignment and size constraints) as other murmur3_32 implementations if we make that the function's name. If we're happy with the alignment and multiple-of-four requirement (I am) we should update the name somehow to reflect that.
Comment Actions
Upated with a name change. Also, leave Jenkins in place for the other
consumer of that hash.
| sys/sys/hash.h | ||
|---|---|---|
| 128 | drop the extra whitespace | |