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

[Stephen Gallagher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/29/2009 11:45 AM, Dmitri Pal wrote:
> Collection improvements:
> 
> 1) In the first implementation the collection elements we always added
> to the end of the collection.  There was no way to insert elements in a
> specific places in the collection.
> Now there is.  The original implementation of add_xxx_property functions
> is wrapped around new insert_xxx_property functions.
> 2) As as pair to the insert functionality  there is now an extract
> functionality also added in this patch. The extraction allows getting
> specific item from the existing collection and operating with it.
> 3) Collection.h has a lot of comments describing the meaning and use of
> different flags.
> 4) Unit test was updated
> 5) Valgrind tests were run using unit test.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Freeipa-devel mailing list
> Freeipa-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/freeipa-devel

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.

- -- 
Stephen Gallagher
RHCE 804006346421761

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkpJJ8QACgkQeiVVYja6o6MGWwCeL6huizZqxkVwUrDD38FnO2+o
ZzgAn1fH/i1AWplf8vjLx77WqFa5PBVn
=zD+E
-----END PGP SIGNATURE-----