I didn't see this patch in the commit list but did see it referred
to in the 2.2 STATUS file... I'm reviewing the patch now but two
things did stick out:

-            apr_brigade_cleanup(ctx->ctxbb);
-            APR_BUCKET_REMOVE(b);
-            APR_BRIGADE_INSERT_TAIL(passbb, b);
-            break;
-        }
-        else if (APR_BUCKET_IS_FLUSH(b)) {
+            apr_brigade_cleanup(ctx->linebb);
             APR_BUCKET_REMOVE(b);
-            APR_BRIGADE_INSERT_TAIL(passbb, b);
-            rv = ap_pass_brigade(f->next, passbb);
-            apr_brigade_cleanup(passbb);
-            if (rv != APR_SUCCESS)
-                return rv;
+            APR_BRIGADE_INSERT_TAIL(ctx->passbb, b);
         }
+        /*
+         * No need to handle FLUSH buckets separately as we call
+         * ap_pass_brigade anyway at the end of the loop.
+         */
         else if (APR_BUCKET_IS_METADATA(b)) {
             APR_BUCKET_REMOVE(b);
-            APR_BRIGADE_INSERT_TAIL(passbb, b);
+            APR_BRIGADE_INSERT_TAIL(ctx->passbb, b);

The reason why the current code handles FLUSH separately
is though, yes, the ap_pass_brigade is done at the end of
the while loop, that is *only* done when we're done handling
the full brigade... The intent was to honor flushes in the
brigade "immediately" and "reset" at that point... Not sure
why this was removed, since the old way is more efficient.

I also see that AP_MIN_BYTES_TO_WRITE is no longer being
used at all... I'm guessing MAX_BUCKETS is designed to
"replace" that?? Again, the idea is to avoid extremely
large in-process data sizes :) The MAX_BUCKETS seems
to be mostly for super-bad cases and not so much for
behaving "nicely" in all cases. (mod_include.c does
the same thing, btw)

Reply via email to