Re: [RFC] Revert/extend/postpone original RFC about read_only, lazy_write sessions

On Sun, Mar 16, 2014 at 3:24 AM, Andrey Andreev <narf@devilix.net> 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 yohgaki@ohgaki.net