[PATCHv2] apr_brigade_partition()

[Cliff Woolley <cl...@yahoo.com>]

--- Greg Stein <gs...@lyra.org> wrote:
> Now that we're actually using this function, I figured it was time to dig in
> and spend the time with this patch. :-)
> *) what happens when you find a bucket with e->length == -1 ? AFAICT, the
>    function doesn't handle it. I'm thinking a check at the top of the loop
>    for a length==-1 and a read(), then do the point/length compare stuff.
> 
> *) after_point looks wrong for the manual-split case. It appears that you'd
>    end up with (B1a, B1b, B2) and return B2 rather than B1b.

Both of these problems should now be taken care of.  My bad.  =-)

One note: I changed a conditional from (point < len) where le is returned from
apr_bucket_read() to (point < e->length) to match a later test for (point == e->length). 
I added a comment explaining why this is okay (namely that len == e->length).  In every
currently existing bucket type, if read wants to return some data that is less than what
the bucket contains, it always morphs the bucket into two buckets and adjusts the
e->length values.  I also am 99% sure that Ryan told me at one point that the current
design doesn't allow for partial bucket reads anyway, which makes sense given what I've
seen.  I guess I really don't understand the point of the len parameter to
apr_bucket_read(), since the value it returns is easily fetched from e->length at any
time.  Most bucket types compute it from end - start as opposed to just using e->length,
but the two should be equal (if they're not, it's a bug AFAICT).

I also added an "outline" for the changes necessary to the byterange filter in Apache
(which is why I copied this message to new-httpd)... but all it does is make the filter
compile with the new API, not do anything useful with the new information.  The part I
left out is that I don't know what the correct way to report an error from
apr_brigade_partition is in terms for handling byteranges.

> Didn't we have another function? A copy() function or something? What
> happened to that one?  (or should I troll my mail archive)

Uhm... apr_brigade_partition has gone through many phases, of course... split_any, then
brigade_split, and now brigade_partition.  But AFAIK, it's the only remaining function in
question.  I mean, we added the bucket copy functions a while back, if that's what you're
thinking of... other than that, I have no idea what you're talking about.  ;-)

I'm attaching the patch rather than inlining it because it contains some long lines that
would wrap with my mail client.  Hope that doesn't cause anybody too much grief.

--Cliff

__________________________________________________
Do You Yahoo!?
Yahoo! Auctions - Buy the things you want at great prices. 
http://auctions.yahoo.com/