httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
Subject Re: svn commit: r718082 - /httpd/httpd/trunk/modules/filters/mod_buffer.c
Date Sun, 16 Nov 2008 21:33:30 GMT


On 11/16/2008 08:22 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Sun Nov 16 11:22:41 2008
> New Revision: 718082
> 
> URL: http://svn.apache.org/viewvc?rev=718082&view=rev
> Log:
> Refactor the input buffer filter to use a context as the output buffer
> filter does. Ensure that the filter properly handles the non blocking
> case, both when a downstream filter returns EAGAIN, or if a filter
> returns APR_SUCCESS and an empty brigade.
> 
> Modified:
>     httpd/httpd/trunk/modules/filters/mod_buffer.c
> 
> Modified: httpd/httpd/trunk/modules/filters/mod_buffer.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_buffer.c?rev=718082&r1=718081&r2=718082&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/filters/mod_buffer.c (original)
> +++ httpd/httpd/trunk/modules/filters/mod_buffer.c Sun Nov 16 11:22:41 2008

> @@ -170,65 +171,84 @@
>          return ap_get_brigade(f->next, bb, mode, block, readbytes);
>      }
>  
> +    /* first time in? create a context */
> +    if (!ctx) {
> +        ctx = f->ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
> +        ctx->bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
> +        ctx->tmp = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
> +        ctx->conf = ap_get_module_config(f->r->per_dir_config, &buffer_module);
> +    }
> +
>      /* just get out of the way of things we don't want. */
>      if (mode != AP_MODE_READBYTES) {
>          return ap_get_brigade(f->next, bb, mode, block, readbytes);
>      }
>  
> -    c = ap_get_module_config(f->r->per_dir_config, &buffer_module);
> -
> -    tmp = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
> -
> -    remaining = readbytes;
> -    while (remaining > 0) {
> -        const char *data;
> -        apr_off_t len;
> -        apr_size_t size;
> -
> -        rv = ap_get_brigade(f->next, tmp, mode, block, remaining);
> -
> -        /* if an error was received, bail out now */
> -        if (rv != APR_SUCCESS) {
> -            APR_BRIGADE_CONCAT(bb, tmp);
> -            return rv;
> -        }
> -
> -        apr_brigade_length(tmp, 1, &len);
> -        remaining -= len;
> -
> -        for (e = APR_BRIGADE_FIRST(tmp); e != APR_BRIGADE_SENTINEL(tmp); e
> -                = APR_BUCKET_NEXT(e)) {
> -
> -            /* if we see an EOS, we are done */
> -            if (APR_BUCKET_IS_EOS(e)) {
> -                APR_BUCKET_REMOVE(e);
> -                APR_BRIGADE_INSERT_TAIL(bb, e);
> -                remaining = 0;
> -                break;
> +    /* if our buffer is empty, read off the network until the buffer is full */
> +    if (APR_BRIGADE_EMPTY(ctx->bb)) {
> +        ctx->remaining = ctx->conf->size;
> +
> +        while (!ctx->seen_eos && ctx->remaining > 0) {
> +            const char *data;
> +            apr_size_t size = 0;
> +
> +            rv = ap_get_brigade(f->next, ctx->tmp, mode, block, ctx->remaining);
> +
> +            /* if an error was received, bail out now. If the error is
> +             * EAGAIN and we have not yet seen an EOS, we will definitely
> +             * be called again, at which point we will send our buffered
> +             * data. Instead of sending EAGAIN, some filters return an
> +             * empty brigade instead when data is not yet available. In
> +             * this case, pass through the APR_SUCCESS and emulate the
> +             * underlying filter.
> +             */
> +            if (rv != APR_SUCCESS || APR_BRIGADE_EMPTY(ctx->tmp)) {
> +                return rv;
>              }
>  
> -            /* pass flush buckets through */
> -            if (APR_BUCKET_IS_FLUSH(e)) {
> -                APR_BUCKET_REMOVE(e);
> -                APR_BRIGADE_INSERT_TAIL(bb, e);
> -                continue;
> -            }
> +            for (e = APR_BRIGADE_FIRST(ctx->tmp); e != APR_BRIGADE_SENTINEL(
> +                    ctx->tmp); e = APR_BUCKET_NEXT(e)) {
>  
> -            /* pass metadata buckets through */
> -            if (APR_BUCKET_IS_METADATA(e)) {
> -                APR_BUCKET_REMOVE(e);
> -                APR_BRIGADE_INSERT_TAIL(bb, e);
> -                continue;
> -            }
> +                /* if we see an EOS, we are done */
> +                if (APR_BUCKET_IS_EOS(e)) {
> +                    APR_BUCKET_REMOVE(e);
> +                    APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
> +                    ctx->seen_eos = 1;
> +                    break;
> +                }
> +
> +                /* pass flush and metadata buckets through */
> +                if (APR_BUCKET_IS_FLUSH(e) || APR_BUCKET_IS_METADATA(e)) {

This is redundant as flush buckets are metadata buckets :-).

> +                    APR_BUCKET_REMOVE(e);
> +                    APR_BRIGADE_INSERT_TAIL(bb, e);
> +                    continue;
> +                }
> +
> +                /* read the bucket in, pack it into the buffer */
> +                if (APR_SUCCESS == (rv = apr_bucket_read(e, &data, &size,
> +                        APR_BLOCK_READ))) {
> +                    apr_brigade_write(ctx->bb, NULL, NULL, data, size);
> +                    ctx->remaining -= size;
> +                    apr_bucket_delete(e);
> +                } else {
> +                    return rv;
> +                }
>  
> -            /* read */
> -            if (APR_SUCCESS == (rv = apr_bucket_read(e, &data, &size,
> -                    APR_BLOCK_READ))) {
> -                apr_brigade_write(bb, NULL, NULL, data, size);
>              }
> +        }
> +    }
>  
> +    /* give the caller the data they asked for from the buffer */
> +    apr_brigade_partition(ctx->bb, readbytes, &after);
> +    e = APR_BRIGADE_FIRST(ctx->bb);
> +    while (e != after) {
> +        if (APR_BUCKET_IS_EOS(e)) {
> +            /* last bucket read, step out of the way */
> +            ap_remove_input_filter(f);
>          }
> -        apr_brigade_cleanup(tmp);
> +        APR_BUCKET_REMOVE(e);
> +        APR_BRIGADE_INSERT_TAIL(bb, e);
> +        e = APR_BRIGADE_FIRST(ctx->bb);
>      }

This should be possible without a loop over the brigade. See
brigade_move in core_filters for a similar function.

Regards

RĂ¼diger


Mime
View raw message