Fixing an extremely suspicious-looking paradigm in murmur_hash.c

The current code is using the same variable name twice, k1, in two different scopes.
A cursory inspection of the code looks like one is trying to sneak a vulnerability by
masking off the reset of the k1 variable out of the loop; which is, in fact, what the
official algorithm does explicitely.

I guess this is a result of the C89-ization of the official algorithm, with the second
k1 variable usually defined AFTER the loop.

This change brings more sanity to the algorithm, making it much better-looking by
removing all ambiguity in what we're doing. The end result is exactly the same, but
at least we're explicit about it.
pull/198/head
Nicolas "Pixel" Noble 10 years ago
parent e2380338bf
commit 9074794e39
  1. 6
      src/core/support/murmur_hash.c

@ -52,7 +52,7 @@ gpr_uint32 gpr_murmur_hash3(const void *key, size_t len, gpr_uint32 seed) {
int i;
gpr_uint32 h1 = seed;
gpr_uint32 k1 = 0;
gpr_uint32 k1;
const gpr_uint32 c1 = 0xcc9e2d51;
const gpr_uint32 c2 = 0x1b873593;
@ -62,7 +62,7 @@ gpr_uint32 gpr_murmur_hash3(const void *key, size_t len, gpr_uint32 seed) {
/* body */
for (i = -nblocks; i; i++) {
gpr_uint32 k1 = GETBLOCK32(blocks, i);
k1 = GETBLOCK32(blocks, i);
k1 *= c1;
k1 = ROTL32(k1, 15);
@ -73,6 +73,8 @@ gpr_uint32 gpr_murmur_hash3(const void *key, size_t len, gpr_uint32 seed) {
h1 = h1 * 5 + 0xe6546b64;
}
k1 = 0;
/* tail */
switch (len & 3) {
case 3:

Loading…
Cancel
Save