Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Date: Wed, 10 Oct 2001 22:14:04 -0700
From: Aaron Bannert <aaron@clove.org>
To: dev@httpd.apache.org
Subject: Re: cvs commit: httpd-2.0/modules/http http_protocol.c
Message-ID: <20011010221404.N7571@clove.org>
References: <20011011044014.43733.qmail@icarus.apache.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
In-Reply-To: <20011011044014.43733.qmail@icarus.apache.org>;
 from rbb@apache.org on Thu, Oct 11, 2001 at 04:40:14AM -0000

On Thu, Oct 11, 2001 at 04:40:14AM -0000, rbb@apache.org wrote:
> rbb         01/10/10 21:40:14
> 
>   Modified:    modules/http http_protocol.c
>   Log:
>   Remove some warnings from the code.  The buckets take a different
>   type for the length than the brigade functions do.  This moves the
>   len_read variable into the correct scope for the two times that it
>   is used, and defines it correctly for each scope.
>   
>   Revision  Changes    Path
>   1.372     +3 -1      httpd-2.0/modules/http/http_protocol.c
>   
>   Index: http_protocol.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
>   retrieving revision 1.371
>   retrieving revision 1.372
>   diff -u -r1.371 -r1.372
>   --- http_protocol.c	2001/10/11 01:40:32	1.371
>   +++ http_protocol.c	2001/10/11 04:40:14	1.372
>   @@ -1355,7 +1355,7 @@
>     */
>    AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, apr_size_t bufsiz)
>    {
>   -    apr_size_t len_read, total;
>   +    apr_size_t total;
>        apr_status_t rv;
>        apr_bucket *b, *old;
>        const char *tempbuf;
>   @@ -1365,6 +1365,7 @@
>        apr_bucket_brigade *bb = req_cfg->bb;
>    
>        do {
>   +        apr_off_t len_read;

You don't want to do that, you are growing and shrinking the stack for
each iteration through the loop.

Was there anything unacceptable from the patch I posted ~1 hour ago? I
also fixed the loop to not call APR_BRIGADE_EMPTY more than we have to.

  do { if (a) { block; } } while (a);
is the same as:
  while (a) { block; }
with half as many evaluations of the (a) condition.

>            if (APR_BRIGADE_EMPTY(bb)) {
>                len_read = r->remaining;
>                if (ap_get_brigade(r->input_filters, bb, AP_MODE_BLOCKING,
>   @@ -1399,6 +1400,7 @@
>        while (total < bufsiz
>               && b != APR_BRIGADE_SENTINEL(bb)
>               && !APR_BUCKET_IS_EOS(b)) {
>   +        apr_size_t len_read;

Same thing about growing and shrinking the stack here...

>    
>            if ((rv = apr_bucket_read(b, &tempbuf, &len_read, APR_BLOCK_READ)) != APR_SUCCESS) {
>                return -1;
>   
>   
>