apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject Re: apr_bucket_simple_split
Date Sun, 26 Aug 2001 16:28:32 GMT
From: "Greg Stein" <gstein@lyra.org>
Sent: Sunday, August 26, 2001 4:00 AM


> On Sat, Aug 25, 2001 at 05:09:14PM -0400, Cliff Woolley wrote:
> >...
> >  APU_DECLARE_NONSTD(apr_status_t) apr_bucket_simple_split(apr_bucket *a,
> > -                                                         apr_off_t point)
> > +                                                         apr_size_t point)
> 
> Euh... why is that an apr_size_t? I thought we just talked about that
> earlier this week? A bucket could hold a *lot* more than what can fit in
> memory. I could easily have a FILE bucket that is 6gig in size (say, a video
> file). A FILE bucket can easily be split without needing to read anything
> into memory, so the apr_off_t works quite well.

No.  There really aren't many sendfile implementations that allow you to transmit
more than an apr_size_t, if you start digging the man pages.  Afraid this was a
concensus decision make while you were on holiday.  Since buckets are atomic units,
they need to correspond to an atomic sendfile/read/whatever.

The -brigade- is effectively unlimited.  But individual buckets represent one
'apr_size_t' worth of data.  The code became immensely less complex, and since 
nobody else worked on buckets type/data safety, I fixed this to be consistent
through the server.

> >  {
> >      apr_bucket *b;
> >      apr_status_t rv;
> > 
> > -    if (point < 0 || point > a->length) {
> > +    if ((point > a->length) || (a->length == (apr_size_t)(-1))) {
> >  return APR_EINVAL;
> >      }
> > 
> > 
> > Why check to see if a->length is -1?  That's a waste of time.  If
> > a->length is -1, the bucket type should have never registered a split
> > function in the first place.
> 
> Untrue. Please explain why a PIPE bucket cannot be split at byte 100? Sure,
> it doesn't know its length, but it can easily read in 100 bytes, give you
> that, and leave itself as the second part of the split.

I agree here, we can split an unknown length pipe bucket at known point.  I'd 
suggest that we really want a constant, APR_BUCKET_UNKNOWN_LEN (which needs to 
map to MAX_SIZE_T or MAX_OFF_T, see below).  But I'm not certain that the
apr_bucket_split_simple() should learn how to do this, sounds like a job for
apr_bucket_split_indeterminate() or something.

> > What we really want to know is if point is
> > before the beginning of the bucket or after the end of the bucket.  The
> > original test gave us that.
> > 
> > What am I missing?
> 
> First off, that the point should be an apr_off_t.

-1, and that's a veto, until you (or anyone else) writes a proper patch to handle
all apr-util and http buckets paying attention to the size arguments, throughout,
and mapping between size and len and off.  Until that happens, this is a size_t.

The original bucket implementors paid no attention to transformations of signedness
or integer width.  I'd wasted near a week getting the emits out of the code, and
this was the straigtest line to something that works.  One other obstacle was the
number of headaches when sizeof(apr_off_t)==sizeof(apr_size_t), but the signedness
changes :(

Bill



Mime
View raw message