httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paul J. Reder" <rede...@remulak.net>
Subject Re: [PATCH] Re: chunking of content in mod_include?
Date Tue, 28 Aug 2001 19:01:41 GMT
In addition to Ryan's comments I have the following comments...

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
> 
> Index: mod_include.c
> ===================================================================
> RCS file: /home/cvspublic/httpd-2.0/modules/filters/mod_include.c,v
> retrieving revision 1.136
> diff -u -r1.136 mod_include.c
> --- mod_include.c    2001/08/27 20:25:42    1.136
> +++ mod_include.c    2001/08/28 06:16:38
> @@ -143,9 +143,10 @@
>   * first byte of the BEGINNING_SEQUENCE (after finding a complete
> match) or it
>   * returns NULL if no match found.
>   */
> -static apr_bucket *find_start_sequence(apr_bucket *dptr, include_ctx_t
> *ctx,
> -                                      apr_bucket_brigade *bb, int
> *do_cleanup)
> +static apr_bucket *find_start_sequence(apr_bucket *dptr, ap_filter_t *f,
> +                                      apr_bucket_brigade **bb, int
> *do_cleanup)
>  {
> +    include_ctx_t *ctx = f->ctx;
>      apr_size_t len;
>      const char *c;
>      const char *buf;
> @@ -156,39 +157,43 @@
>      *do_cleanup = 0;
> 
>      do {
> +        apr_status_t rv;
> +
>          if (APR_BUCKET_IS_EOS(dptr)) {
>              break;
>          }
> -        apr_bucket_read(dptr, &buf, &len, APR_BLOCK_READ);
> -        /* XXX handle retcodes */
> -        if (len == 0) { /* end of pipe? */
> -            break;
> +
> +        if ((ctx->bytes_parsed >= BYTE_COUNT_THRESHOLD) &&
> +            !APR_BRIGADE_EMPTY(*bb)) {
> +            apr_bucket_brigade *remainder = apr_brigade_split(*bb, dptr);

This should check ctx->head_start_bucket. If it is non-NULL, use it instead
of dptr in the brigade split or you may be passing part of the tag along.

> +            rv = ap_pass_brigade(f->next, *bb);
> +            *bb = remainder;

You need to reset ctx->bytes_parsed to 0 here or this will trip again.

> +            if (rv != APR_SUCCESS) {
> +                return NULL;
> +            }

You may want to emulate the way the old code worked for threshold processing.
It split the bucket, but you don't *have* to do that. The important part is
what it did with the returned pointer and the context. It allowed the
code in send_parsed_content to do the actual pass_brigade. Thus the code
to split and pass the brigade existed in one common place for the
find_start_sequence and find_end_sequence code. So all you would have to 
do here is reset the context and return a pointer to the head_start_bucket.
You would do the same thing in find_end_sequence. This would limit the
duplicated code to one set of code instead of four.

>          }
> -        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) {
> +            if (!APR_BRIGADE_EMPTY(*bb)) {
> +                apr_bucket_brigade *remainder = apr_brigade_split(*bb,
> dptr);
> +                rv = ap_pass_brigade(f->next, *bb);
> +                *bb = remainder;

Same comments here...

> +                if (rv != APR_SUCCESS) {
> +                    return NULL;
>                  }
> -                else {
> -                    start_index  = (c - buf);
> -                    start_bucket = dptr;
> -                }
> -                apr_bucket_split(start_bucket, start_index);
> -                tmp_bkt = APR_BUCKET_NEXT(start_bucket);
> -                if (ctx->head_start_index > 0) {
> -                    ctx->head_start_index  = 0;
> -                    ctx->head_start_bucket = tmp_bkt;
> -                    ctx->parse_pos = 0;
> -                    ctx->state = PRE_HEAD;
> -                }
> -
> -                return tmp_bkt;
>              }
> +            rv = apr_bucket_read(dptr, &buf, &len, APR_BLOCK_READ);
> +        }
> +        if (rv != APR_SUCCESS) {
> +            return NULL;
> +        }
> 
> +        if (len == 0) { /* end of pipe? */
> +            break;
> +        }
> +        c = buf;
> +        while (c < buf + len) {
>              if (*c == str[ctx->parse_pos]) {
>                  if (ctx->state == PRE_HEAD) {
>                      ctx->state             = PARSE_HEAD;
> @@ -244,7 +249,7 @@
>              ctx->bytes_parsed++;
>          }
>          dptr = APR_BUCKET_NEXT(dptr);
> -    } while (dptr != APR_BRIGADE_SENTINEL(bb));
> +    } while (dptr != APR_BRIGADE_SENTINEL(*bb));
>      return NULL;
>  }
> 
> @@ -2363,7 +2368,7 @@
>              int do_cleanup = 0;
>              apr_size_t cleanup_bytes = ctx->parse_pos;
> 
> -            tmp_dptr = find_start_sequence(dptr, ctx, *bb, &do_cleanup);
> +            tmp_dptr = find_start_sequence(dptr, f, bb, &do_cleanup);
> 
>              /* The few bytes stored in the ssi_tag_brigade turned out
> not to
>               * be a tag after all. This can only happen if the starting

Mime
View raw message