On Fri, September 19, 2014 14:56, Anatol Belski wrote:
> On Fri, September 19, 2014 12:57, Nikita Popov wrote:
>
>> On Fri, Sep 19, 2014 at 12:39 PM, Anatol Belski <a...@php.net> wrote:
>>
>>
>>
>>> Commit:    6bbebc60ea0de6ce09ea45094b3bed1823d96cec
>>> Author:    Anatol Belski <a...@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=6bbebc60ea0de6ce09ea4
>>> 50
>>> 94b3bed1823d96cec
>>>
>>>
>>>
>>> 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?
>>
> Ah, you're right, those will be warned as unreferenced, and even if not -
>  that's an empty call.
>
> A grep reports like 34 usage occurrences in the core, not that many.
> Those
> I've touched was using strlen(), are about 5 AFAIR. In the other cases the
>  path length is delivered. Though, a couple of places can be optimized.
>
> I'll try with the inline function, it's cleaner. If it takes too much,
> will just go through the places I've touched and #ifdef them. Those macros
>  are normally not changed/used that often, so should be ok, too.
>
> Regards
>
>
> Anatol
>
>
>
Found another solution to that, just expand the macro :)

Anatol


-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to