On Sat, Apr 12, 2008 at 10:27:40AM -0700, Chris Elving wrote: > sed is an inherently line-oriented editor. It seems wrong to buffer > multiple lines within libsed. Consider, for example, how adding such > multi-line buffering to libsed would complicate implementing an > interactive sed like sed(1). > > Instead, it seems like such buffering should occur in mod_sed. mod_sed > is what couples libsed to the bucket brigade, and mod_sed knows that > such such buffering is appropriate.
As suggested by Chris Elving, I moved the buffering code from sed code to mod_sed filter code. Diff from first version is attached. New sources can be viewed at : http://src.opensolaris.org/source/xref/webstack/mod_sed/ (as well as by mercurial : hg clone ssh://[EMAIL PROTECTED]/hg/webstack/mod_sed ) Regards, Basant. Index: mod_sed.c =================================================================== --- mod_sed.c 4 Apr 2008 02:33:57 -0000 1.20.2.6 +++ mod_sed.c 17 Apr 2008 21:33:01 -0000 1.20.2.8 @@ -26,6 +26,7 @@ #include "libsed.h" static const char *sed_filter_name = "Sed"; +#define MODSED_OUTBUF_SIZE 4000 typedef struct sed_expr_config { @@ -45,6 +46,9 @@ sed_eval_t eval; request_rec *r; apr_bucket_brigade *bb; + char *outbuf; + char *curoutbuf; + int bufsize; } sed_filter_ctxt; module AP_MODULE_DECLARE_DATA sed_module; @@ -67,10 +71,32 @@ sed_cfg->last_error = error; } +/* + * flush_output_buffer + * Flush the output data (stored in ctx->outbuf) + */ +static void flush_output_buffer(sed_filter_ctxt *ctx, char* buf, int sz) +{ + int size = ctx->curoutbuf - ctx->outbuf; + char *out; + if (size + sz <= 0) + return; + out = apr_palloc(ctx->r->pool, size + sz); + if (size) { + memcpy(out, ctx->outbuf, size); + } + if (buf && (sz > 0)) { + memcpy(out + size, buf, sz); + } + /* Reset the output buffer position */ + ctx->curoutbuf = ctx->outbuf; + apr_bucket *b = apr_bucket_pool_create(out, size + sz, ctx->r->pool, + ctx->r->connection->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(ctx->bb, b); +} + /* This is a call back function. When libsed wants to generate the output, - * this function will be invoked. Caller creates a copy of the buffer from the - * request pool so we don't need to create a copy here. In this method, we - * simply create a bucket and append into the brigade at the tail. + * this function will be invoked. */ static void sed_write_output(void *dummy, char *buf, int sz) { @@ -78,9 +104,14 @@ * of sed_eval_buffer */ sed_filter_ctxt *ctx = (sed_filter_ctxt *) dummy; - apr_bucket *b = apr_bucket_pool_create(buf, sz, ctx->r->pool, - ctx->r->connection->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(ctx->bb, b); + if (((ctx->curoutbuf - ctx->outbuf) + sz) >= ctx->bufsize) { + /* flush current buffer */ + flush_output_buffer(ctx, buf, sz); + } + else { + memcpy(ctx->curoutbuf, buf, sz); + ctx->curoutbuf += sz; + } } /* Compile a sed expression. Compiled context is saved in sed_cfg->sed_cmds. @@ -119,6 +150,34 @@ return APR_SUCCESS; } +/* Initialize sed filter context. If successful then context is set in f->ctx + */ +static apr_status_t init_context(ap_filter_t *f, sed_expr_config *sed_cfg) +{ + apr_status_t status; + sed_filter_ctxt* ctx; + request_rec *r = f->r; + /* Create the context. Call sed_init_eval. libsed will generated + * output by calling sed_write_output and generates any error by + * invoking log_sed_errf. + */ + ctx = apr_pcalloc(r->pool, sizeof(sed_filter_ctxt)); + ctx->r = r; + ctx->bb = NULL; + status = sed_init_eval(&ctx->eval, sed_cfg->sed_cmds, log_sed_errf, + r, &sed_write_output, r->pool); + if (status != APR_SUCCESS) { + return status; + } + apr_pool_cleanup_register(r->pool, &ctx->eval, sed_eval_cleanup, + apr_pool_cleanup_null); + ctx->bufsize = MODSED_OUTBUF_SIZE; + ctx->outbuf = apr_palloc(r->pool, ctx->bufsize + 1); + ctx->curoutbuf = ctx->outbuf; + f->ctx = ctx; + return APR_SUCCESS; +} + /* Entry function for Sed output filter */ static apr_status_t sed_response_filter(ap_filter_t *f, apr_bucket_brigade *bb) @@ -144,21 +203,10 @@ return ap_pass_brigade(f->next, bb); } - /* Create the context. Call sed_init_eval. libsed will generated - * output by calling sed_write_output and generates any error by - * invoking log_sed_errf. - */ - ctx = f->ctx = apr_pcalloc(f->r->pool, sizeof(sed_filter_ctxt)); - ctx->r = f->r; - ctx->bb = NULL; - - status = sed_init_eval(&ctx->eval, sed_cfg->sed_cmds, log_sed_errf, - f->r, &sed_write_output, f->r->pool); - if (status != APR_SUCCESS) { - return status; - } - apr_pool_cleanup_register(f->r->pool, &ctx->eval, sed_eval_cleanup, - apr_pool_cleanup_null); + status = init_context(f, sed_cfg); + if (status != APR_SUCCESS) + return status; + ctx = f->ctx; } ctx->bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc); @@ -190,6 +238,7 @@ apr_bucket *b1 = APR_BUCKET_NEXT(b); /* Now clean up the internal sed buffer */ sed_finalize_eval(&ctx->eval, ctx); + flush_output_buffer(ctx, NULL, 0); APR_BUCKET_REMOVE(b); /* Insert the eos bucket to ctx->bb brigade */ APR_BRIGADE_INSERT_TAIL(ctx->bb, b); @@ -216,6 +265,7 @@ if (status != APR_SUCCESS) { return status; } + flush_output_buffer(ctx, NULL, 0); APR_BUCKET_REMOVE(b); apr_bucket_delete(b); b = b1; @@ -258,19 +308,10 @@ /* XXX : Should we filter the sub requests too */ return ap_get_brigade(f->next, bb, mode, block, readbytes); } - /* Create the context. Call sed_init_eval. libsed will generate - * output by calling sed_write_output and generates any error by - * invoking log_sed_errf. - */ - ctx = f->ctx = apr_pcalloc(f->r->pool, sizeof(sed_filter_ctxt)); - ctx->r = f->r; - status = sed_init_eval(&ctx->eval, sed_cfg->sed_cmds, log_sed_errf, - f->r, &sed_write_output, f->r->pool); - if (status != APR_SUCCESS) { - return status; - } - apr_pool_cleanup_register(f->r->pool, &ctx->eval, sed_eval_cleanup, - apr_pool_cleanup_null); + status = init_context(f, sed_cfg); + if (status != APR_SUCCESS) + return status; + ctx = f->ctx; ctx->bb = apr_brigade_create(f->r->pool, f->c->bucket_alloc); } @@ -310,6 +351,7 @@ if (APR_BUCKET_IS_EOS(b)) { /* eos bucket. Clear the internal sed buffers */ sed_finalize_eval(&ctx->eval, ctx); + flush_output_buffer(ctx, NULL, 0); APR_BUCKET_REMOVE(b); APR_BRIGADE_INSERT_TAIL(ctx->bb, b); break; @@ -323,6 +365,7 @@ status = sed_eval_buffer(&ctx->eval, buf, bytes, ctx); if (status != APR_SUCCESS) return status; + flush_output_buffer(ctx, NULL, 0); } } apr_brigade_cleanup(bbinp); Index: sed1.c =================================================================== --- sed1.c 4 Apr 2008 21:21:39 -0000 1.11.2.19 +++ sed1.c 17 Apr 2008 21:42:23 -0000 @@ -840,7 +840,7 @@ while ((apr_file_read(fi, buf, &n)) == APR_SUCCESS) { if (n == 0) break; - eval->writefn(eval->fout, apr_pmemdup(eval->pool, buf, n), n); + eval->writefn(eval->fout, buf, n); n = sizeof(buf); } apr_file_close(fi); @@ -855,9 +855,7 @@ */ static void wline(sed_eval_t *eval, char *buf, int sz) { - char *out = apr_palloc(eval->pool, sz + 1); - memcpy(out, buf, sz); - *(out + sz) = '\n'; - eval->writefn(eval->fout, out, sz + 1); + eval->writefn(eval->fout, buf, sz); + eval->writefn(eval->fout, "\n", 1); }
