On 11:31 Tue 27 Mar , Apollon Oikonomopoulos wrote:
Hi,
On 12:55 Mon 26 Mar , Josef 'Jeff' Sipek wrote:
On Mon, Mar 26, 2018 at 15:57:01 +0300, Apollon Oikonomopoulos wrote: ...
I'd be happy to test the patch, thanks!
Ok, try the attached patch. (It is a first pass at the issue, so it may not be the final diff that'll end up getting committed. It'd be good to know if it actually fixes the issue for you - sadly, I don't have a big endian system to play with.)
Thanks for the quick response!
Unfortunately still fails, although with fewer assertion errors than before:
test-murmurhash3.c:34: Assert(#8) failed: memcmp(result, vectors[i].result, sizeof(result)) == 0 test-murmurhash3.c:34: Assert(#11) failed: memcmp(result, vectors[i].result, sizeof(result)) == 0 test-murmurhash3.c:34: Assert(#12) failed: memcmp(result, vectors[i].result, sizeof(result)) == 0 murmurhash3 (murmurhash3_32) ......................................... : FAILED test-murmurhash3.c:34: Assert(#12) failed: memcmp(result, vectors[i].result, sizeof(result)) == 0 murmurhash3 (murmurhash3_128) ........................................ : FAILED
It turns out there's a missing byte-inversion when loading the blocks which should be addressed in getblock{32,64}. Murmurhash treats each block as an integer expecting little-endian storage. Applying this additional change fixes the build on s390x (and does not break it on x864_64): --- b/src/lib/murmurhash3.c +++ b/src/lib/murmurhash3.c @@ -23,7 +23,7 @@ static inline uint32_t getblock32(const uint32_t *p, int i) { - return p[i]; + return cpu32_to_le(p[i]); } //----------------------------------------------------------------------------- @@ -105,7 +105,7 @@ static inline uint64_t getblock64(const uint64_t *p, int i) { - return p[i]; + return cpu64_to_le(p[i]); } Regards, Apollon