On Jan 12, 2011, at 11:11 PM, Stas Malyshev wrote:

> Hi!
> 
> Some of questions:
> 1. Why clone is not defined? uspoof_clone exists.

Done

> 2.
> /* {{{ proto void Spoofchecker::areConfusable( string $str1, string $str2[, 
> int $error_code ] )
> + * Checks if a given text contains any confusable characters
> + */
> 
> It says void but code returns bool. Same with isSuspicious.
> Also, is 'issued_found' a typo?
> Why wouldn't this function just return the error code?

Yeah, it's meant to return bool. Just a typo in the prototype.

> 
> 3. +  if (U_FAILURE(SPOOFCHECKER_ERROR_CODE(co))) {
> +             zend_throw_exception(zend_exception_get_default(TSRMLS_C), 
> u_errorName(SPOOFCHECKER_ERROR_CODE(co)), SPOOFCHECKER_ERROR_CODE(co) 
> TSRMLS_CC);
> 
> None of intl parts throw exceptions on errors. Please bring the code in sync 
> with the rest of the module.

what value would it return instead of an exception? false? It returns a bool 
already, so false means its not suspicious. So if some random error occurred it 
would imply there is no security issue.

> 
> 4. +  SPOOFCHECKER_METHOD_FETCH_OBJECT
> Please put ; at the end of the line. I know it's not required, but the line 
> without ; at the end makes the code less readable - each time you look at it 
> you think "is there a problem here?"

Done, but surely we should kill ; from the macro then so it becomes required.

> 
> 5. Why setAllowedLocales but no setAllowedChars?

Couldn't see an immediate use, but i'll add it shortly.

- Scott




--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to