On Tue, 2013-04-30 at 10:05 +0200, Michael Wallner wrote:
>
> - the spl_ce_BadFunctionCallException must not be declared static, but
> extern.
Neither. One should include ext/spl/spl_exceptions.h and use the
declaration from the header instead of declaring yourself.
In this specific case though instead of using the exception the function
should be non-existent on windows
#ifndef PHP_WIN32
PHP_FUNCTION(scrypt_pickparams)
{
/* ... */
}
#endif
and similar around the function table entry. This is consistent what we
doing other parts of PHP and allows function_exists() as a check.
I've just scrolled a bit over the code and a few things caught my eye:
hex = (unsigned char*) emalloc(keyLength * 2 + 1);
Please use safe_emalloc to avoid overflows.
Also in
} else if (keyLength > 137438953440) { //(2^32 - 1) * 32
you can rely on the compiler to do some optimization and have it more
readable instead of having the magic number there ..
Given that value is a signed long:
} else if (value > UINT64_MAX) {
how could this happen? Only on a 128bit machine a signed long might
reach that value (this check isn't wrong, though, but together with the
next comment gives a bad feeling while reading) also that function's
declaration claims to return an unsigned value, but you're using -1 as
error code ... Mike already mentioned to use php_error_docref ... and
please use the constants, not the value "1" ... nobody understands what
this is (and throwing fatal errors is highly discouraged unless you are
in the engine and have no good way to recover the execution state)
With
RETURN_BOOL(0);
please use RETURN_TRUE or RETURN_FALSE to make this explicit (in some
parts [zend hash API] the meaning of numeric constants is reversed,
better be explicit)
johannes
--
PECL development discussion Mailing List (http://pecl.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
