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
