Re: [Patch] Async write completion for the full connection filter stack

[Yann Ylavic]

On Thu, Sep 11, 2014 at 1:51 AM, Yann Ylavic <ylavic....@gmail.com> wrote:
> On Wed, Sep 10, 2014 at 6:07 PM, Graham Leggett <minf...@sharp.fm> wrote:
>
> +AP_DECLARE(int) ap_filter_reinstate_brigade(ap_filter_t *f,
> +                                            apr_bucket_brigade *buffered_bb,
> +                                            apr_bucket_brigade *bb,
> +                                            apr_bucket **flush_upto)
> +{
> +    apr_bucket *bucket, *next;
> +    apr_size_t bytes_in_brigade, non_file_bytes_in_brigade;
> +    int eor_buckets_in_brigade, morphing_bucket_in_brigade;
> +    int loglevel = ap_get_conn_module_loglevel(f->c, APLOG_MODULE_INDEX);
> +
> +    if ((buffered_bb != NULL) && !APR_BRIGADE_EMPTY(buffered_bb)) {
> +        APR_BRIGADE_PREPEND(bb, buffered_bb);
> +        f->c->data_in_output_filters--;
>
> Maybe we could use instead :
>
>          if (!APR_BRIGADE_EMPTY(bb)) {
>             f->c->data_in_output_filters--;
>          }
>          APR_BRIGADE_PREPEND(bb, buffered_bb);
>
> so that it remains symetric with ap_filter_setaside_brigade() above
> (which increments c->data_in_output_filters).
>
> It also has the advantage to simplify the calling code in
> ap_core_output_filter(), where we could simply do :
>
>     if ((ctx->buffered_bb != NULL) && !APR_BRIGADE_EMPTY(ctx->buffered_bb)) {
>         should_write = ap_filter_reinstate_brigade(f, ctx->buffered_bb, bb,
>                 &flush_upto);
>     }
>     else if (APR_BRIGADE_EMPTY(bb)) {
>         return APR_SUCCESS;
>     }

Or even simpler :

    should_write = ap_filter_reinstate_brigade(f, ctx->buffered_bb, bb,
                             &flush_upto);
    if (APR_BRIGADE_EMPTY(bb)) {
        return APR_SUCCESS;
    }

>
>     if (flush_upto != NULL) {
>         ...
>     }
>
>     if (should_write) {
>         ...
>     }
>
> removing the "empty" variable danse and the duplicated
> send_brigade_nonblocking() code.
>
> +    }