-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ext/session: Various refactorings #19041
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
You broke something hard, and it seems related to the ini quantity parsing. I don't think quantity parsing makes a lot of sense for those settings |
20eda53
to
d13ef59
Compare
ext/session/session.c
Outdated
date_fmt = php_format_date("D, d M Y H:i:s \\G\\M\\T", sizeof("D, d M Y H:i:s \\G\\M\\T")-1, t, 0); | ||
smart_str_appends(&ncookie, COOKIE_EXPIRES); | ||
date_fmt = php_format_date(ZEND_STRL("D, d M Y H:i:s \\G\\M\\T"), t, false); | ||
smart_str_appendl(&ncookie, ZEND_STRL(COOKIE_EXPIRES)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For cases like these, I prefer the original code. The strlen() call will almost certainly be evaluated at compile time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I forgot that this is inlined. I was expecting the need for a _literal
suffix API.
To make it easier for IDEs to understand what is going on
This is unnecessary now that the session name is a zend_string
f656530
to
82c1e8c
Compare
Commits should be reviewed one by one