On Sun, Jul 3, 2011 at 4:15 PM, Felipe Pena <felipe...@gmail.com> wrote:
> I like the idea (and voted +1 on it), but I've some consideration to do:
>
> +/* {{{ proto bool SessionHandler::open(string save_path, string session_name)
> + Wraps the old open handler */
> +PHP_METHOD(SessionHandler, open)
> +{
> + PS_SANITY_CHECK;
> +
> + PS(mod_user_is_open) = 1;
> + RETVAL_LONG(PS(default_mod)->s_open(&PS(mod_data), PS(save_path),
> PS(session_name) TSRMLS_CC));
> +}
> [..]
> +ZEND_BEGIN_ARG_INFO(arginfo_session_class_open, 0)
> + ZEND_ARG_INFO(0, save_path)
> + ZEND_ARG_INFO(0, session_name)
> +ZEND_END_ARG_INFO()
>
> This method does not take the save_path and session_name parameter, it
> just use the INI entry.
>
> This lead to following behavior:
> $x = new SessionHandler
> $x->open(1,2,3,4); // param is not used, and no param check at all
>
> It's missing void param check for the close method as well.
>
Hi,
Thanks for pointing that out. I've updated the patch with the param
checks and a couple of tests. Note that the close call still proceeds
if it gets arguments, although it raises the standard warning, because
the default handler would otherwise be left dangling.
The new patches (including 5.4 now):
http://spellign.com/patches/php-trunk-session-oo11.patch
http://spellign.com/patches/php-5.4-session-oo11.patch
http://spellign.com/patches/php-session-oo11-tests.patch
Cheers,
Arpad
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
