Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » _FILE_OFFSET_BITS=64 considered harmful »

_FILE_OFFSET_BITS=64 considered harmful

php.internals

Joe Orton

21 years ago
There are serious problems from enabling LFS support like this in a project like PHP. If I have some library which uses off_t in its API, e.g. zlib, and I happened to not compile it with LFS support, e.g. as in most Linux distributions, I now *cannot* call the zlib functions using off_t from PHP, because the size of off_t when you #include <zlib.h> will be different from the size when zlib is compiled. Similar cases for any SAPI module if the server interface exposes off_t. The safe way of doing this which we use in APR 1.0 is to define a wrapper type (e.g php_off_t) which is typedef'd to off64_t when -D_LARGEFILE64_SOURCE is supported, and off_t otherwise, to open files using O_LARGEFILE, and to use the "transitional LFS" functions like lseek64 etc in place of lseek etc when php_off_t is off64_t. ----- Forwarded message from Edin Kadribasic <edink@php.net> ----- From: "Edin Kadribasic" <edink@php.net> To: php-cvs@lists.php.net Date: Thu, 21 Oct 2004 23:57:39 -0000 Subject: [PHP-CVS] cvs: php-src(PHP_5_0) / configure.in edink Thu Oct 21 19:57:39 2004 EDT Modified files: (Branch: PHP_5_0) /php-src configure.in Log: MFB: Linux LFS support, fixes 27792 http://cvs.php.net/diff.php/php-src/configure.in?r1=1.514.2.11&r2=1.514.2.12&ty=u Index: php-src/configure.in diff -u php-src/configure.in:1.514.2.11 php-src/configure.in:1.514.2.12 --- php-src/configure.in:1.514.2.11 Tue Sep 28 05:13:32 2004 +++ php-src/configure.in Thu Oct 21 19:57:37 2004 @@ -1,4 +1,4 @@ -dnl ## $Id: configure.in,v 1.514.2.11 2004/09/28 09:13:32 wez Exp $ -*- sh -*- +dnl ## $Id: configure.in,v 1.514.2.12 2004/10/21 23:57:37 edink Exp $ -*- sh -*- dnl ## Process this file with autoconf to produce a configure script. divert(1) @@ -196,6 +196,13 @@ LIBS="$LIBS -lbe -lroot";; *mips*) CPPFLAGS="$CPPFLAGS -D_XPG_IV";; +*linux*) + AC_MSG_CHECKING([for Linux LFS_CFLAGS]) + LFS_LINUX_CFLAGS=`getconf LFS_CFLAGS` + if test "x$LFS_LINUX_CFLAGS" != "x"; then + CFLAGS="$CFLAGS $LFS_LINUX_CFLAGS" + fi + AC_MSG_RESULT([$LFS_LINUX_CFLAGS]) esac
-- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php ----- End forwarded message -----

Wez Furlong

21 years ago
What I planned to do with the streams API for 5.1 was define php_stream_off_t to be a 64-bit type (regardless of LFS support), adjust the API where it is needed, and handle the LFS stuff centrally, using the transitional LFS functions you mentioned if they are present. I confess that I haven't delved in to LFS on *nix very deeply at this point, so if you have any tips, I'd welcome your opinion. --Wez. On Fri, 22 Oct 2004 07:22:49 +0100, Joe Orton <jorton@redhat.com> wrote:

Joe Orton

21 years ago
On Fri, Oct 22, 2004 at 09:53:14AM +0100, Wez Furlong wrote:
> What I planned to do with the streams API for 5.1 was define > php_stream_off_t to be a 64-bit type (regardless of LFS support), > adjust the API where it is needed, and handle the LFS stuff centrally, > using the transitional LFS functions you mentioned if they are > present.
The difference in that approach is that you introduce a bunch of new error cases on platforms *without* LFS support, where you'd have to check for passing an out-of-range 64-bit php_stream_off_t value to a function taking a 32-bit off_t. I think using php_stream_off_t = off_t or off64_t as appropriate is simpler for that reason. But both approaches are feasible, the important thing is to avoid using -D_FILE_OFFSET_BITS=64, which just breaks so much. Regards, joe

Wez Furlong

