dgryski / go-farm

go-farm: a pure-Go farmhash implementation
MIT License
245 stars 22 forks source link

Function hash32Len0to4 is returning different values than reference implementation #9

Closed nicolaasuni closed 6 years ago

nicolaasuni commented 7 years ago

The function hash32Len0to4 is returning different values than the reference implementation. I can see the following difference: https://github.com/google/farmhash/blob/master/src/farmhash.cc#L1055 https://github.com/dgryski/go-farm/blob/master/farmhashcc.go#L30

In the original implementation v is a signed char and here is a uint32. The function seems exploiting the overflow behaviour but I am not sure how to correctly reproduce this in golang.

dgryski commented 6 years ago

I'll take a look at this.

dgryski commented 6 years ago

Indeed, a quick hacky test does show different values coming from these two routines, and it is indeed due to overflow. I'll push a fix tonight when I'm back home.

dgryski commented 6 years ago

Sorry this took so long. So, oddly our tests do cover this line but our tests don't fail after the change. I guess our input doesn't include a byte value that would turn negative.