apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Stoddard" <b...@wstoddard.com>
Subject Re: cvs commit: apr-util/buckets apr_buckets_file.c
Date Thu, 05 Jul 2001 14:55:49 GMT
What specific problem are we trying  to fix?  I agree for the need for a seek() in the
case you are describing below but it seems to be an unusual case, at least for the
webserver. We need to focus on making APR work efficiently for the most common cases and
resist the urge to create an overgeneralized solutions to every problem.  Apache httpd
already has serious performance problems and patches like this are not helping us move in
the right direction/


> On Thu, 5 Jul 2001, Bill Stoddard wrote:
> > If you perform a read on a file and don't specifiy an offset, then you
> > should assume you will be reading from the current file pointer
> > maintained by the system (or by apr_file_t in the XTHREAD case).  If
> > you have an apr_file_t open and you are reading from the file
> > someplace else using the fd, then you are screwed. You shouldn't be
> > mixing apr_file_* calls with non apr_file_* calls on the same fd. If
> > you insist on doing this, then you need to ensure that your non
> > apr_file_* calls leaves the file pointer in the proper state when they
> > are done.  You definitely shouldn't be horking up apr_file* calls to
> > defend against this case.
> [scratching head]
> I don't think we're talking about mixing apr_file_* and non apr_file_*
> calls.  A file bucket *always* specifies an offset [although sometimes
> that offset might be 0].
> Consider this:
>     apr_file_t *f = ...
>     apr_bucket *a, *b;
>     a = apr_bucket_file_create(f, 0, len, pool);
>     b = apr_bucket_split(a, 500);
> Now you have two file buckets referencing the same apr_file_t, one at
> offset 0 and one at offset 500.  The one at offset 500 is BEFORE the one
> at offset 0 in the brigade.  When you read from the buckets in the brigade
> (assume for a minute that !APR_HAS_MMAP), you get to the offset==500
> bucket first, seek to offset 500, and read from there.  Then you get to
> the offset==0 bucket, so you sure as hell better seek back to offset 0
> before you read!  Not doing so is a bug in the file buckets code, not in
> APR.  If you want to combine the apr_file_seek/apr_file_read calls, that's
> fine, but you'll STILL need to have removed this offset-test conditional.
> So Ryan's patch is a correct fix either way, not just a hack.
> --Cliff
> --------------------------------------------------------------
>    Cliff Woolley
>    cliffwoolley@yahoo.com
>    Charlottesville, VA

View raw message