Re: com php-src: avoid multiple strlen calls for the same buffer: Zend/zend_virtual_cwd.c

On Fri, Sep 19, 2014 at 12:39 PM, Anatol Belski <ab@php.net> wrote: > Commit: 6bbebc60ea0de6ce09ea45094b3bed1823d96cec > Author: Anatol Belski <ab@php.net> Fri, 19 Sep 2014 12:39:17 > +0200 > Parents: d8de53d498cf75936c9cd6781456c39b01f5af60 > Branches: master > > Link: > > http://git.php.net/?p=php-src.git;a=commitdiff;h=6bbebc60ea0de6ce09ea45094b3bed1823d96cec > > Log: > avoid multiple strlen calls for the same buffer > > Changed paths: > M Zend/zend_virtual_cwd.c > > > Diff: > diff --git a/Zend/zend_virtual_cwd.c b/Zend/zend_virtual_cwd.c > index 665829d..acc83ec 100644 > --- a/Zend/zend_virtual_cwd.c > +++ b/Zend/zend_virtual_cwd.c > @@ -1442,16 +1442,20 @@ CWD_API char *virtual_realpath(const char *path, > char *real_path TSRMLS_DC) /* { > if (VCWD_GETCWD(cwd, MAXPATHLEN)) { > path = cwd; > } > - } else if (!IS_ABSOLUTE_PATH(path, strlen(path))) { > - CWD_STATE_COPY(&new_state, &CWDG(cwd)); > } else { > - new_state.cwd = (char*)emalloc(1); > - if (new_state.cwd == NULL) { > - retval = NULL; > - goto end; > + size_t path_len = strlen(path); > + > + if (!IS_ABSOLUTE_PATH(path, path_len)) { > + CWD_STATE_COPY(&new_state, &CWDG(cwd)); > + } else { > + new_state.cwd = (char*)emalloc(1); > + if (new_state.cwd == NULL) { > + retval = NULL; > + goto end; > + } > + new_state.cwd[0] = '\0'; > + new_state.cwd_length = 0; > } > - new_state.cwd[0] = '\0'; > - new_state.cwd_length = 0; > } > > if (virtual_file_ex(&new_state, path, NULL, CWD_REALPATH > TSRMLS_CC)==0) { > @@ -1967,17 +1971,21 @@ CWD_API char *tsrm_realpath(const char *path, char > *real_path TSRMLS_DC) /* {{{ > if (VCWD_GETCWD(cwd, MAXPATHLEN)) { > path = cwd; > } > - } else if (!IS_ABSOLUTE_PATH(path, strlen(path)) && > - VCWD_GETCWD(cwd, MAXPATHLEN)) { > - new_state.cwd = estrdup(cwd); > - new_state.cwd_length = strlen(cwd); > } else { > - new_state.cwd = (char*)emalloc(1); > - if (new_state.cwd == NULL) { > - return NULL; > + size_t path_len = strlen(path); > + > + if (!IS_ABSOLUTE_PATH(path, strlen(path)) && > + VCWD_GETCWD(cwd, MAXPATHLEN)) { > + new_state.cwd = estrdup(cwd); > + new_state.cwd_length = strlen(cwd); > + } else { > + new_state.cwd = (char*)emalloc(1); > + if (new_state.cwd == NULL) { > + return NULL; > + } > + new_state.cwd[0] = '\0'; > + new_state.cwd_length = 0; > } > - new_state.cwd[0] = '\0'; > - new_state.cwd_length = 0; > } > > if (virtual_file_ex(&new_state, path, NULL, CWD_REALPATH > TSRMLS_CC)) { > In the second case, you forgot to replace the strlen(path) in the IS_ABSOLUTE_PATH invocation. Also, these changes will cause unused variable warnings on non-windows systems, because IS_ABSOLUTE_PATH only uses the length on windows. I'm not sure what's the best way to avoid that - maybe make IS_ABSOLUTE_PATH an inline function instead of a macro? Nikita