From d412aa9d636b22e65b11a9c4308fff7d9387fca8 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Wed, 17 Aug 2011 15:03:18 -0500 Subject: [PATCH] Fix H3 assumption of an 8-bit byte/char. The hash function was internally casting the void* data argument into an unsigned char* and then using values from that to index another internal array that's dimensioned based on the assumption of 256 values possible for an unsigned char (8-bit chars/bytes). This is probably a correct assumption most of the time, but should be safer to use the limits as defined in standard headers to get it right for the particular system/compiler. There was an unused uint8* casted variable in HashKey::HashBytes that seemed like it might have been meant to be passed to H3's hash function as an unfinished attempt to solve the 8-bit byte assumption problem, but that doesn't seem as good as taking care of that internally in H3 so users of the API are only concerned with byte sizes as reported by `sizeof`. Removing the unused variable addresses #530. Also a minor tweak to an hmac_md5 call that was casting away const from one argument (which doesn't match the prototype). --- src/H3.h | 17 +++++++++++------ src/Hash.cc | 3 +-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/H3.h b/src/H3.h index 2a5368a93d..1c2d621f3d 100644 --- a/src/H3.h +++ b/src/H3.h @@ -59,8 +59,13 @@ #ifndef H3_H #define H3_H +#include + +// the number of values representable by a byte +#define H3_BYTE_RANGE (UCHAR_MAX+1) + template class H3 { - T byte_lookup[N][256]; + T byte_lookup[N][H3_BYTE_RANGE]; public: H3(); ~H3() { free(byte_lookup); } @@ -90,9 +95,9 @@ public: template H3::H3() { - T bit_lookup[N * 8]; + T bit_lookup[N * CHAR_BIT]; - for (size_t bit = 0; bit < N * 8; bit++) { + for (size_t bit = 0; bit < N * CHAR_BIT; bit++) { bit_lookup[bit] = 0; for (size_t i = 0; i < sizeof(T)/2; i++) { // assume random() returns at least 16 random bits @@ -101,12 +106,12 @@ H3::H3() } for (size_t byte = 0; byte < N; byte++) { - for (unsigned val = 0; val < 256; val++) { + for (unsigned val = 0; val < H3_BYTE_RANGE; val++) { byte_lookup[byte][val] = 0; - for (size_t bit = 0; bit < 8; bit++) { + for (size_t bit = 0; bit < CHAR_BIT; bit++) { // Does this mean byte_lookup[*][0] == 0? -RP if (val & (1 << bit)) - byte_lookup[byte][val] ^= bit_lookup[byte*8+bit]; + byte_lookup[byte][val] ^= bit_lookup[byte*CHAR_BIT+bit]; } } } diff --git a/src/Hash.cc b/src/Hash.cc index ffabe36f83..1902af4f37 100644 --- a/src/Hash.cc +++ b/src/Hash.cc @@ -168,13 +168,12 @@ hash_t HashKey::HashBytes(const void* bytes, int size) { if ( size <= UHASH_KEY_SIZE ) { - const uint8* b = reinterpret_cast(bytes); // H3 doesn't check if size is zero return ( size == 0 ) ? 0 : (*h3)(bytes, size); } // Fall back to HMAC/MD5 for longer data (which is usually rare). hash_t digest[16]; - hmac_md5(size, (unsigned char*) bytes, (unsigned char*) digest); + hmac_md5(size, (const unsigned char*) bytes, (unsigned char*) digest); return digest[0]; }