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 Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
