apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Cliff Woolley <cliffwool...@yahoo.com>
Subject Re: [PATCH] ap_brigade_partition() (was Re: brigade/bucket splitting)
Date Tue, 12 Dec 2000 17:11:07 GMT

--- Greg Stein <gstein@lyra.org> 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?


Do You Yahoo!?
Yahoo! Shopping - Thousands of Stores. Millions of Products.
View raw message