Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:73192
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.217.171 as permitted sender)
MIME-Version: 1.0
Sender: yohgaki@gmail.com
In-Reply-To: <CAPhkiZw5xWRkc=ofh-nB5ZkjfThZFkH_+pK5RZaQFLKhno7u7A@mail.gmail.com>
References: <CAPhkiZw5xWRkc=ofh-nB5ZkjfThZFkH_+pK5RZaQFLKhno7u7A@mail.gmail.com>
Date: Sun, 16 Mar 2014 16:57:02 +0900
Message-ID: <CAGa2bXZEsiqN_8hxTcR3GX+PH=LG76_LOMe29JqbSa0TvE0GVA@mail.gmail.com>
To: Andrey Andreev <narf@devilix.net>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a1133ab625c9d1104f4b4a805
Subject: Re: [PHP-DEV] [RFC] Revert/extend/postpone original RFC about
 read_only, lazy_write sessions
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

--001a1133ab625c9d1104f4b4a805
Content-Type: text/plain; charset=UTF-8

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

--001a1133ab625c9d1104f4b4a805--