Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » primary file realpath change »

primary file realpath change

php.internals

Brian J. France

20 years ago
Does anybody see a problem with this patch (which is currently against the 5.1.2 release)? expand_filepath will eventual do a realpath, but it also uses the realpath cache before calling realpath. VCWD_REALPATH just maps directly to realpath and doesn't use the realpath cache so for every request you get a realpath call (and all the syscalls that go with it) on the primary file. I think I need to pass TSRMLS_CC, but I am not sure. Any problems committing it to PHP_5_1 and HEAD? Brian --- ../php-5.1.2.orig/main/main.c Sun Jan 1 04:50:17 2006 +++ main/main.c Thu Mar 9 20:00:47 2006 @@ -1684,7 +1684,7 @@ char realfile[MAXPATHLEN]; int realfile_len; int dummy = 1; - if (VCWD_REALPATH(primary_file->filename, realfile)) { + if (expand_filepath(primary_file->filename, realfile TSRMLS_CC)) { realfile_len = strlen(realfile); zend_hash_add(&EG(included_files), realfile, realfile_len+1, (void *)&dummy, sizeof(int), NULL); if (strncmp(realfile, primary_file-

Andi Gutmans

20 years ago
Are you sure VCWD_REALPATH doesn't use the realpath cache? It did last time I checked and I think is still the right method to use there... At 09:13 PM 3/9/2006, Brian J. France wrote:

Rasmus Lerdorf

20 years ago
Andi Gutmans wrote:
> Are you sure VCWD_REALPATH doesn't use the realpath cache? It did last > time I checked and I think is still the right method to use there...
quite #define VCWD_REALPATH(path, real_path) realpath(path, real_path) -Rasmus

Rasmus Lerdorf

20 years ago
Rasmus Lerdorf wrote:
> Andi Gutmans wrote: >> Are you sure VCWD_REALPATH doesn't use the realpath cache? It did last >> time I checked and I think is still the right method to use there... > > quite > > #define VCWD_REALPATH(path, real_path) realpath(path, real_path)
By the way, I agree that expand_filepath() is not the right fix here since the first thing it does is a getcwd. We have just done a chdir and haven't executed any script yet so we know the cwd. Feed that directly to virtual_file_ex() there perhaps? I haven't really dug into it. -Rasmus

Brian J. France

20 years ago
On Mar 10, 2006, at 1:57 AM, Rasmus Lerdorf wrote:
> Rasmus Lerdorf wrote: >> Andi Gutmans wrote: >>> Are you sure VCWD_REALPATH doesn't use the realpath cache? It did >>> last time I checked and I think is still the right method to use >>> there... >> quite >> #define VCWD_REALPATH(path, real_path) realpath(path, real_path) > > By the way, I agree that expand_filepath() is not the right fix > here since the first thing it does is a getcwd. We have just done > a chdir and haven't executed any script yet so we know the cwd. > Feed that directly to virtual_file_ex() there perhaps? I haven't > really dug into it.
Were you seeing two getcwd calls? I can remove one, but I don't see away around the second one. I changed to virtual_realpath which does remove the getcwd in php_execute_script function, but it seems that is shows up a little later when it uses the stream open which called expand_filepath: #0 0x203ab410 in __getcwd () from /usr/lib/libc.so.4 #1 0x203b2ab6 in getcwd () from /usr/lib/libc.so.4 #2 0x2060de86 in expand_filepath (filepath=0x2cbc08 "<path>/ bench_main.php", real_path=0x0) at main/fopen_wrappers.c:521 #3 0x2061c1bd in _php_stream_fopen (filename=0x2cbc08 "<path>/ bench_main.php", mode=0x206f1fe3 "rb", opened_path=0x9fbff7cc, options=133) at /main/streams/plain_wrapper.c:855 #4 0x2061c974 in _php_stream_fopen_with_path (filename=0x2cbc08 "<path>/bench_main.php", mode=0x206f1fe3 "rb", path=0x12b640 ".:/<path>/pear", opened_path=0x9fbff7cc, options=133) at /main/streams/plain_wrapper.c:1238 #5 0x2061c338 in php_plain_files_stream_opener (wrapper=0x2077c008, path=0x2cbc08 "<path>/bench_main.php", mode=0x206f1fe3 "rb", options=133, opened_path=0x9fbff7cc, context=0x0) at/main/streams/plain_wrapper.c:931 #6 0x2061962e in _php_stream_open_wrapper_ex (path=0x2cbc08 "<path>bench_main.php", mode=0x206f1fe3 "rb", options=141, opened_path=0x9fbff7cc, context=0x0) at main/streams/streams.c:1771 #7 0x2060943b in php_stream_open_for_zend (filename=0x2cbc08 "<path>/ bench_main.php", handle=0x9fbff7c4) at main/main.c:901 #8 0x2064a866 in zend_stream_open (filename=0x2cbc08 "<path>/ bench_main.php", handle=0x9fbff7c4) at Zend/zend_stream.c:47 #9 0x2064a8dc in zend_stream_fixup (file_handle=0x9fbff7c4) at Zend/ zend_stream.c:62 #10 0x20623ee7 in open_file_for_scanning (file_handle=0x9fbff7c4) at Zend/zend_language_scanner.c:2982 #11 0x20623fce in compile_file (file_handle=0x9fbff7c4, type=2) at Zend/zend_language_scanner.c:3068 #12 0x2063cd1c in zend_execute_scripts (type=8, retval=0x0, file_count=3) at Zend/zend.c:1093 #13 0x2060a653 in php_execute_script (primary_file=0x9fbff7c4) at main/main.c:1724 Brian

