apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: [PATCH] ap_brigade_partition() (was Re: brigade/bucket splitting)
Date Tue, 12 Dec 2000 12:20:44 GMT
On Mon, Dec 11, 2000 at 10:15:01PM -0800, Cliff Woolley wrote:
> --- Cliff Woolley <cliffwoolley@yahoo.com> wrote:
> > ...
> > +            if (point < len) {
> > +                if (ap_bucket_split(e, point) == APR_SUCCESS)
> > +                    return AP_BUCKET_NEXT(e);
> > +                else
> > +                    return NULL;
> > +            }
> > +            else if (point == len)
> > +                return AP_BUCKET_NEXT(e);
> > +        }
> >      }
> 
> Oops, I just thought of a little buglet in this bit... I forgot to subtract len from
> point if point > len as is done in the surrounding case.  Following is a revised
> patch with this problem fixed.

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);

I figure that getting in the work-so-far is better than we had, and it isn't
much of a big deal to iterate in source control since nobody is trying to
call this bugger yet. (doing it this way makes it easier for people to
review when a commit message comes by)

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.

Thanks for writing this! We'll get there RSN and then get this code into the
byterange filter (which really needs it!).

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message