On Sun, May 18, 2008 at 8:14 PM, David Soria Parra <[EMAIL PROTECTED]> wrote:
> Hannes Magnusson wrote:
>> On Sun, May 18, 2008 at 7:15 PM, David Soria Parra <[EMAIL PROTECTED]> wrote:
>>> dsp             Sun May 18 17:15:08 2008 UTC
>>>
>>>  Modified files:
>>>    /php-src/ext/mcrypt mcrypt.c
>>>    /php-src/ext/mcrypt/tests   mcrypt_enc_self_test.phpt
>>>  Log:
>>>  MFB: Make mcrypt_enc_self_test() return value compatible with 
>>> documentation and mcrypt_module_self_test()
>>>
>>>
>>> http://cvs.php.net/viewvc.cgi/php-src/ext/mcrypt/mcrypt.c?r1=1.109&r2=1.110&diff_format=u
>>> Index: php-src/ext/mcrypt/mcrypt.c
>>> diff -u php-src/ext/mcrypt/mcrypt.c:1.109 php-src/ext/mcrypt/mcrypt.c:1.110
>>> --- php-src/ext/mcrypt/mcrypt.c:1.109   Mon Dec 31 07:12:11 2007
>>> +++ php-src/ext/mcrypt/mcrypt.c Sun May 18 17:15:08 2008
>>> @@ -16,7 +16,7 @@
>>>    |          Derick Rethans <[EMAIL PROTECTED]>                    |
>>>    +----------------------------------------------------------------------+
>>>  */
>>> -/* $Id: mcrypt.c,v 1.109 2007/12/31 07:12:11 sebastian Exp $ */
>>> +/* $Id: mcrypt.c,v 1.110 2008/05/18 17:15:08 dsp Exp $ */
>>>
>>>  #ifdef HAVE_CONFIG_H
>>>  #include "config.h"
>>> @@ -476,7 +476,12 @@
>>>  PHP_FUNCTION(mcrypt_enc_self_test)
>>>  {
>>>        MCRYPT_GET_TD_ARG
>>> -       RETURN_LONG(mcrypt_enc_self_test(pm->td));
>>> +
>>> +       if (mcrypt_enc_self_test(pm->td) == 0) {
>>> +               RETURN_TRUE;
>>> +       } else {
>>> +               RETURN_FALSE;
>>> +       }
>>>  }
>>>  /* }}} */
>>>
>>> http://cvs.php.net/viewvc.cgi/php-src/ext/mcrypt/tests/mcrypt_enc_self_test.phpt?r1=1.1&r2=1.2&diff_format=u
>>> Index: php-src/ext/mcrypt/tests/mcrypt_enc_self_test.phpt
>>> diff -u php-src/ext/mcrypt/tests/mcrypt_enc_self_test.phpt:1.1 
>>> php-src/ext/mcrypt/tests/mcrypt_enc_self_test.phpt:1.2
>>> --- php-src/ext/mcrypt/tests/mcrypt_enc_self_test.phpt:1.1      Sat May 17 
>>> 23:27:42 2008
>>> +++ php-src/ext/mcrypt/tests/mcrypt_enc_self_test.phpt  Sun May 18 17:15:08 
>>> 2008
>>> @@ -7,4 +7,4 @@
>>>  $td = mcrypt_module_open(MCRYPT_RIJNDAEL_256, '', MCRYPT_MODE_CBC, '');
>>>  var_dump(mcrypt_enc_self_test($td));
>>>  --EXPECT--
>>> -int(0)
>>> \ No newline at end of file
>>> +bool(true)
>>
>> This is a massive BC break...
>> Imagine people who did if (mcrypt_enc_self_test($td) == 0) { ... }
>> that has now changed from dealing with success to failure.
>>
>> I'd say rather fix the docs
>>
>> -Hannes
>
> Yes I noticed that, that's why its not in 5.2. I think it's kind of "cleaning 
> up" as
> mcrypt_module_self_test return a boolean too and the docs say it too. Maybe 
> it's just in my opinion
> that the function itself is not that often used as it would be real issue, 
> but if people have
> objections for sure we can revert that.

The point is this is a very subtle change and not something people
will suspect when their applications stop working out of the blue,
without so much as a single warning.
A function rename would be obvious as it would give a fatal error, but
this is a return value change from false to true...

IMO this should be reverted and the docs fixed.

-Hannes

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

Reply via email to