Rasmus Lerdorf

20 years ago
After poking this a bit more, it looks like this whole block of code in main.c is redundant. Here is the story: We start our journey on a dark and stormy night in June 2002 with this bug: http://bugs.php.net/17720 This led to this fix: http://cvs.php.net/viewcvs.cgi/php-src/main/main.c?r1=1.461&r2=1.462 which had this commit message: Main script should not be parsed when (include|require)_once()'ed, #17720 # Derick, MFH? Now, at the time this was the correct fix. However, fast forward to March 2005 and we have a patch from Stas to zend_execute_scripts: http://cvs.php.net/viewcvs.cgi/ZendEngine2/zend.c?view=diff&r1=1.297&r2=1.298 which adds the opened file to the included_files list. This means that we are now trying to add the main script to the included_file list twice which in itself is bad. We'll test this a bit more, but it looks safe to remove that block of code. The test case in bug 17720 works fine without that block of code in main.c and I see no other reason to have it there. -Rasmus Brian J. France wrote:

Rasmus Lerdorf

20 years ago
Rasmus Lerdorf wrote:
> After poking this a bit more, it looks like this whole block of code in > main.c is redundant. Here is the story: > > We start our journey on a dark and stormy night in June 2002 with this > bug: http://bugs.php.net/17720 > > This led to this fix: > http://cvs.php.net/viewcvs.cgi/php-src/main/main.c?r1=1.461&r2=1.462 > which had this commit message: > Main script should not be parsed when (include|require)_once()'ed, #17720 > # Derick, MFH? > > Now, at the time this was the correct fix. However, fast forward to > March 2005 and we have a patch from Stas to zend_execute_scripts: > http://cvs.php.net/viewcvs.cgi/ZendEngine2/zend.c?view=diff&r1=1.297&r2=1.298 > > which adds the opened file to the included_files list. This means that > we are now trying to add the main script to the included_file list twice > which in itself is bad. > > We'll test this a bit more, but it looks safe to remove that block of > code. The test case in bug 17720 works fine without that block of code > in main.c and I see no other reason to have it there.
Just as a final follow-up to this. Further testing showed that in CLI mode where we pass in a ZEND_HANDLE_FP instead of a ZEND_HANDLE_FILENAME we still need this block of code. So the fix is to check and only skip this block if we are passed a ZEND_HANDLE_FILENAME. Commit coming up. -Rasmus

Andi Gutmans

20 years ago
Seems that needs fixing then (non-TSRM). We should support the realpath cache also in non-TSRM mode. At 10:31 PM 3/9/2006, Rasmus Lerdorf wrote: