ext/phar: More minor refactorings #20186
Conversation
Girgias
force-pushed
the
phar-web-zstr
branch
from
3231092 to
7bee734
Compare
| /* }}} */ | ||
|
|
||
| static void phar_do_404(phar_archive_data *phar, char *fname, size_t fname_len, char *f404, size_t f404_len) /* {{{ */ | ||
| static void phar_do_404(phar_archive_data *phar, char *fname, size_t fname_len, zend_string *f404) /* {{{ */ |
There was a problem hiding this comment.
I don't think this particular commit is an improvement as it just adds extra NULL checks
There was a problem hiding this comment.
My main motivation was to make phar_file_action() use zend_strings in the long run, but to do so I need all the inputs to ideally also be zstrs
There was a problem hiding this comment.
Does phar_file_action benefit that much from zend_strings?
There was a problem hiding this comment.
We could propagate it down to phar_mung_server_vars() to use some nicer zstr APIs and prevent a call to ZVAL_STRINGL()
| #include "stub.h" /* Generated phar_get_stub() function from makestub.php script */ | ||
|
|
||
| zend_string *phar_create_default_stub(const char *index_php, const char *web_index, char **error) /* {{{ */ | ||
| zend_string *phar_create_default_stub(const zend_string *php_index_str, const zend_string *web_index_str, char **error) /* {{{ */ |
There was a problem hiding this comment.
I think the old code in this function is clearer.
High chance that the compiler pre-computes the string length anyway in the !index and !web_index branches, so no need to optimize that.
The changes to "P" instead of "S" in ZPP should be kept though
There was a problem hiding this comment.
Should I also keep the change where it returns NULL in case the path is over 400 even if error == NULL?
There was a problem hiding this comment.
Also the other motivation was to get rid of a strlen call that is not needed
There was a problem hiding this comment.
Should I also keep the change where it returns NULL in case the path is over 400 even if error == NULL?
Yes
Also the other motivation was to get rid of a strlen call that is not needed
Right, but I think the compiler is smart enough to avoid it.
Anyway, reading this again it's clearer to me now, so you may keep it anyway.
…ction() This is incredibly confusing.
Girgias
force-pushed
the
phar-web-zstr
branch
from
7bee734 to
37f456f
Compare
2 participants
No description provided.