--- Greg Stein <[EMAIL PROTECTED]> wrote:
> This patch is much better, and I've gone ahead and applied it. However,
> you're discarding the return values from ap_bucket_read(). Those should be
> returned. The prototype ought to look like:
> 
>   apr_status_t ap_brigade_partition(ap_bucket_brigade *b, apr_off_t point,
>                                     ap_bucket **after_point);

Okay.  It was returning a pointer because it was trying to match 
ap_bucket_split() back
in the days when it was _split_offset() returning a brigade.  I have no problem 
with it
returning an apr_status_t now that it has a life of its own.  Patch included.

> Also, this patch (and your last one) was monkeyed up when you inserted it
> into your message. Some lines get wrapped, and the patch doesn't apply
> cleanly. Please attach it, or watch out for wrapping from your mailer agent.

Okay, it's an attachment this time.  Sorry about that.  =-)

Besides changing the return values, one thing this patch does is to ditch the 
checks for
AP_BUCKET_IS_EOS, because they're kind of superfluous... if point==brigade_len,
after_point will be the EOS bucket anyway, so as long as there's never any data 
after the
EOS, this function doesn't need to check for EOS.  That lets us collapse some 
of the
if/elseif/else stuff and have less code duplication.

As far as after_point goes, I've assumed in this patch that after_point need 
only be set
in success cases... does that jive?

--Cliff


__________________________________________________
Do You Yahoo!?
Yahoo! Shopping - Thousands of Stores. Millions of Products.
http://shopping.yahoo.com/

Attachment: bucketpartition.patch
Description: bucketpartition.patch

Reply via email to