On October 2, 2002 05:13 am, Sascha Schumann wrote: > Could some kind soul with a thorough understanding of the > engine details please review the attached patch? > > Changes: > > - Fix two simple memory leaks. > - Fix a bug which did not initialize $_SESSION, if > is_array($HTTP_SESSION_VARS) was true. > > - Sascha
The patch does solve the problems it sets out to fix, most important of which in my opinion is the memory leak. There is however another problem, which I believe should be addressed. The problem being that session_register() function does not work unless the user has register_globals enabled. This particular problem causes problems for anyone using php software that was written before this limitation was added. In fact the sessions tests inside ext/session/tests fail for that very reason. To this affect, I have made a few modifications to the patch written by Sascha that address this problem. After the patch is applied, register_session() works regardless of whether or not register_globals are enabled or disabled. Ilia
Index: session.c =================================================================== RCS file: /repository/php4/ext/session/session.c,v retrieving revision 1.323 diff -u -3 -p -u -r1.323 session.c --- session.c 1 Oct 2002 11:59:45 -0000 1.323 +++ session.c 2 Oct 2002 14:48:27 -0000 @@ -241,6 +241,7 @@ typedef struct { void php_add_session_var(char *name, size_t namelen TSRMLS_DC) { zval **sym_track = NULL; + zval **sym_global = NULL; zend_hash_find(Z_ARRVAL_P(PS(http_session_vars)), name, namelen + 1, (void *) &sym_track); @@ -249,29 +250,25 @@ void php_add_session_var(char *name, siz * Set up a proper reference between $_SESSION["x"] and $x. */ - if (PG(register_globals)) { - zval **sym_global = NULL; - - zend_hash_find(&EG(symbol_table), name, namelen + 1, - (void *) &sym_global); + zend_hash_find(&EG(symbol_table), name, namelen + 1, (void *) &sym_global); - if (sym_global == NULL && sym_track == NULL) { - zval *empty_var; + if (sym_global == NULL && sym_track == NULL) { + zval *empty_var; - ALLOC_INIT_ZVAL(empty_var); - zend_set_hash_symbol(empty_var, name, namelen, 1, 2, Z_ARRVAL_P(PS(http_session_vars)), &EG(symbol_table)); - } else if (sym_global == NULL) { - zend_set_hash_symbol(*sym_track, name, namelen, 1, 1, &EG(symbol_table)); - } else if (sym_track == NULL) { - zend_set_hash_symbol(*sym_global, name, namelen, 1, 1, Z_ARRVAL_P(PS(http_session_vars))); - } - } else { - if (sym_track == NULL) { - zval *empty_var; - - ALLOC_INIT_ZVAL(empty_var); - zend_set_hash_symbol(empty_var, name, namelen, 0, 1, Z_ARRVAL_P(PS(http_session_vars))); - } + ALLOC_INIT_ZVAL(empty_var); /* this sets refcount to 1 */ + ZVAL_DELREF(empty_var); /* our module does not maintain a ref */ + /* The next call will increase refcount by NR_OF_SYM_TABLES==2 */ + if (PG(register_globals)) { + zend_set_hash_symbol(empty_var, name, namelen, 1, 2, + Z_ARRVAL_P(PS(http_session_vars)), &EG(symbol_table)); + } else { + zend_set_hash_symbol(empty_var, name, namelen, 1, 1, + Z_ARRVAL_P(PS(http_session_vars))); + } + } else if (sym_global == NULL) { + zend_set_hash_symbol(*sym_track, name, namelen, 1, 1, &EG(symbol_table)); + } else if (sym_track == NULL) { + zend_set_hash_symbol(*sym_global, name, namelen, 1, 1, Z_ARRVAL_P(PS(http_session_vars))); } } @@ -465,23 +462,16 @@ break_outer_loop: static void php_session_track_init(TSRMLS_D) { - zval **old_vars = NULL; + /* Unconditionally destroy existing arrays -- possible dirty data */ + zend_hash_del(&EG(symbol_table), "HTTP_SESSION_VARS", + sizeof("HTTP_SESSION_VARS")); + zend_hash_del(&EG(symbol_table), "_SESSION", sizeof("_SESSION")); - if (zend_hash_find(&EG(symbol_table), "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS"), (void **)&old_vars) == SUCCESS && Z_TYPE_PP(old_vars) == IS_ARRAY) { - PS(http_session_vars) = *old_vars; - zend_hash_clean(Z_ARRVAL_P(PS(http_session_vars))); - } else { - if(old_vars) { - zend_hash_del(&EG(symbol_table), "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS")); - zend_hash_del(&EG(symbol_table), "_SESSION", sizeof("_SESSION")); - } - MAKE_STD_ZVAL(PS(http_session_vars)); - array_init(PS(http_session_vars)); - PS(http_session_vars)->refcount = 2; - PS(http_session_vars)->is_ref = 1; - zend_hash_update(&EG(symbol_table), "HTTP_SESSION_VARS", sizeof("HTTP_SESSION_VARS"), &PS(http_session_vars), sizeof(zval *), NULL); - zend_hash_update(&EG(symbol_table), "_SESSION", sizeof("_SESSION"), &PS(http_session_vars), sizeof(zval *), NULL); - } + MAKE_STD_ZVAL(PS(http_session_vars)); + array_init(PS(http_session_vars)); + + ZEND_SET_GLOBAL_VAR("HTTP_SESSION_VARS", PS(http_session_vars)); + ZEND_SET_GLOBAL_VAR("_SESSION", PS(http_session_vars)); } static char *php_session_encode(int *newlen TSRMLS_DC)
-- PHP Development Mailing List <http://www.php.net/> To unsubscribe, visit: http://www.php.net/unsub.php