On Mon, 14 Jun 2004, Alexander Valyalkin wrote:
> Thank you for good explanation and comments.
> Now I understood that the current crc32 implementation is better than mine.
> But it consists some ugly bugs (read my comments):
> PHP_NAMED_FUNCTION(php_if_crc32)
> {
> unsigned int crc = ~0;
> char *p;
> int len, nr; /*!!! is special sense in [len] var here? Remove it! */
>
> if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &p, &nr) ==
> FAILURE) {
> return;
> }
> /* !!! there is no error check nr < 0 */
Of course not, that's pointless as a string can never have a
negative length.
> len = 0 ; /* !!! remove it! */
Why? It's used one line below and you HAVE to initalize a variable.
> for (len += nr; nr--; ++p) {
> CRC32(crc, *p);
> }
> RETVAL_LONG(~crc);
> }
>
> Below is corrected function with speed improvement in main cycle
It also violates our coding standards BIG time. And there is no reason
to expand that macro at all, nor is your code regarding the || nr <0
needed.
Please STOP wasting our time with those silly improvements which aren't
improvements. You're wasting our and your own time.
Derick
k
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
