httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ryan Bloom <...@covalent.net>
Subject Re: [PATCH] Re: chunking of content in mod_include?
Date Tue, 28 Aug 2001 13:57:32 GMT

Good patch, I have just a few fixes that need to be made to it.
Most of these are incredibly non-obvious or intuitive.

On Monday 27 August 2001 23:27, Brian Pane wrote:

> How does this patch look?  It does the check only on bucket
> boundaries, and it pushes out the buffered content if either:
>   * the amount of buffered content is too large, or
>   * the apr_bucket_read would block.
>
> If this technique looks sound, I can create another patch that
> does the same thing for find_end_sequence.
>
> --Brian
>
>
> -        c = buf;
> -        while (c < buf + len) {
> -            if (ctx->bytes_parsed >= BYTE_COUNT_THRESHOLD) {
> -                apr_bucket *start_bucket;
>
> -                if (ctx->head_start_index > 0) {
> -                    start_index  = ctx->head_start_index;
> -                    start_bucket = ctx->head_start_bucket;
> +        rv = apr_bucket_read(dptr, &buf, &len, APR_NONBLOCK_READ);
> +        if (rv == APR_EAGAIN) {

This needs to be if (APR_STATUS_IS_EAGAIN(rv)) {

EAGAIN is one of those annoying error conditions, that is completely non-portable,
so we have to use a macro to check for it.

> +                rv = ap_pass_brigade(f->next, *bb);
> +                *bb = remainder;
> +                if (rv != APR_SUCCESS) {
> +                    return NULL;

This looks like a problem.  Basically, a future filter has just informed us that there
was a problem, but we are going to silently ignore it.  If we get a status other than
APR_SUCCESS from a future filter, then we _must_ return that error code to the
filter that called us.  If we don't, then we will continue to send data, even though
lower filters think that everything is done, and everything gets confused.  I have
closed this bug in mod_include multiple times, so I am VERY aware of it.  :-)

> +            rv = apr_bucket_read(dptr, &buf, &len, APR_BLOCK_READ);
> +        }
> +        if (rv != APR_SUCCESS) {
> +            return NULL;
> +        }

Same as above.  You can't lose that error code.

Fix those three things, and I'll give it a +1.

Ryan

______________________________________________________________
Ryan Bloom				rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Mime
View raw message