21 years ago
On Fri, 22 Oct 2004 10:45:08 +0100, Joe Orton <jorton@redhat.com> wrote:
> On Fri, Oct 22, 2004 at 09:53:14AM +0100, Wez Furlong wrote: > > What I planned to do with the streams API for 5.1 was define > > php_stream_off_t to be a 64-bit type (regardless of LFS support), > > adjust the API where it is needed, and handle the LFS stuff centrally, > > using the transitional LFS functions you mentioned if they are > > present. > > The difference in that approach is that you introduce a bunch of new > error cases on platforms *without* LFS support, where you'd have to > check for passing an out-of-range 64-bit php_stream_off_t value to a > function taking a 32-bit off_t. I think using php_stream_off_t = off_t > or off64_t as appropriate is simpler for that reason.
I think it is important to force the size of the type, because we need to be aware of the size of the type when passing it to/from zvals. I'm happy with checking the range for non-LFS platforms, as this should have minimal impact compared to the cost of actually operating on the descriptor, and helps us to standardize on the API.
> But both approaches are feasible, the important thing is to avoid using > -D_FILE_OFFSET_BITS=64, which just breaks so much.
OK, so we should roll back Edins patch from all branches and wait for a safer fix. --Wez.

Joe Orton

21 years ago
On Fri, Oct 22, 2004 at 10:53:25AM +0100, Wez Furlong wrote:
> On Fri, 22 Oct 2004 10:45:08 +0100, Joe Orton <jorton@redhat.com> wrote: > > On Fri, Oct 22, 2004 at 09:53:14AM +0100, Wez Furlong wrote: > > > What I planned to do with the streams API for 5.1 was define > > > php_stream_off_t to be a 64-bit type (regardless of LFS support), > > > adjust the API where it is needed, and handle the LFS stuff centrally, > > > using the transitional LFS functions you mentioned if they are > > > present. > > > > The difference in that approach is that you introduce a bunch of new > > error cases on platforms *without* LFS support, where you'd have to > > check for passing an out-of-range 64-bit php_stream_off_t value to a > > function taking a 32-bit off_t. I think using php_stream_off_t = off_t > > or off64_t as appropriate is simpler for that reason. > > I think it is important to force the size of the type, because we need to > be aware of the size of the type when passing it to/from zvals. > I'm happy with checking the range for non-LFS platforms, > as this should have minimal impact compared to the cost of actually > operating on the descriptor, and helps us to standardize on the API.
It's not the runtime cost which was my concern for APR so much as the cost of more complex code and the risk of having more if() conditions which aren't exercised in testing. But in httpd/APR we can ensure that file offsets are stored in apr_off_t's in 99% of the code so it's a different ball game if you need to pass these about with zvals I guess.
> > But both approaches are feasible, the important thing is to avoid > > using -D_FILE_OFFSET_BITS=64, which just breaks so much. > > OK, so we should roll back Edins patch from all branches and wait for > a safer fix.
Gets my vote... joe

Edin Kadribasic

21 years ago
On Friday 22 October 2004 12:08, Joe Orton wrote:
> > > But both approaches are feasible, the important thing is to avoid > > > using -D_FILE_OFFSET_BITS=64, which just breaks so much.
As far as I can tell we use off_t only once when calling external libraries. And yes it is in a call to zlib when seeking inside gzipped files. However I consider crashing apache children with signal 25 when doing simple is_file() or fopen() on large files really harmful. Apache flat out refuses to start if you have enabled php error log and that file happen to be 2GB or bigger.
> > OK, so we should roll back Edins patch from all branches and wait for > > a safer fix. > > Gets my vote...
OK. Reverted. Edin

Joe Orton

21 years ago
On Fri, Oct 22, 2004 at 12:59:04PM +0200, Edin Kadribasic wrote:
> However I consider crashing apache children with signal 25 when doing simple > is_file() or fopen() on large files really harmful. Apache flat out refuses > to start if you have enabled php error log and that file happen to be 2GB or > bigger.
For Linux at least that was fixed since 2.0.50, or are you using 1.3 still? w.r.t is_file() or open(), can you give more details? open() on a >2Gb file will give EFBIG not die with SIGXFSZ. *writing* beyond 2Gb will die with SIGXFSZ, yes, is that what you mean? I had a patch to make sure SIGXFSZ was ignored but didn't bother with it since the large logfile stuff went in which solves the common problem. Regards, joe

Edin Kadribasic

