Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:13450
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Date: Fri, 22 Oct 2004 11:08:42 +0100
To: internals@lists.php.net
Message-ID: <20041022100842.GB9395@redhat.com>
Mail-Followup-To: internals@lists.php.net
References: <20041022062249.GA8983@redhat.com> <4e89b426041022015310a5165f@mail.gmail.com> <20041022094508.GA9395@redhat.com> <4e89b42604102202533086a25d@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <4e89b42604102202533086a25d@mail.gmail.com>
User-Agent: Mutt/1.4.1i
Subject: Re: [PHP-DEV] _FILE_OFFSET_BITS=64 considered harmful
From: jorton@redhat.com (Joe Orton)

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