On Sun, Mar 16, 2014 at 3:24 AM, Andrey Andreev <[email protected]> wrote:
> I'm announcing the following RFC for discussion, with the hope that it
> can get through before the PHP 5.6 release:
> https://wiki.php.net/rfc/session-read_only-lazy_write
>
> As noted in it, I don't feel like
> https://wiki.php.net/rfc/session-lock-ini was
> handled properly. Lack
> of attention to it alone is demonstrated by the fact that a total of
> only 10 people have voted. I hope that this follow-up receives more
> attention, so that we can avoid a potential mess.
>
1. Because it was designed as a function argument only (and not an INI
setting), there is no API to tell users if/when 'lazy_write' is currently
used.
I understand this is because users don't like INI settings and that there
are way too many of them for sessions already. This is a valid argument of
course, but it completely ignores the fact that INIs exist for a reason and
they are a necessity in some cases. This is one of those cases.
I agree this part. INI is not bad thing.
To illustrate why sticking with INI is good, I coded session_start() as
follows.
If options are INIs, we may eliminate strncmp()s. Longer lines are
truncated, please refer to
https://github.com/yohgaki/php-src/compare/PHP-5.6-rfc-session-lock
for
complete patch.
-#define php_session_set_opt_bool(hash, target, key) do { \
+#define php_session_start_set_opt_bool(hash, target, key) do { \
zval **entry; \
if (zend_hash_find(Z_ARRVAL_P(hash), key, sizeof(key), (void
**)&entry) == SUCCESS) { \
convert_to_boolean(*entry); \
@@ -2276,11 +2277,21 @@ static PHP_FUNCTION(session_decode)
} \
} while(0);
+
+static int php_session_start_set_ini(char *varname, uint varname_len, char
*new_value, uint new_value_len TSR
+ char buf[128];
+ snprintf(buf, 127, "%s.%s", "session", varname);
+ return zend_alter_ini_entry_ex(buf, strlen(buf)+1, new_value,
new_value_len, PHP_INI_USER, PHP_INI_STA
+}
+
+
/* {{{ proto bool session_start(void)
Begin session - reinitializes freezed variables, registers browsers etc
*/
static PHP_FUNCTION(session_start)
{
zval *options = NULL;
+ zval **value;
+ HashPosition pos;
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|a",
&options) == FAILURE) {
RETURN_FALSE;
@@ -2288,12 +2299,40 @@ static PHP_FUNCTION(session_start)
/* set options */
if (options) {
- /* I'll refactor option handling later. It's not smart. */
- PS(read_only) = 0;
- php_session_set_opt_bool(options, PS(read_only),
"read_only"); /* this does not have to
- php_session_set_opt_bool(options, PS(lazy_write),
"lazy_write");
+ /* Iterate through hash */
+ zend_hash_internal_pointer_reset_ex(Z_ARRVAL_P(options),
&pos);
+ while (zend_hash_get_current_data_ex(Z_ARRVAL_P(options),
(void **)&value, &pos) == SUCCESS) {
+ char *key;
+ zend_uint key_len;
+ ulong index;
+
+ zend_hash_get_current_key_ex(Z_ARRVAL_P(options),
&key, &key_len, &index, 0, &pos);
+ switch(Z_TYPE_PP(value)) {
+ case IS_ARRAY:
+ case IS_DOUBLE:
+ case IS_OBJECT:
+ case IS_NULL:
+ case IS_RESOURCE:
+ case IS_CALLABLE:
+ php_error_docref(NULL TSRMLS_CC,
E_WARNING, "Option(%s) value must be
+ break;
+ default:
+ if (!strncmp(key, "read_only",
sizeof("read_only")-1)) {
+ PS(read_only) = 0;
+
php_session_start_set_opt_bool(options, PS(read_only), "
+ } else if (!strncmp(key,
"lazy_write", sizeof("lazy_write")-1)) {
+
php_session_start_set_opt_bool(options, PS(lazy_write), "
+ } else {
+ convert_to_string(*value);
+ if
(php_session_start_set_ini(key, key_len, Z_STRVAL_PP(value)
+
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Setting o
+ }
+ }
+ break;
+ }
+ zend_hash_move_forward_ex(Z_ARRVAL_P(options),
&pos);
+ }
}
- /* allow to set INI options? */
php_session_start(TSRMLS_C);
This could be optimized/cleaned by using ini modifying handler function
hash. In return, there
would be number of similar functions. (I'm not sure which one is faster)
Using INI could make code a little simpler.
Regards,
--
Yasuo Ohgaki
[email protected]