Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36429
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 212.25.124.162 as permitted sender)
Message-ID: <47E7B89C.7060708@zend.com>
Date: Mon, 24 Mar 2008 17:20:12 +0300
User-Agent: Thunderbird 2.0.0.9 (Windows/20071031)
MIME-Version: 1.0
To: Gregory Beaver <greg@chiaraquartet.net>
CC: internals Mailing List <internals@lists.php.net>, 
 Stanislav Malyshev <stas@zend.com>
References: <47E2F8FA.20107@chiaraquartet.net> <47E37C42.10308@zend.com> <47E3E7F1.3060200@chiaraquartet.net> <47E792E1.9070803@zend.com> <47E7AC60.1040705@chiaraquartet.net>
In-Reply-To: <47E7AC60.1040705@chiaraquartet.net>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] REMINDER - stream wrappers in include_path
From: dmitry@zend.com (Dmitry Stogov)

REALPATH_FAILED looks like a hack :(
PHP itself doesn't need it at all, according to your patch 
php_stream_open() may just return NULL immediately instead of setting of 
this flag.
However I am not sure that it will work for all cases. The fact that we 
cannot resolve the path doesn't mean that we cannot open the file. (On 
some systems getcwd() may fail on some reasons).

I didn't test the patch with pecl/phar.
Could you explain why php goes into the endless loop?

Thanks. Dmitry.

Gregory Beaver wrote:
> Dmitry Stogov wrote:
>> Hi Greg,
>>
>> In your second patch you forgot "break", so it couldn't work.
>> I don't see any reason in second patch at all, because you call
>> php_resolve_patch() from _php_stream_open_wrapper_ex() anyway.
>>
>> The bad thing with the first part that it calls realpath() function
>> twice for each include("relative_path.php") and it makes several
>> file-system accesses (look into `strace ... 2>&1| grep lstat`). With
>> your patch I see 6 syscalls more.
>>
>> So I would prefer my patch that is more efficient for regular files.
>> The same patch with fixed white-spaces is attached. 
> 
> Your patch is broken, it causes an endless loop in
> tests/include_path.phpt when running phar tests.
> 
> Incidentally, I was unaware of the strace command (I've seen it bandied
> about on irc, but thought it was something else), so thank you for
> edumacatin me there.
> 
> The attached patch has no additional syscalls over the current case for
> both existing and non-existing files, and actually works for phar as
> well :).  It's also simpler (I removed the include stuff from
> zend_vm_def.h), although you may want to add it back in and bench it to
> see if it makes any difference in performance, since I found such a
> dramatic reduction in operations here with it.
> 
> Greg