I think there was miscommunication. That's fine. We can discuss them again :)
Sascha Schumann wrote: > Here are comments regarding each commit. > > > http://news.php.net/article.php?group=php.cvs&article=9700 > > I've restored the old behaviour regarding SID (#15322). 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. > > A reordering of estrdups has also been committed. I suppose > the message "fix small memory leak" refers to that.> > These estrdup-related patches were flawed, because they did > not take into account that some values are overwritten during > the execution of the function. So, the estrdup has to occur > earlier. This one, you are right. Messed up badly. Originally, I had estrdup() just after parameter checks, but I messed up at some point. > > http://news.php.net/article.php?group=php.cvs&article=9701 > > Rejected semantical change. You are forcing "user" save handler to return "" string anyway. You are treating meaning of FAILURE from session read function as - failed to read valid session data. where it should be translated - fatal error like network connection lost, db query failed with strange error First tranlation is wrong since: 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) 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.) > > > http://news.php.net/article.php?group=php.cvs&article=9703 > > TSRM API changes, reapplied. The patch was mixed with > anti-mem leak and semantical changes. Those were left out. > > > http://news.php.net/article.php?group=php.cvs&article=9705 > > Good. > > > http://news.php.net/article.php?group=php.cvs&article=9706 > > Flawed and related to the rejected semantical change > regarding the decode process. See above. > > > http://news.php.net/article.php?group=php.cvs&article=9707 > > Does not make sense to me and needs to be discussed further. > Why should we prevent users from changing the session module? I didn't explain why I start using bit vector for status. There are sevral reasons and I explains some now. 1. Since session module functions may not be called sometimes depends of its stauts, if function cannot work, it should fail. 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. There are bug reports for these. Details are in there. > > > http://news.php.net/article.php?group=php.cvs&article=9708 > > Should be reapplied. > > > http://news.php.net/article.php?group=php.cvs&article=9709 > > Why was rinit_mod = 1 removed? (added in 9707) I think I found the real cause in other place. (Not in session module) > > > 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. If we would like to have configurable path, we need new value. > > > http://news.php.net/article.php?group=php.cvs&article=9714 > > Rejected. data->fd is already set by ps_files_close(). I'm not sure if this is not needed or not. It is related to bit vector status and I've get rid of bit-vector status. > > I fail to see how this is related to a segfault in the output > buffering section as discussed in #14232? Does not make sense web server dies when user set save_path to "host=localhost db=session" for their user save handler. If we would like to have configurable path, we need new value. Finally, I have few suggestions - Use return value to indicate if function worked right or not - Do not use implicit status using value other than status. - Treat session module as simple finite state machine. (It a lot easier to maintain) - Handle error as soon as possible. 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 ;) -- Yasuo Ohgaki [EMAIL PROTECTED] -- PHP Development Mailing List <http://www.php.net/> To unsubscribe, visit: http://www.php.net/unsub.php
