Sascha Schumann wrote:
>>I thought the reporter want valid SID defined always, since I wanted
>>valid SID defined always. Document does not memtion when SID is
>>defined, IIRC.
>>
>>Document should be updated.
>>
> 
>     It has been documented since I first wrote the session
>     documentation.
> 
>     "Alternatively, you can use the constant SID which is defined,
>     if the client did not send the appropriate cookie. SID is
>     either of the form session_name=session_id or is an empty
>     string.

I'll check and update doc if needed.

> 
> 
>>You are forcing "user" save handler to return "" string anyway.
>>
> 
>     No.  Try to read the code again.  A user handler should
>     simply return a non-string.

It works sometimes, but it segfaults sometimes. (with original code)

> 
> 
>>1. It does not work simply with current code.
>>    (It seems mm works for you. While it does not for me. I guess
>>     you enable register_globals)
>>
> 
>     I don't which you can easily derive from the test script I
>     sent to you earlier.

Which test script?

> 
> 
>>2. There is no way to return fatal error from reading.
>>    (This could be serious design flaw, since when read failed
>>     due to lost connection,etc, write function should never try
>>     to write.)
>>
> 
>     A php function can itself halt the script execution, so there
>     is no need to enforce bailing out.

It's not my point.
Current code is not obvious that session save handler does not
write bogus session data when read is failed with other errors
like network and database.

> 
> 
>>1. Since session module functions may not be called sometimes depends
>>of its stauts, if function cannot work, it should fail.
>>
> 
>     This is completely independent of using a bit-vector and is
>     easily achieved using php_session_status.

Of couse, we can do that.
We just have to type more.

> 
>>2. Since original code uses PS(mod) and it's member for current
>>status of session module (or save handler), you failed to write
>>code correctly. It's much easier and cleaner to use single
>>status value to represent session status.
>>
> 
>     I don't get what you are trying to say there.

AFAIK, session module has 2 bugs due to this design.
I suggest to use one single status to indicate current session
module's status.

> 
> 
>>>    http://news.php.net/article.php?group=php.cvs&article=9712
>>>
>>>    Does not make sense.  Die, if session module cannot be
>>>    initialized.
>>>
>>Does not make sense web server dies when user set save_path to
>>"host=localhost db=session" for their user save handler.
>>
> 
>     Then the user should either set save_handler to "user" or
>     not change it all (files is perfectly happy then).

files are happy, since it shares save_path. What about
others?

> 
> 
>>If we would like to have configurable path, we need new value.
>>
> 
>     No.

Then static path?

>>  - Use return value to indicate if function worked right or not
>>
> 
>     Yes, this makes sense at a couple of places.
> 
> 
>>  - Do not use implicit status using value other than status.
>>
> 
>     Parse error..

Do not use implicit status value other than variable that is
dedicated for status.

Is this better?

> 
> 
>>  - Treat session module as simple finite state machine.
>>    (It a lot easier to maintain)
>>  - Handle error as soon as possible.
>>
> 
>     I.e. when the server starts up.

If we can decide 100% error and should never allow that,
yes. But it's not the case for mm start up.

>>Most of problems that I've found is related to these.
>>
>>BTW, you are better not to say other's patches are bogus.
>>We know there are sevral serious bugs. One can easily argue
>>that your code or coding style is bogus.
>>But, it's not productive at all ;)
>>

I was spending my time to fix sevral serious sessoin module
bugs, but it seems we have a lot different opinion for
coding style.

I'll just keep my own session module bug free for my usage.
(I leave session module as is. Fix other bugs)

-- 
Yasuo Ohgaki
[EMAIL PROTECTED]


-- 
PHP Development Mailing List <http://www.php.net/>
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to