On Wed, 16 Jan 2008 23:10:36 -0500 "Moshe Weitzman" <weitzman@tejasa.com> wrote:
i can't really get excited one way or another. if you want a universal way to get session id, call session_id(). your patch is a very slight improvement, i suppose.
I'm not excited as well... because it's wrong. Thank you for pointing it out in a gentle way (very slightly improvement <g>). It is not at all an improvement... it is error prone mixing current user sid with sid of an user object. Previously the code was: Index: includes/session.inc =================================================================== --- includes/session.inc (revisione 419) +++ includes/session.inc (copia locale) @@ -30,6 +30,7 @@ // Otherwise, if the session is still active, we have a record of the client's session in the database. $user = db_fetch_object(db_query("SELECT u.*, s.* FROM {users} u INNER JOIN {sessions} s ON u.uid = s.uid WHERE s.sid = '%s'", $key)); + $sid=$user->sid; // We found the client's session record and they are an authenticated user if ($user && $user->uid > 0) { @@ -48,6 +49,7 @@ else { $session = isset($user->session) ? $user->session : ''; $user = drupal_anonymous_user($session); + $user->sid=$sid; } return $user->session; The situation I'm trying to deal with are anonymous users who own temporary objects not suited to be placed in $session, eg. e-commerce carts. I didn't use $key to fill $user->sid because I need a pk in {sessions}. Most of my functions accept a $user as argument so I can manipulate the current user or a user from the admin interface. They see if the $user is authenticated, otherwise they resort to the sid to find the objects. Does it make any more sense now? Never pretend you can judge if your own code is elegant near midnight... especially if you went to sleep at 5:00am the previous day. Sorry. -- Ivan Sergio Borgonovo http://www.webthatworks.it