Hi,
A while back I wrote a patch for ext/hash to add support for the 32- and
64-bit variants of the FNV-1 hash algorithm. Since we now have a new
trunk, I thought I'd clean it up a bit, and thanks to some suggestions
from Pierre and Johannes on IRC it's a bit better now. Would anyone
object to applying this to trunk (and could someone do it for me, as I
have no karma for that)?
Description of the algorithm:
http://www.isthe.com/chongo/tech/comp/fnv/index.html
http://en.wikipedia.org/wiki/Fowler-Noll-Vo_hash_function
The patch:
http://mgdm.net/~michael/patches/php-fnv.txt
--
Cheers,
Michael
A while back I wrote a patch for ext/hash to add support for the 32- and
64-bit variants of the FNV-1 hash algorithm. Since we now have a new trunk, I
thought I'd clean it up a bit, and thanks to some suggestions from Pierre and
Johannes on IRC it's a bit better now. Would anyone object to applying this to
trunk (and could someone do it for me, as I have no karma for that)?
You do have karma; looks great!
with kind regards,
Derick
http://derickrethans.nl | http://xdebug.org
Like Xdebug? Consider a donation: http://xdebug.org/donate.php
twitter: @derickr and @xdebug
The patch:
http://mgdm.net/~michael/patches/php-fnv.txt
just to throw something out there, shouldn't the various inputLen
parameters be of type size_t instead of unsigned int?
sean
sean finney wrote:
The patch:
http://mgdm.net/~michael/patches/php-fnv.txtjust to throw something out there, shouldn't the various inputLen
parameters be of type size_t instead of unsigned int?
The function signature in php_hash.g says unsigned int, so that's what I
used (though I note some of the other algorithms appear not to). We
don't support files > 2^32 on 64-bit, so as I understand it, it's not
going to cause issues right now, but if a large file support patch lands
it's something that I might revisit.
--
Cheers,
Michael
Michael Maclean wrote:
The function signature in php_hash.g says unsigned int, so that's what I
used (though I note some of the other algorithms appear not to). We
don't support files > 2^32 on 64-bit, so as I understand it, it's not
going to cause issues right now, but if a large file support patch lands
it's something that I might revisit.
And of course, when I say "file" here, I mean "string". Ahem.
--
Cheers,
Michael
just to throw something out there, shouldn't the various inputLen
parameters be of type size_t instead of unsigned int?The function signature in php_hash.g says unsigned int, so that's
what I used (though I note some of the other algorithms appear not
to). We don't support files > 2^32 on 64-bit, so as I understand it,
it's not going to cause issues right now, but if a large file
support patch lands it's something that I might revisit.
okay, so the hash module is generally broken for files > 2GB? didn't
know that. i see update functions using both uint and size_t, but the
function pointer signature uses uint so i doubt the hash results would
be correct.
sean
hi Sean,
okay, so the hash module is generally broken for files > 2GB? didn't
know that. i see update functions using both uint and size_t, but the
function pointer signature uses uint so i doubt the hash results would
be correct.
As far as I can tell that's not only for the hash module. Many areas
in php and its libraries rely on (u)int instead of size_t. It is or
was a common mistake. The problem is to change the signature now
without breaking ABI.
That's something we could fix in trunk.
Cheers,
Pierre
@pierrejoye | http://blog.thepimp.net | http://www.libgd.org
As far as I can tell that's not only for the hash module. Many areas
in php and its libraries rely on (u)int instead of size_t. It is or
was a common mistake. The problem is to change the signature now
without breaking ABI.That's something we could fix in trunk.
yeah, i only brought it up because it was new code being proposed, and
no reason to have yet another undocumented place where this needed
to be fixed.
but, it seems from further info that the hash functions are used elsewhere in
the module in such a way that the parameter ends up treated as a uint no
matter what, so i don't think there's much to be done without tackling
the larger problem. so, sorry for the noise :)
sean