21 years ago
On Friday 22 October 2004 13:33, Joe Orton wrote:
> On Fri, Oct 22, 2004 at 12:59:04PM +0200, Edin Kadribasic wrote: > > However I consider crashing apache children with signal 25 when doing > > simple is_file() or fopen() on large files really harmful. Apache flat > > out refuses to start if you have enabled php error log and that file > > happen to be 2GB or bigger. > > For Linux at least that was fixed since 2.0.50, or are you using 1.3 > still?
I still use 1.3 (like, I guess, most of the PHP installations outthere).
> w.r.t is_file() or open(), can you give more details? open() on a >2Gb > file will give EFBIG not die with SIGXFSZ. *writing* beyond 2Gb will > die with SIGXFSZ, yes, is that what you mean? I had a patch to make > sure SIGXFSZ was ignored but didn't bother with it since the large > logfile stuff went in which solves the common problem.
Yes, fwrite() will die with SIGXFSZ. is_file() file_exists() and friends will fail returning bugus results. A number of bug reports mention these issues. Just a quick google search gave me #15260 #27765 #24411 #27792. Edin

Joe Orton

21 years ago
On Fri, Oct 22, 2004 at 02:26:50PM +0200, Edin Kadribasic wrote:
> On Friday 22 October 2004 13:33, Joe Orton wrote: > > On Fri, Oct 22, 2004 at 12:59:04PM +0200, Edin Kadribasic wrote: > > > However I consider crashing apache children with signal 25 when doing > > > simple is_file() or fopen() on large files really harmful. Apache flat > > > out refuses to start if you have enabled php error log and that file > > > happen to be 2GB or bigger. > > > > For Linux at least that was fixed since 2.0.50, or are you using 1.3 > > still? > > I still use 1.3 (like, I guess, most of the PHP installations outthere).
Then consider this your official wake-up call :)
> > w.r.t is_file() or open(), can you give more details? open() on a >2Gb > > file will give EFBIG not die with SIGXFSZ. *writing* beyond 2Gb will > > die with SIGXFSZ, yes, is that what you mean? I had a patch to make > > sure SIGXFSZ was ignored but didn't bother with it since the large > > logfile stuff went in which solves the common problem. > > Yes, fwrite() will die with SIGXFSZ.
I'll fix this in httpd, hopefully for 2.0.53. joe

Ilia A.

21 years ago
Joe Orton wrote:
> Then consider this your official wake-up call :)
LOL. I use Apache 1.3 and found it to work way better then 2.0. 2.0 does not offer any useful improvements over the 1.3 base, so why switch? If anything in my personal experience 1.3 works way better then 2.0 with PHP. Ilia

Derick Rethans

21 years ago
On Fri, 22 Oct 2004, Joe Orton wrote:
> > I still use 1.3 (like, I guess, most of the PHP installations outthere). > > Then consider this your official wake-up call :)
Perhaps we can consider that when Apache 2 uses static modules like Apache 1.3. But for now Apache 1.3 works great, and that can not always be said about Apache 2, so I don't see a real reason why to switch over. Derick
-- Derick Rethans http://derickrethans.nl | http://ez.no | http://xdebug.org

Edin Kadribasic

21 years ago
On Friday 22 October 2004 15:42, Joe Orton wrote:
> On Fri, Oct 22, 2004 at 02:26:50PM +0200, Edin Kadribasic wrote: > > On Friday 22 October 2004 13:33, Joe Orton wrote: > > > On Fri, Oct 22, 2004 at 12:59:04PM +0200, Edin Kadribasic wrote: > > > > However I consider crashing apache children with signal 25 when doing > > > > simple is_file() or fopen() on large files really harmful. Apache > > > > flat out refuses to start if you have enabled php error log and that > > > > file happen to be 2GB or bigger. > > > > > > For Linux at least that was fixed since 2.0.50, or are you using 1.3 > > > still? > > > > I still use 1.3 (like, I guess, most of the PHP installations outthere). > > Then consider this your official wake-up call :)
This is unrealistic. I'd rather have PHP working with my Apache 1.3 without crashing it. So so far _FILE_OFFSET_BITS=64 fixes one crash, makes is_*() functions work, allows opening an writing large files. On the downside it creates binary compatibility problems with external libraries which use off_t in function prototypes (I found one such call in php4 source, but it could be that I have missed some). I know I will have it enabled in all my PHP builds... Edin

Andi Gutmans

21 years ago
At 02:42 PM 10/22/2004 +0100, Joe Orton wrote:
> > > For Linux at least that was fixed since 2.0.50, or are you using 1.3 > > > still? > > > > I still use 1.3 (like, I guess, most of the PHP installations outthere). > >Then consider this your official wake-up call :)
Most people are still using 1.3.x. I don't personally see a compelling reason to move to 2.0.x. Andi