On Thursday 09 August 2001 20:26, Doug MacEachern wrote:
> On Thu, 9 Aug 2001, MATHIHALLI,MADHUSUDAN (HP-Cupertino,ex1) wrote:
> > Another observation : shouldn't we ought to be checking the return value
> > of apr_brigade_partition (line 682, http_protocol.c)
>
> yes, but the question is what todo if it is not APR_SUCCESS. i have a
> bandaid, which i'm sure is not the correct fix. i plan to look at this
> with rbb tommorrow to get the right fix in place.
Just a quick review.
> Index: modules/http/http_protocol.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
> retrieving revision 1.351
> diff -u -r1.351 http_protocol.c
> --- modules/http/http_protocol.c 2001/08/09 04:56:23 1.351
> +++ modules/http/http_protocol.c 2001/08/10 03:18:36
> @@ -677,11 +677,22 @@
> ### READBYTES bytes, and we wouldn't have to do any work.
> */
>
> - apr_brigade_partition(ctx->b, *readbytes, &e);
> - APR_BRIGADE_CONCAT(b, ctx->b);
> - ctx->b = apr_brigade_split(b, e);
> - apr_brigade_length(b, 1, &total);
> - *readbytes -= total;
> + e = APR_BRIGADE_FIRST(ctx->b);
> + if (e->length == *readbytes) {
> + APR_BUCKET_REMOVE(e);
> + APR_BRIGADE_INSERT_TAIL(b, e);
> + *readbytes = 0;
> + }
> + else if (e->length == 0) {
> + APR_BUCKET_REMOVE(e);
> + }
Why do we need this? If the 0 length bucket is causing a problem with a filter,
that is a bug in the filter.
> +
> + if (*readbytes && apr_brigade_partition(ctx->b, *readbytes, &e) ==
>APR_SUCCESS) {
> + APR_BRIGADE_CONCAT(b, ctx->b);
> + ctx->b = apr_brigade_split(b, e);
> + apr_brigade_length(b, 1, &total);
> + *readbytes -= total;
> + }
We should just check the return code of each of the apr_brigade_ functions,
and return the value if it is not APR_SUCCESS.
>
> /* ### this is a hack. it is saying, "if we have read everything
> ### that was requested, then we are at the end of the request."
> Index: srclib/apr-util/buckets/apr_brigade.c
> ===================================================================
> RCS file: /home/cvs/apr-util/buckets/apr_brigade.c,v
> retrieving revision 1.22
> diff -u -r1.22 apr_brigade.c
> --- srclib/apr-util/buckets/apr_brigade.c 2001/08/08 22:24:04 1.22
> +++ srclib/apr-util/buckets/apr_brigade.c 2001/08/10 03:18:37
> @@ -181,7 +181,7 @@
> }
> }
> if (point == e->length) {
> - *after_point = APR_BUCKET_NEXT(e);
> + *after_point = e;
> return APR_SUCCESS;
> }
> point -= e->length;
This looks wrong to me. We are in apr_brigade_partition, and we are setting the return
parameter, after_point, which should be the first bucket after the offset we are
splitting at.
This change has us returning the bucket that contains the offset we are splitting at.
Ryan
_____________________________________________________________________________
Ryan Bloom [EMAIL PROTECTED]
Covalent Technologies [EMAIL PROTECTED]
-----------------------------------------------------------------------------
