Unit test in keyHash.m fails on big endian system

[rlaboiss]

This has been reported in bug #1133123, filed against the Debian's octave-datatype package. At least one of the unit tests in keyHash.m fails on big endian systems. For instance, on a s390x system:

octave:2> test keyHash
***** test
 A = uint64(128);
 E = uint64(8038837787959150693);
 key = keyHash (A);
 assert (isequal(key, E), sprintf("k: %lx e: %lx d: %lx", key, E, key - E));
 assert (key, E);
!!!!! test failed
k: 39c37805f8f12165 e: 6f8fb060664ac865 d: 0

[rlaboiss]

As a wild guess, I think that the problem might be in lines 86 and 87 of the __nkeyHash__.cc file:

datatypes/src/__nkeyHash__.cc

Line 86 in 69e9ec0

const void *in = args(0).mex_get_data ();

datatypes/src/__nkeyHash__.cc

Line 87 in 69e9ec0

const char *buf = static_cast<const char *>(in);

That void * cast should get the bytes of args(0) in the reverse order on a big endian system.

I did not test this hypothesis, though.

[rlaboiss]

I ran some tests and confirmed the hypothesis. The algorithm implemented in __nkeyHash__.cc depends on the byte order of the data used to compute the key.

The fix does not seem trivial because the keyHash function appears to accept various input argument types, including scalars (such as strings, integers and floats), as well as matrices containing these types of scalar. If the argument of the __nkeyHash__ function is a numeric scalar, reversing its bytes, according to the system's endianness, would fix the problem. However, when the argument is a numeric matrix, each element must be reversed separately.

[rlaboiss]

The fix does not seem trivial because the keyHash function appears to accept various input argument types, including scalars (such as strings, integers and floats), as well as matrices containing these types of scalar. If the argument of the __nkeyHash__ function is a numeric scalar, reversing its bytes, according to the system's endianness, would fix the problem. However, when the argument is a numeric matrix, each element must be reversed separately.

I prepared PR #44, which resolves the issue based on this idea. The changes have been tested on both amd64 (little-endian) and s390x (big-endian) systems. Unfortunately, backward compatibility is lost for numeric and logical inputs. This may render the PR inappropriate, but it remains a proof of concept.

The __nkeyHash__ function, whose result depends on endianness, is still used in other functions, namely calendarDuration, categorical, datetime, duration, and string. The endianness issue persists.