[PATCH] fix unnecessary logic, fix warning, add comment

[Aaron Bannert <aa...@clove.org>]

You'll probably want to commit the following patch as well:

- converts do-while+if to just a while, so we don't call
    AP_BRIGADE_EMPTY twice per loop.
- fixes a warning (the *readbytes parameter is an apr_off_t for some reason)
- adds a little comment to the mystery loop (is my comment correct?)

-aaron


Index: modules/http/http_protocol.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.371
diff -u -r1.371 http_protocol.c
--- modules/http/http_protocol.c	2001/10/11 01:40:32	1.371
+++ modules/http/http_protocol.c	2001/10/11 02:27:15
@@ -1356,6 +1356,7 @@
 AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, apr_size_t bufsiz)
 {
     apr_size_t len_read, total;
+    apr_off_t len_bypassed;
     apr_status_t rv;
     apr_bucket *b, *old;
     const char *tempbuf;
@@ -1364,21 +1365,20 @@
                                                     &core_module);
     apr_bucket_brigade *bb = req_cfg->bb;
 
-    do {
-        if (APR_BRIGADE_EMPTY(bb)) {
-            len_read = r->remaining;
-            if (ap_get_brigade(r->input_filters, bb, AP_MODE_BLOCKING,
-                               &len_read) != APR_SUCCESS) {
-                /* if we actually fail here, we want to just return and
-                 * stop trying to read data from the client.
-                 */
-                r->connection->keepalive = -1;
-                apr_brigade_destroy(bb);
-                return -1;
-            }
-            r->remaining -= len_read;
+    /* Call down the input stack until we get a non-empty bucket. */
+    while (APR_BRIGADE_EMPTY(bb)) {
+        len_bypassed = r->remaining;
+        if (ap_get_brigade(r->input_filters, bb, AP_MODE_BLOCKING,
+                           &len_bypassed) != APR_SUCCESS) {
+            /* if we actually fail here, we want to just return and
+             * stop trying to read data from the client.
+             */
+            r->connection->keepalive = -1;
+            apr_brigade_destroy(bb);
+            return -1;
         }
-    } while (APR_BRIGADE_EMPTY(bb));
+        r->remaining -= len_bypassed;
+    }
 
     b = APR_BRIGADE_FIRST(bb);
     if (APR_BUCKET_IS_EOS(b)) { /* reached eos on previous invocation */

Re: [PATCH] fix unnecessary logic, fix warning, add comment

[Justin Erenkrantz <je...@ebuilt.com>]

On Wed, Oct 10, 2001 at 07:36:17PM -0700, Aaron Bannert wrote:
> You'll probably want to commit the following patch as well:
> 
> - converts do-while+if to just a while, so we don't call
>     AP_BRIGADE_EMPTY twice per loop.

Yup.  General goodness.

> - fixes a warning (the *readbytes parameter is an apr_off_t for some reason)

Once things settle down, I'd like to kill apr_off_t in the filters
and always use apr_size_t.  It'd make things so much more consistent.

If anyone wants to enlighten me on the origins of apr_off_t in
the filter code, I'd like to know.

> - adds a little comment to the mystery loop (is my comment correct?)

Looks right.  Just remember that ap_get_client_block should be
called in a loop as well.  =)  Loops in loops in loops.  Oh, what
fun.  

So, +1.  I'll let Ryan sort this out since he committed a 
conflicting patch.  -- justin