Re: cvs: php-src /ext/mcrypt mcrypt.c /ext/mcrypt/tests mcrypt_enc_self_test.phpt

Hannes Magnusson wrote: > On Sun, May 18, 2008 at 7:15 PM, David Soria Parra <dsp@php.net> 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 <derick@derickrethans.nl> | >> +----------------------------------------------------------------------+ >> */ >> -/* $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.