-
Notifications
You must be signed in to change notification settings - Fork 8k
main/php_ini.c: remove dead code #4512
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
f41df19 to
93741a5
Compare
cc @nikic I believe is the right ping here for the review. :) Merging dead code to PHP 7.4 is from my point of view still ok. Less diffs between 7.4 and 8.0 is very smart if we can do that.
It might make more sense to factor out this repetitive code-pattern into a function (append_ini_path or something).
@nikic exactly my idea. have some static method inside that same source file to avoid the repeative code.
ideally, I'd move those large platform-specific #ifdef blocks inside method also as separate static functions, then the main function is easier to follow.
86067f3 to
f4ecfac
Compare
... added something
main/php_ini.c
Outdated
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.
I think the size parameter should be min(size, MAXPATHLEN), not to overflow phprc_path.
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.
@nikic fix this or leave as is?
Appveyor:
main\php_ini.c(419): warning C4101: 'reg_location': unreferenced local variable
main\php_ini.c(524): error C2065: 'reg_location': undeclared identifier
main\php_ini.c(524): warning C4047: '=': 'int' differs in levels of indirection from 'char *'
main\php_ini.c(525): error C2065: 'reg_location': undeclared identifier
main\php_ini.c(525): warning C4047: '!=': 'int' differs in levels of indirection from 'void *'
main\php_ini.c(526): error C2065: 'reg_location': undeclared identifier
main\php_ini.c(526): warning C4047: 'function': 'char *' differs in levels of indirection from 'int'
main\php_ini.c(526): warning C4024: 'append_ini_path': different types for formal and actual parameter 3
main\php_ini.c(527): error C2065: 'reg_location': undeclared identifier
main\php_ini.c(527): warning C4022: '_efree': pointer mismatch for actual parameter 1
php_scandir.c
moved too much, fixed 62b362e
3082610 to
a1735a4
Compare
do you want me to rebase this to lowest branch? or maintainers can handle that? and in that case, what is that branch?
main/php_ini.c
Outdated
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.
You are returning a reference to stack memory here. The pointer will be invalidated when the function returns.
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.
suggestions? always strdup value, return NULL if no value? playing with is_allocated variable seems overkill, as this code path is invoked only on init (not request init)?
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.
One option would be to not return anything from get_env_location, but to pass the buffer into the function.
What's the status of this RFC? Is it still relevant? Should it target PHP-8.0?
150d268 to
201a4b2
Compare
Only 3 years, 4 months to merge! thanks :)
Uh oh!
There was an error while loading. Please reload this page.
main/php_ini.c:492 always initializes
php_ini_search_pathbeing empty:That change was introduced in acb1e07 by @dstogov