[Freeipa-devel] [PATCH] Improvements to collection API

[Dmitri Pal]

>
> Nack.
>
> Your PRIME hash function is dangerously limited. You guarantee an
> overflow on 64-bit systems with strings of 13 characters or more. Worse
> on 32 bit systems. At minimum, declare phash a uint64_t. Furthermore, in
> your algorithm, the strings "mystring" and "mystirng" will produce an
> identical hash. Consider locating a library that provides a safer and
> less collision-prone hash. (I have no recommendations, perhaps someone
> else on the list can help.)
>
> Beyond that, I'm not going to look too deeply into the internals.
> Syntactically everything looks fine on a quick scan. It compiles cleanly
> against the current head and does not break any existing code, so once
> the above change is made, I'm comfortable with committing it and working
> out any bugs later on.
>
This is really funny.
The hash function is the exact copy of the key hash function from the
dhash.c lines 131 - 132.
This function constructs the key for the hash table entries so I assumed
that it is sufficient enough.
I view this hash as a quick internal aid to searching.
If it worked in dhash.c why it would not work for me.
The limitation you bring up related to overflow and order of letters is
not a worry at the moment.
It can be enhanced later in a separate patch.

By the way the dhash.c has a bug in this code (line 131).
The value of h in the result of the hashing of a string will always be 0
since h is initialized to 0 and then multiplied.

If there are no other issues I suggest accepting patch and I will log an
enhancement request to improve hashing in collection.



_______________________________________________
Freeipa-devel mailing list
Freeipa-devel at redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


-- 
Thank you,
Dmitri Pal

Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/