Originally Posted by
JFonseka
Now it gives different hash values for strings longer than 8 characters, but I get an odd feeling that it's not right, like that feeling you get when someone's behind with you a knife, but much stronger.
Unfortunately, with strings >= 8 bytes, you're murdering some part of the world
Remember that strncpy does not insert a null char if the string is longer than the max! So what you are doing is reading beyond the buffer, looking for data.
Otherwise, this sample seems unnecessarily complex and does not do what it is intended to do.
Code:
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#define LONGLONGSIZE (sizeof (unsigned long long))
union intstr {
unsigned long long ikey;
char skey[LONGLONGSIZE];
};
unsigned hashShortString (char v[], unsigned M)
{
union intstr *is;
/* I think you have the idea wrong here. The union is supposed to be
used to convert the string into an uint64_t, nothing else. So use a local variable to store
the hash.
Further, you should probably just use your old algorithm on each 8 bytes and append
the result to your hash.
Another thing to think of is what happens if your string doesn't align on 8-byte
boundaries. You'll get garbage after the null char, and you probably don't want to has
that. */
for (; *v != '\0'; v++)
is->ikey = (LONGLONGSIZE*is->ikey + *v) % M;
return is->ikey % M;
}
int main (int argc, char *argv[])
{
char key[LONGLONGSIZE];
if (2 != argc) {
printf ("USAGE: hash_short STRING\n");
printf (" (STRING will be truncated after %u characters)\n",
LONGLONGSIZE);
return EXIT_FAILURE;
}
strncpy (key, argv[1], LONGLONGSIZE);
printf ("Hash: %u\n", hashShortString (key, 8191));
return EXIT_SUCCESS;
}
Now see my comments in the code.
And also think about your strcpy in the beginning (why not just pass it directly?).