You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by rb...@covalent.net on 2001/02/06 06:42:11 UTC
[PATCH] final brigade buffering
This is the final brigade buffering patch. What it basically does. We
use a simple heap bucket to buffer the data. Whenever we fill a buffer,
we automatically flush the entire brigade, using a flush function from the
brigade itself. We don't copy unless we can copy the entire string into
the brigade with a single copy.
The only remaining complaint that I know of with this patch, is that it
forces handlers to use r->bb as their brigade if they want to make sure
that the data is never sent out of order. This also removes the OLD_WRITE
buffer, and it begins to remove some of the other buffering logic we have
added.
This also allows modules like mod_perl to remove their special buffering
logic. I would really like to get this committed. I will not even try to
say that this code is bug free, but it has served a lot of pages very
cleanly with this patch now.
Since we are no longer freezing the tree, I would like to get this in
soon.
Ryan
Index: include/http_protocol.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_protocol.h,v
retrieving revision 1.51
diff -u -d -b -w -u -r1.51 http_protocol.h
--- include/http_protocol.h 2001/02/05 03:31:43 1.51
+++ include/http_protocol.h 2001/02/06 05:19:31
@@ -88,13 +88,13 @@
/**
* Send the minimal part of an HTTP response header.
* @param r The current request
- * @param buf The buffer to add the header to.
+ * @param bb The brigade to add the header to.
* @warning Modules should be very careful about using this, and should
* prefer ap_send_http_header(). Much of the HTTP/1.1 implementation
* correctness depends on code in ap_send_http_header().
- * @deffunc void ap_basic_http_header(request_rec *r, char *buf)
+ * @deffunc void ap_basic_http_header(request_rec *r, apr_bucket_brigade *bb)
*/
-AP_DECLARE(void) ap_basic_http_header(request_rec *r, char *buf);
+AP_DECLARE(void) ap_basic_http_header(request_rec *r, apr_bucket_brigade *bb);
/**
* Send the Status-Line and header fields for HTTP response
@@ -310,7 +310,11 @@
* @return The number of bytes sent
* @deffunc int ap_rputc(int c, request_rec *r)
*/
-AP_DECLARE(int) ap_rputc(int c, request_rec *r);
+#define ap_rputc(c, r) \
+ if (!r->bb) { \
+ r->bb = apr_brigade_create(r->pool); \
+ } \
+ ap_fputc(r->output_filters, r->bb, c)
/**
* Output a string for the current request
* @param str The string to output
@@ -318,7 +322,11 @@
* @return The number of bytes sent
* @deffunc int ap_rputs(const char *str, request_rec *r)
*/
-AP_DECLARE(int) ap_rputs(const char *str, request_rec *r);
+#define ap_rputs(str, r) \
+ if (!r->bb) { \
+ r->bb = apr_brigade_create(r->pool); \
+ } \
+ ap_fputs(r->output_filters, r->bb, str)
/**
* Write a buffer for the current request
* @param buf The buffer to write
@@ -327,7 +335,11 @@
* @return The number of bytes sent
* @deffunc int ap_rwrite(const void *buf, int nbyte, request_rec *r)
*/
-AP_DECLARE(int) ap_rwrite(const void *buf, int nbyte, request_rec *r);
+#define ap_rwrite(str, n, r) \
+ if (!r->bb) { \
+ r->bb = apr_brigade_create(r->pool); \
+ } \
+ ap_fwrite(r->output_filters, r->bb, str, n)
/**
* Write an unspecified number of strings to the request
* @param r The current request
@@ -335,7 +347,11 @@
* @return The number of bytes sent
* @deffunc int ap_rvputs(request_rec *r, ...)
*/
-AP_DECLARE_NONSTD(int) ap_rvputs(request_rec *r,...);
+#define ap_rvputs(r, args...) \
+ if (!r->bb) { \
+ r->bb = apr_brigade_create(r->pool); \
+ } \
+ ap_fvputs(r->output_filters, r->bb, ##args)
/**
* Output data to the client in a printf format
* @param r The current request
@@ -344,7 +360,11 @@
* @return The number of bytes sent
* @deffunc int ap_vrprintf(request_rec *r, const char *fmt, va_list vlist)
*/
-AP_DECLARE(int) ap_vrprintf(request_rec *r, const char *fmt, va_list vlist);
+#define ap_vrprintf(r, fmt, vlist) \
+ if (!r->bb) { \
+ r->bb = apr_brigade_create(r->pool); \
+ } \
+ ap_vfprintf(r->output_filters, r->bb, fmt, vlist)
/**
* Output data to the client in a printf format
* @param r The current request
@@ -353,15 +373,22 @@
* @return The number of bytes sent
* @deffunc int ap_rprintf(request_rec *r, const char *fmt, ...)
*/
-AP_DECLARE_NONSTD(int) ap_rprintf(request_rec *r, const char *fmt,...)
- __attribute__((format(printf,2,3)));
+#define ap_rprintf(r, fmt, args...) \
+ if (!r->bb) { \
+ r->bb = apr_brigade_create(r->pool); \
+ } \
+ ap_fprintf(r->output_filters, r->bb, fmt, ##args)
/**
* Flush all of the data for the current request to the client
* @param r The current request
* @return The number of bytes sent
* @deffunc int ap_rflush(request_rec *r)
*/
-AP_DECLARE(int) ap_rflush(request_rec *r);
+#define ap_rflush(r) \
+ if (!r->bb) { \
+ r->bb = apr_brigade_create(r->pool); \
+ } \
+ ap_fflush(r->output_filters, r->bb)
/**
* Index used in custom_responses array for a specific error code
Index: include/httpd.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/httpd.h,v
retrieving revision 1.134
diff -u -d -b -w -u -r1.134 httpd.h
--- include/httpd.h 2001/02/05 12:55:11 1.134
+++ include/httpd.h 2001/02/06 05:19:31
@@ -86,6 +86,7 @@
#include "apr_pools.h"
#include "apr_time.h"
#include "apr_network_io.h"
+#include "apr_buckets.h"
#ifdef CORE_PRIVATE
@@ -798,6 +799,8 @@
/** A flag to determine if the eos bucket has been sent yet
* @defvar int eos_sent */
int eos_sent;
+
+ apr_bucket_brigade *bb;
/* Things placed at the end of the record to avoid breaking binary
* compatibility. It would be nice to remember to reorder the entire
Index: include/util_filter.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/util_filter.h,v
retrieving revision 1.38
diff -u -d -b -w -u -r1.38 util_filter.h
--- include/util_filter.h 2001/02/03 20:25:13 1.38
+++ include/util_filter.h 2001/02/06 05:19:31
@@ -393,6 +393,16 @@
AP_DECLARE(apr_status_t) ap_save_brigade(ap_filter_t *f, apr_bucket_brigade **save_to,
apr_bucket_brigade **b);
+AP_DECLARE(apr_status_t) ap_fwrite(ap_filter_t *f, apr_bucket_brigade *bb,
+ const char *str, apr_size_t nbyte);
+AP_DECLARE(apr_status_t) ap_fputs(ap_filter_t *f, apr_bucket_brigade *bb,
+ const char *str);
+AP_DECLARE(apr_status_t) ap_fputc(ap_filter_t *f, apr_bucket_brigade *bb,
+ char str);
+AP_DECLARE(apr_status_t) ap_fvputs(ap_filter_t *f, apr_bucket_brigade *bb, ...);
+AP_DECLARE(apr_status_t) ap_fprintf(ap_filter_t *f, apr_bucket_brigade *bb, const char *fmt, ...);
+
+
#ifdef __cplusplus
}
#endif
Index: modules/http/http_core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_core.c,v
retrieving revision 1.254
diff -u -d -b -w -u -r1.254 http_core.c
--- modules/http/http_core.c 2001/02/05 18:28:47 1.254
+++ modules/http/http_core.c 2001/02/06 05:19:35
@@ -3593,8 +3593,6 @@
AP_FTYPE_CONTENT);
ap_register_output_filter("CHUNK", chunk_filter, AP_FTYPE_TRANSCODE);
ap_register_output_filter("COALESCE", coalesce_filter, AP_FTYPE_CONTENT);
- ap_register_output_filter("OLD_WRITE", ap_old_write_filter,
- AP_FTYPE_CONTENT - 1);
}
AP_DECLARE_DATA module core_module = {
Index: modules/http/http_protocol.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.289
diff -u -d -b -w -u -r1.289 http_protocol.c
--- modules/http/http_protocol.c 2001/02/01 21:54:18 1.289
+++ modules/http/http_protocol.c 2001/02/06 05:19:38
@@ -1587,13 +1587,15 @@
static void end_output_stream(request_rec *r)
{
- apr_bucket_brigade *bb;
apr_bucket *b;
- bb = apr_brigade_create(r->pool);
+ if (!r->bb) {
+ r->bb = apr_brigade_create(r->pool);
+ }
+
b = apr_bucket_create_eos();
- APR_BRIGADE_INSERT_TAIL(bb, b);
- ap_pass_brigade(r->output_filters, bb);
+ APR_BRIGADE_INSERT_TAIL(r->bb, b);
+ ap_pass_brigade(r->output_filters, r->bb);
}
void ap_finalize_sub_req_protocol(request_rec *sub)
@@ -1792,7 +1794,7 @@
typedef struct header_struct {
request_rec *r;
- char *buf;
+ apr_bucket_brigade *bb;
} header_struct;
/* Send a single HTTP header field to the client. Note that this function
@@ -1807,16 +1809,7 @@
headfield = apr_pstrcat(h->r->pool, fieldname, ": ", fieldval, CRLF, NULL);
ap_xlate_proto_to_ascii(headfield, strlen(headfield));
- apr_cpystrn(h->buf, headfield, strlen(headfield) + 1);
- h->buf += strlen(headfield);
- return 1;
-}
-
-static int compute_header_len(apr_size_t *length, const char *fieldname,
- const char *fieldval)
-{
- /* The extra five are for ": " and CRLF, plus one for a '\0'. */
- *length = *length + strlen(fieldname) + strlen(fieldval) + 6;
+ apr_brigade_write(h->bb, headfield, strlen(headfield) + 1);
return 1;
}
@@ -1844,7 +1837,7 @@
}
}
-static void basic_http_header(request_rec *r, char *buf, const char *protocol)
+static void basic_http_header(request_rec *r, apr_bucket_brigade *bb, const char *protocol)
{
char *date = NULL;
char *tmp;
@@ -1857,14 +1850,13 @@
tmp = apr_pstrcat(r->pool, protocol, " ", r->status_line, CRLF, NULL);
ap_xlate_proto_to_ascii(tmp, strlen(tmp));
- apr_cpystrn(buf, tmp, strlen(tmp) + 1);
- buf += strlen(tmp);
+ apr_brigade_write(bb, tmp, strlen(tmp) + 1);
date = apr_palloc(r->pool, APR_RFC822_DATE_LEN);
apr_rfc822_date(date, r->request_time);
h.r = r;
- h.buf = buf;
+ h.bb = bb;
form_header_field(&h, "Date", date);
form_header_field(&h, "Server", ap_get_server_version());
@@ -1872,12 +1864,12 @@
apr_table_unset(r->headers_out, "Server");
}
-AP_DECLARE(void) ap_basic_http_header(request_rec *r, char *buf)
+AP_DECLARE(void) ap_basic_http_header(request_rec *r, apr_bucket_brigade *bb)
{
const char *protocol;
basic_http_header_check(r, &protocol);
- basic_http_header(r, buf, protocol);
+ basic_http_header(r, bb, protocol);
}
/* Navigator versions 2.x, 3.x and 4.0 betas up to and including 4.0b2
@@ -1897,18 +1889,19 @@
* It is more expensive to check the User-Agent than it is to just add the
* bytes, so we haven't used the BrowserMatch feature here.
*/
-static void terminate_header(char *buf)
+static void terminate_header(apr_bucket_brigade *bb)
{
- int len = strlen(buf);
- char *headfield = buf + len;
char *tmp = "X-Pad: avoid browser bug" CRLF;
+ char *crlf = CRLF;
+ apr_bucket *b = APR_BRIGADE_LAST(bb);
+ apr_bucket_shared *s = b->data;
- if (len >= 255 && len <= 257) {
- apr_cpystrn(headfield, tmp, strlen(tmp) + 1);
- headfield += strlen(tmp);
+ if (s->end >= 255 && s->end <= 257) {
+ ap_xlate_proto_to_ascii(tmp, strlen(tmp));
+ apr_brigade_write(bb, tmp, strlen(tmp) + 1);
}
- apr_cpystrn(headfield, CRLF, strlen(CRLF) + 1);
- ap_xlate_proto_to_ascii(buf + len, strlen(buf + len));
+ ap_xlate_proto_to_ascii(crlf, strlen(crlf));
+ apr_brigade_write(bb, crlf, strlen(crlf) + 1);
}
/*
@@ -2154,42 +2147,28 @@
int ap_send_http_options(request_rec *r)
{
- char *buff;
- apr_bucket *b;
- apr_bucket_brigade *bb;
- apr_size_t len = 0;
+ apr_bucket_brigade *bb = apr_brigade_create(r->pool);
header_struct h;
if (r->assbackwards)
return DECLINED;
-
- apr_table_do((int (*) (void *, const char *, const char *)) compute_header_len,
- (void *) &len, r->headers_out, NULL);
- /* Need to add a fudge factor so that the CRLF at the end of the headers
- * and the basic http headers don't overflow this buffer.
- */
- len += strlen(ap_get_server_version()) + 100;
- buff = apr_pcalloc(r->pool, len);
- ap_basic_http_header(r, buff);
+ ap_basic_http_header(r, bb);
apr_table_setn(r->headers_out, "Content-Length", "0");
apr_table_setn(r->headers_out, "Allow", make_allow(r));
ap_set_keepalive(r);
h.r = r;
- h.buf = buff;
+ h.bb = bb;
apr_table_do((int (*) (void *, const char *, const char *)) form_header_field,
(void *) &h, r->headers_out, NULL);
- terminate_header(buff);
+ terminate_header(bb);
r->bytes_sent = 0;
- bb = apr_brigade_create(r->pool);
- b = apr_bucket_create_pool(buff, strlen(buff), r->pool);
- APR_BRIGADE_INSERT_TAIL(bb, b);
ap_pass_brigade(r->output_filters, bb);
return OK;
@@ -2462,11 +2441,10 @@
int i;
char *date = NULL;
request_rec *r = f->r;
- char *buff, *buff_start;
const char *clheader;
const char *protocol;
apr_bucket *e;
- apr_bucket_brigade *b2;
+ apr_bucket_brigade *b2 = apr_brigade_create(r->pool);
apr_size_t len = 0;
header_struct h;
header_filter_ctx *ctx = f->ctx;
@@ -2483,7 +2461,7 @@
}
APR_BRIGADE_FOREACH(e, b) {
- if (APR_BRIGADE_FIRST(b)->type == &ap_bucket_type_error) {
+ if (e->type == &ap_bucket_type_error) {
ap_bucket_error *eb = e->data;
ap_die(eb->status, r);
@@ -2580,33 +2558,11 @@
!strcmp(clheader, "0")) {
apr_table_unset(r->headers_out, "Content-Length");
}
-
- if (r->status == HTTP_NOT_MODIFIED) {
- apr_table_do((int (*)(void *, const char *, const char *)) compute_header_len,
- (void *) &len, r->headers_out,
- "Connection",
- "Keep-Alive",
- "ETag",
- "Content-Location",
- "Expires",
- "Cache-Control",
- "Vary",
- "Warning",
- "WWW-Authenticate",
- "Proxy-Authenticate",
- NULL);
- }
- else {
- apr_table_do((int (*) (void *, const char *, const char *)) compute_header_len,
- (void *) &len, r->headers_out, NULL);
- }
- buff_start = buff = apr_pcalloc(r->pool, len);
- basic_http_header(r, buff, protocol);
- buff += strlen(buff);
+ basic_http_header(r, b2, protocol);
h.r = r;
- h.buf = buff;
+ h.bb = b2;
if (r->status == HTTP_NOT_MODIFIED) {
apr_table_do((int (*)(void *, const char *, const char *)) form_header_field,
@@ -2628,13 +2584,10 @@
(void *) &h, r->headers_out, NULL);
}
- terminate_header(buff);
+ terminate_header(b2);
r->sent_bodyct = 1; /* Whatever follows is real body stuff... */
- b2 = apr_brigade_create(r->pool);
- e = apr_bucket_create_pool(buff_start, strlen(buff_start), r->pool);
- APR_BRIGADE_INSERT_HEAD(b2, e);
ap_pass_brigade(f->next, b2);
if (r->header_only) {
@@ -2985,244 +2938,6 @@
return mm->size; /* XXX - change API to report apr_status_t? */
}
#endif /* APR_HAS_MMAP */
-
-typedef struct {
- char *buf;
- char *cur;
- apr_size_t avail;
-} old_write_filter_ctx;
-
-AP_CORE_DECLARE_NONSTD(apr_status_t) ap_old_write_filter(
- ap_filter_t *f, apr_bucket_brigade *bb)
-{
- old_write_filter_ctx *ctx = f->ctx;
-
- AP_DEBUG_ASSERT(ctx);
-
- if (ctx->buf != NULL) {
- apr_size_t nbyte = ctx->cur - ctx->buf;
-
- if (nbyte != 0) {
- /* whatever is coming down the pipe (we don't care), we
- can simply insert our buffered data at the front and
- pass the whole bundle down the chain. */
- apr_bucket *b = apr_bucket_create_heap(ctx->buf, nbyte, 0, NULL);
- APR_BRIGADE_INSERT_HEAD(bb, b);
- ctx->buf = NULL;
- }
- }
-
- return ap_pass_brigade(f->next, bb);
-}
-
-static apr_status_t flush_buffer(request_rec *r, old_write_filter_ctx *ctx,
- const char *extra, apr_size_t extra_len)
-{
- apr_bucket_brigade *bb = apr_brigade_create(r->pool);
- apr_size_t nbyte = ctx->cur - ctx->buf;
- apr_bucket *b = apr_bucket_create_heap(ctx->buf, nbyte, 0, NULL);
-
- APR_BRIGADE_INSERT_TAIL(bb, b);
- ctx->buf = NULL;
-
- /* if there is extra data, then send that, too */
- if (extra != NULL) {
- b = apr_bucket_create_transient(extra, extra_len);
- APR_BRIGADE_INSERT_TAIL(bb, b);
- }
-
- return ap_pass_brigade(r->output_filters, bb);
-}
-
-static apr_status_t buffer_output(request_rec *r,
- const char *str, apr_size_t len)
-{
- ap_filter_t *f;
- old_write_filter_ctx *ctx;
- apr_size_t amt;
- apr_status_t status;
-
- if (len == 0)
- return APR_SUCCESS;
-
- /* ### future optimization: record some flags in the request_rec to
- ### say whether we've added our filter, and whether it is first. */
-
- /* this will typically exit on the first test */
- for (f = r->output_filters; f != NULL; f = f->next)
- if (strcmp("OLD_WRITE", f->frec->name) == 0)
- break;
- if (f == NULL) {
- /* our filter hasn't been added yet */
- ctx = apr_pcalloc(r->pool, sizeof(*ctx));
- ap_add_output_filter("OLD_WRITE", ctx, r, r->connection);
- }
-
- /* if the first filter is not our buffering filter, then we have to
- deliver the content through the normal filter chain */
- if (strcmp("OLD_WRITE", r->output_filters->frec->name) != 0) {
- apr_bucket_brigade *bb = apr_brigade_create(r->pool);
- apr_bucket *b = apr_bucket_create_transient(str, len);
- APR_BRIGADE_INSERT_TAIL(bb, b);
-
- return ap_pass_brigade(r->output_filters, bb);
- }
-
- /* grab the context from our filter */
- ctx = r->output_filters->ctx;
-
- /* if there isn't a buffer in the context yet, put one there. */
- if (ctx->buf == NULL) {
- /* use the heap so it will get free'd after being flushed */
- ctx->avail = AP_MIN_BYTES_TO_WRITE;
- ctx->buf = ctx->cur = malloc(ctx->avail);
- }
-
- /* squeeze the data into the existing buffer */
- if (len <= ctx->avail) {
- memcpy(ctx->cur, str, len);
- ctx->cur += len;
- if ((ctx->avail -= len) == 0)
- return flush_buffer(r, ctx, NULL, 0);
- return APR_SUCCESS;
- }
-
- /* the new content can't fit in the existing buffer */
-
- if (len >= AP_MIN_BYTES_TO_WRITE) {
- /* it is really big. send what we have, and the new stuff. */
- return flush_buffer(r, ctx, str, len);
- }
-
- /* the new data is small. put some into the current buffer, flush it,
- and then drop the remaining into a new buffer. */
- amt = ctx->avail;
- memcpy(ctx->cur, str, amt);
- ctx->cur += amt;
- ctx->avail = 0;
- if ((status = flush_buffer(r, ctx, NULL, 0)) != APR_SUCCESS)
- return status;
-
- ctx->buf = malloc(AP_MIN_BYTES_TO_WRITE);
- memcpy(ctx->buf, str + amt, len - amt);
- ctx->cur = ctx->buf + (len - amt);
- ctx->avail = AP_MIN_BYTES_TO_WRITE - (len - amt);
-
- return APR_SUCCESS;
-}
-
-AP_DECLARE(int) ap_rputc(int c, request_rec *r)
-{
- char c2 = (char)c;
-
- if (r->connection->aborted) {
- return -1;
- }
-
- if (buffer_output(r, &c2, 1) != APR_SUCCESS)
- return -1;
-
- return c;
-}
-
-AP_DECLARE(int) ap_rputs(const char *str, request_rec *r)
-{
- apr_size_t len;
-
- if (r->connection->aborted)
- return -1;
-
- if (buffer_output(r, str, len = strlen(str)) != APR_SUCCESS)
- return -1;
-
- return len;
-}
-
-AP_DECLARE(int) ap_rwrite(const void *buf, int nbyte, request_rec *r)
-{
- if (r->connection->aborted)
- return -1;
-
- if (buffer_output(r, buf, nbyte) != APR_SUCCESS)
- return -1;
-
- return nbyte;
-}
-
-AP_DECLARE(int) ap_vrprintf(request_rec *r, const char *fmt, va_list va)
-{
- char buf[4096];
- apr_size_t written;
-
- if (r->connection->aborted)
- return -1;
-
- /* ### fix this mechanism to allow more than 4K of output */
- written = apr_vsnprintf(buf, sizeof(buf), fmt, va);
- if (buffer_output(r, buf, written) != APR_SUCCESS)
- return -1;
-
- return written;
-}
-
-AP_DECLARE_NONSTD(int) ap_rprintf(request_rec *r, const char *fmt, ...)
-{
- va_list va;
- int n;
-
- if (r->connection->aborted)
- return -1;
-
- va_start(va, fmt);
- n = ap_vrprintf(r, fmt, va);
- va_end(va);
-
- return n;
-}
-
-AP_DECLARE_NONSTD(int) ap_rvputs(request_rec *r, ...)
-{
- va_list va;
- const char *s;
- apr_size_t len;
- apr_size_t written = 0;
-
- if (r->connection->aborted)
- return -1;
-
- /* ### TODO: if the total output is large, put all the strings
- ### into a single brigade, rather than flushing each time we
- ### fill the buffer */
- va_start(va, r);
- while (1) {
- s = va_arg(va, const char *);
- if (s == NULL)
- break;
-
- len = strlen(s);
- if (buffer_output(r, s, len) != APR_SUCCESS) {
- return -1;
- }
-
- written += len;
- }
- va_end(va);
-
- return written;
-}
-
-AP_DECLARE(int) ap_rflush(request_rec *r)
-{
- apr_bucket_brigade *bb;
- apr_bucket *b;
-
- bb = apr_brigade_create(r->pool);
- b = apr_bucket_create_flush();
- APR_BRIGADE_INSERT_TAIL(bb, b);
- if (ap_pass_brigade(r->output_filters, bb) != APR_SUCCESS)
- return -1;
- return 0;
-}
static const char *add_optional_notes(request_rec *r,
const char *prefix,
Index: server/util_filter.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/util_filter.c,v
retrieving revision 1.43
diff -u -d -b -w -u -r1.43 util_filter.c
--- server/util_filter.c 2001/01/19 07:04:29 1.43
+++ server/util_filter.c 2001/02/06 05:19:44
@@ -265,3 +265,74 @@
APR_BRIGADE_CONCAT(*saveto, *b);
return APR_SUCCESS;
}
+
+static apr_status_t brigade_flush(apr_bucket_brigade *bb, void *ctx)
+{
+ ap_filter_t *f = ctx;
+
+ return ap_pass_brigade(f, bb);
+}
+
+AP_DECLARE(apr_status_t) ap_fwrite(ap_filter_t *f, apr_bucket_brigade *bb,
+ const char *str, apr_size_t nbyte)
+{
+ apr_bucket *b;
+ apr_status_t status;
+
+ if (nbyte == 0)
+ return 0;
+
+ b = APR_BRIGADE_LAST(bb);
+
+ bb->flush = brigade_flush;
+ bb->ctx = f->next;
+ status = apr_brigade_write(bb, str, nbyte);
+
+ /* We are just checking to see if we just converted the
+ * buffer into a new bucket and put it at the end of the brigade. If
+ * we did, we want to pass the brigade to the next filter. If not,
+ * we just keep going. This allows us to use the network to limit how
+ * much data we send at any one time.
+ */
+ if (b != APR_BRIGADE_LAST(bb)) {
+ ap_pass_brigade(f->next, bb);
+ }
+ return status;
+}
+
+AP_DECLARE(apr_status_t) ap_fputs(ap_filter_t *f, apr_bucket_brigade *bb,
+ const char *str)
+{
+ int j = strlen(str);
+
+ return ap_fwrite(f, bb, str, j);
+}
+
+AP_DECLARE(apr_status_t) ap_fputc(ap_filter_t *f, apr_bucket_brigade *bb,
+ char str)
+{
+ return ap_fwrite(f, bb, &str, 1);
+}
+
+AP_DECLARE(apr_status_t) ap_fvputs(ap_filter_t *f, apr_bucket_brigade *bb, ...)
+{
+ apr_size_t written;
+ va_list va;
+
+ va_start(va, bb);
+ written = apr_brigade_vputstrs(bb, va);
+ va_end(va);
+ return written;
+}
+
+AP_DECLARE(apr_status_t) ap_fprintf(ap_filter_t *f, apr_bucket_brigade *bb, const char *fmt, ...)
+{
+
+ apr_size_t written;
+ va_list va;
+
+ va_start(va, fmt);
+ written = apr_brigade_vprintf(bb, fmt, va);
+ va_end(va);
+ return written;
+}
Index: srclib/apr-util/buckets/apr_brigade.c
===================================================================
RCS file: /home/cvs/apr-util/buckets/apr_brigade.c,v
retrieving revision 1.3
diff -u -d -b -w -u -r1.3 apr_brigade.c
--- srclib/apr-util/buckets/apr_brigade.c 2001/01/19 07:01:59 1.3
+++ srclib/apr-util/buckets/apr_brigade.c 2001/02/06 05:20:12
@@ -56,6 +56,7 @@
#include "apr_lib.h"
#include "apr_pools.h"
#include "apr_tables.h"
+#include "apr_buckets.h"
#include "apr_errno.h"
#include <stdlib.h>
@@ -66,8 +67,6 @@
#include <sys/uio.h>
#endif
-#include "apr_buckets.h"
-
static apr_status_t apr_brigade_cleanup(void *data)
{
apr_bucket_brigade *b = data;
@@ -97,8 +96,9 @@
{
apr_bucket_brigade *b;
- b = apr_palloc(p, sizeof(*b));
+ b = apr_pcalloc(p, sizeof(*b));
b->p = p;
+
APR_RING_INIT(&b->list, apr_bucket, link);
apr_register_cleanup(b->p, b, apr_brigade_cleanup, apr_brigade_cleanup);
@@ -185,12 +185,45 @@
return vec - orig;
}
+static int check_brigade_flush(const char **str,
+ apr_size_t *n, apr_bucket_brigade *bb)
+{
+ apr_bucket *b = APR_BRIGADE_LAST(bb);
+
+ if (APR_BRIGADE_EMPTY(bb)) {
+ if (*n > APR_BUCKET_BUFF_SIZE) {
+ apr_bucket *e = apr_bucket_create_transient(*str, *n);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ return 1;
+ }
+ else {
+ return 0;
+ }
+ }
+
+ if (APR_BUCKET_IS_HEAP(b)) {
+ apr_bucket_shared *s = b->data;
+ apr_bucket_heap *h = s->data;
+
+ if (*n > (h->alloc_len - (s->end - s->start))) {
+ apr_bucket *e = apr_bucket_create_transient(*str, *n);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ return 1;
+ }
+ }
+ else if (*n > APR_BUCKET_BUFF_SIZE) {
+ apr_bucket *e = apr_bucket_create_transient(*str, *n);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ return 1;
+ }
+
+ return 0;
+}
+
APU_DECLARE(int) apr_brigade_vputstrs(apr_bucket_brigade *b, va_list va)
{
- apr_bucket *r;
const char *x;
int j, k;
- apr_size_t i;
for (k = 0;;) {
x = va_arg(va, const char *);
@@ -198,20 +231,91 @@
break;
j = strlen(x);
- /* XXX: copy or not? let the caller decide? */
- r = apr_bucket_create_heap(x, j, 1, &i);
- if (i != j) {
- /* Do we need better error reporting? */
- return -1;
+ k += apr_brigade_write(b, x, j);
}
- k += i;
+ return k;
+}
- APR_BRIGADE_INSERT_TAIL(b, r);
+APU_DECLARE(int) apr_brigade_putc(apr_bucket_brigade *b, const char c)
+{
+ apr_size_t nbyte = 1;
+ const char *str = &c;
+
+ if (check_brigade_flush(&str, &nbyte, b)) {
+ if (b->flush) {
+ return b->flush(b, b->ctx);
+ }
}
+ else {
+ apr_bucket *buck = APR_BRIGADE_LAST(b);
+ apr_bucket_shared *s;
+ apr_bucket_heap *h;
+ char *buf;
- return k;
+ if (!APR_BUCKET_IS_HEAP(buck) || APR_BRIGADE_EMPTY(b)) {
+ buf = malloc(APR_BUCKET_BUFF_SIZE);
+
+ buck = apr_bucket_create_heap(buf, APR_BUCKET_BUFF_SIZE, 0, NULL);
+ s = buck->data;
+ s->start = s->end = 0;
+ h = s->data;
+
+ APR_BRIGADE_INSERT_TAIL(b, buck);
+ }
+ else {
+ buf = h->base + s->end;
+
+ s = buck->data;
+ h = s->data;
+ }
+ memcpy(buf, &c, 1);
+ s->end++;
+ }
+
+ return 1;
+}
+
+APU_DECLARE(int) apr_brigade_write(apr_bucket_brigade *b, const char *str, apr_size_t nbyte)
+{
+ if (check_brigade_flush(&str, &nbyte, b)) {
+ if (b->flush) {
+ return b->flush(b, b->ctx);
+ }
+ }
+ else {
+ apr_bucket *buck = APR_BRIGADE_LAST(b);
+ apr_bucket_shared *s;
+ apr_bucket_heap *h;
+ char *buf;
+
+ if (!APR_BUCKET_IS_HEAP(buck) || APR_BRIGADE_EMPTY(b)) {
+ buf = malloc(APR_BUCKET_BUFF_SIZE);
+
+ buck = apr_bucket_create_heap(buf, APR_BUCKET_BUFF_SIZE, 0, NULL);
+ s = buck->data;
+ s->start = s->end = 0;
+ h = s->data;
+
+ APR_BRIGADE_INSERT_TAIL(b, buck);
+ }
+ else {
+ s = buck->data;
+ h = s->data;
+
+ buf = h->base + s->end;
}
+ memcpy(buf, str, nbyte);
+ s->end += nbyte;
+ }
+ return nbyte;
+}
+
+APU_DECLARE(int) apr_brigade_puts(apr_bucket_brigade *b, const char *str)
+{
+ return apr_brigade_write(b, str, strlen(str));
+}
+
APU_DECLARE_NONSTD(int) apr_brigade_putstrs(apr_bucket_brigade *b, ...)
{
va_list va;
@@ -240,13 +344,9 @@
* directly into a bucket. I'm being lazy right now. RBB
*/
char buf[4096];
- apr_bucket *r;
- int res;
- res = apr_vsnprintf(buf, 4096, fmt, va);
-
- r = apr_bucket_create_heap(buf, strlen(buf), 1, NULL);
- APR_BRIGADE_INSERT_TAIL(b, r);
+ apr_vsnprintf(buf, 4096, fmt, va);
- return res;
+ return apr_brigade_puts(b, buf);
}
+
Index: srclib/apr-util/include/apr_buckets.h
===================================================================
RCS file: /home/cvs/apr-util/include/apr_buckets.h,v
retrieving revision 1.68
diff -u -d -b -w -u -r1.68 apr_buckets.h
--- srclib/apr-util/include/apr_buckets.h 2001/01/27 14:52:55 1.68
+++ srclib/apr-util/include/apr_buckets.h 2001/02/06 05:20:14
@@ -78,6 +78,8 @@
* @package Bucket Brigades
*/
+#define APR_BUCKET_BUFF_SIZE 9000
+
typedef enum {APR_BLOCK_READ, APR_NONBLOCK_READ} apr_read_type_e;
/*
@@ -237,6 +239,8 @@
* the destroying function is responsible for killing the cleanup.
*/
apr_pool_t *p;
+ void *ctx;
+ apr_status_t (*flush)(apr_bucket_brigade *bb, void *ctx);
/** The buckets in the brigade are on this list. */
/*
* XXX: the apr_bucket_list structure doesn't actually need a name tag
@@ -533,7 +537,7 @@
* modified, it is only used to free the bucket.
*/
char *base;
- /** how much memory was allocated. This may not be necessary */
+ /** how much memory was allocated */
size_t alloc_len;
};
@@ -629,8 +633,7 @@
struct iovec *vec, int nvec);
/**
- * This function writes a list of strings into a bucket brigade. We just
- * allocate a new heap bucket for each string.
+ * This function writes a list of strings into a bucket brigade.
* @param b The bucket brigade to add to
* @param va A list of strings to add
* @return The number of bytes added to the brigade
@@ -639,8 +642,34 @@
APU_DECLARE(int) apr_brigade_vputstrs(apr_bucket_brigade *b, va_list va);
/**
+ * This function writes an string into a bucket brigade.
+ * @param b The bucket brigade to add to
+ * @param str The string to add
+ * @return The number of bytes added to the brigade
+ * @deffunc int apr_brigade_write(ap_bucket_brigade *b, const char *str)
+ */
+APU_DECLARE(int) apr_brigade_write(apr_bucket_brigade *b, const char *str, apr_size_t nbyte);
+
+/**
+ * This function writes an string into a bucket brigade.
+ * @param b The bucket brigade to add to
+ * @param str The string to add
+ * @return The number of bytes added to the brigade
+ * @deffunc int apr_brigade_puts(ap_bucket_brigade *b, const char *str)
+ */
+APU_DECLARE(int) apr_brigade_puts(apr_bucket_brigade *b, const char *str);
+
+/**
+ * This function writes a character into a bucket brigade.
+ * @param b The bucket brigade to add to
+ * @param c The character to add
+ * @return The number of bytes added to the brigade
+ * @deffunc int apr_brigade_putc(apr_bucket_brigade *b, const char c)
+ */
+APU_DECLARE(int) apr_brigade_putc(apr_bucket_brigade *b, const char c);
+
+/**
* This function writes an unspecified number of strings into a bucket brigade.
- * We just allocate a new heap bucket for each string.
* @param b The bucket brigade to add to
* @param ... The strings to add
* @return The number of bytes added to the brigade
@@ -649,7 +678,7 @@
APU_DECLARE_NONSTD(int) apr_brigade_putstrs(apr_bucket_brigade *b, ...);
/**
- * Evaluate a printf and put the resulting string into a bucket at the end
+ * Evaluate a printf and put the resulting string at the end
* of the bucket brigade.
* @param b The brigade to write to
* @param fmt The format of the string to write
@@ -661,7 +690,7 @@
const char *fmt, ...);
/**
- * Evaluate a printf and put the resulting string into a bucket at the end
+ * Evaluate a printf and put the resulting string at the end
* of the bucket brigade.
* @param b The brigade to write to
* @param fmt The format of the string to write
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] final brigade buffering
Posted by rb...@covalent.net.
I am done arguing about this. I have handled every single issue you have
raised, and I am no longer interested in working on this. The current
OLD_WRITE solution does not solve our problem, it hides it. This solution
solves the problem, and is faster than what we are currently using.
> I'd like to see a patch with *just* the brigade buffering. This "all or
> none" is a bit much, especially because I have a different solution that I'd
> like to propose for the r->bb ordering thing.
Then commit this, and then do the work. Please let us move forward with
this. Just because we have something in the tree doesn't mean that we
can't change it.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] final brigade buffering
Posted by Gordon Henriksen <go...@actifunds.com>.
On Fri, 9 Feb 2001, Rodent of Unusual Size wrote:
> rbb@covalent.net wrote:
>
>>> I have an issue with the ap_r* implementation in the patch.
>
> Ryan (and everyone), when you quote someone, *please* leave its name
> at the top of the quotation.. in long and involved threads with lots
> of back-and-forthing, it becomes very difficult to figure out who said
> what otherwise.
It's really not all that tough to figure it out; if Ryan is posting, the
quoted material is Greg's, and if Greg is posting, the quoted material is
Ryan's.
--
Gordon Henriksen
gordon@actifunds.com
Re: [PATCH] final brigade buffering
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
rbb@covalent.net wrote:
>
> > I have an issue with the ap_r* implementation in the patch.
Ryan (and everyone), when you quote someone, *please* leave
its name at the top of the quotation.. in long and involved
threads with lots of back-and-forthing, it becomes very
difficult to figure out who said what otherwise.
--
#ken P-)}
Ken Coar <http://Golux.Com/coar/>
Apache Software Foundation <http://www.apache.org/>
"Apache Server for Dummies" <http://Apache-Server.Com/>
"Apache Server Unleashed" <http://ApacheUnleashed.Com/>
Re: [PATCH] final brigade buffering
Posted by rb...@covalent.net.
> I have an issue with the ap_r* implementation in the patch. I believe we can
> solve the problem without r->bb, thus avoiding ordering problems or
> requirements that people always use r->bb. I'd ask that you leave out that
> portion, and I can reimplement ap_r* and OLD_WRITE in terms of the new
> brigade buffering stuff.
I seriously dislike this. You are now solving the same problem two
different ways. Plus, the filter is the wrong solution. The ap_r*
problem is one of buffering data.
Also, by simply using the r->bb, we put all of the control in the hands of
the actual programmer. The only person who can mess things up, is the
programmer who wrote the handler. By using a filter for this, I have to
rely on other filters to be written properly, or I remove the benefit of
the buffering.
I am very much against solving the buffering problem with a filter.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] final brigade buffering
Posted by rb...@covalent.net.
> It is nicer to have it right before the patch is applied, and I'm more than
> willing to do that (therefore, my offer to code a patch to show what I
> mean). But the important part is that we are of a like mind and can agree
> that it can/should be changed in a second round.
Why is it nice to have everything done before we commit? It is much
better to get the code in the tree, where I don't have to port everytime a
change is made.
> I'm hoping that you'll understand, rather than give in. The abstractions in
> the code are important, especially when we're basing so much of our data
> flow around them. My intent is to explain where the abstraction in the
> current patch breaks down, and offer suggestions on how to improve it. I
> feel that I'm not explaining this well, because (while it may be obvious to
> me) it hasn't been clicking into place with others. I don't know how to
> explain better, despite trying; I've offered to do the code as a way to
> demonstrate what I mean.
Greg, I do understand. I disagree. I have decided to agree to disagree
and to allow this to move forward. That is very different than giving
in. Giving in is what I did a few weeks ago when I got so sick of this
that I seriously considered leaving the group (and did for a day and a
half). Since that time, I have discovered that I don't care enough about
this to fight anymore. I would rather get out of the way and let the
project move forward.
> The brigade buffering portion of the patch is fine to check in as-is, if
> you're fine with changing it in a second step. If you want to do it
> beforehand, then fine.
I will fix it and commit later tonight.
> I have an issue with the ap_r* implementation in the patch. I believe we can
> solve the problem without r->bb, thus avoiding ordering problems or
> requirements that people always use r->bb. I'd ask that you leave out that
> portion, and I can reimplement ap_r* and OLD_WRITE in terms of the new
> brigade buffering stuff.
I will commit without this portion tonight, but as I have already stated,
I am very much against this change.
> Were there any other pieces to the patch beyond that?
> [ 1) brigade buffering. 2) ap_r* revamp. 3) ap_f*. 4) header filter ]
Nope.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] final brigade buffering
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 08, 2001 at 07:24:35PM -0800, rbb@covalent.net wrote:
>...
> The apr_brigade_* stuff is more general, and requires a bit more work for
> the programmer. This is very similar to the way that we had the BUFF code
> in 1.3, and nobody ever wrote to the BUFF, because everybody used ap_r*.
I'm trying to say that it *doesn't* require more work for the programmer.
The programmer shouldn't have to remember who stuck values into ->flush and
->ctx. There is no reason to give them work.
> > I'll code up a patch that demonstrates this flushing stuff, and which
> > doesn't have these interoperation/coupling problems.
>
> Why? I know what you want to do, you want to move the flush function and
> the ctx pointer out of the brigade and into the parameter list. This
> doesn't require a patch, it is a difference of opinion, and we can safely
> ignore this until the code is actually committed, at which point, we could
> easily change it.
We are discussing the patch. I'm saying, "the mechanism for flushing is
incorrect, and I'd like to see it fixed." I'm hoping that you'll understand
my reasons behind that, rather than simply giving in.
It is nicer to have it right before the patch is applied, and I'm more than
willing to do that (therefore, my offer to code a patch to show what I
mean). But the important part is that we are of a like mind and can agree
that it can/should be changed in a second round.
>...
> > Flushing *does* depend on where you are. If you're in a handler or in a
> > filter, the flush must operate very different (specifically: where do you
> > send the brigade). You're covering that up by forcing people to use ap_r*
> > and ap_f*. That is simply a bandaid over the real problem.
>
> Flushing has nothing to do with where you are. It has to do with the
> brigade you are operating on. In Apache, that brigade changes based on
> where the brigade is. That is in Apache.
You are saying the exact same thing. Depending on where you are in the code,
the flushing must be performed different. If you want to attach that to the
brigade concept, then feel free. But it has nothing to do with the brigade,
and everything about how a particular piece of code needs to flush the
brigade to the next piece. The brigade is an independent structure within
that control flow. Brigades have no semantic other than being a blob.
The act of writing into a brigade is what may cause a flush, and that act
imposes different requirements on how to flush at each point.
> Take a cache as an example, let's assume I create a cache that is
> basically a bucket brigade, one element per bucket. When that bucket
> brigade becomes full, I want to flush to disk. How I flush to disk
> doesn't change, the ctx doesn't change. This is because the brigade has a
> specified behavior that it does when the brigade gets too full. I can
> safely fill out some fields in the brigade after I create it, and
> everything just works forever.
>
> Another example. I am writing my word processor that uses a bucket
> brigade, one word per bucket. I create the brigade and tell it how to
> flush to disk. Again, the flush function and the flush ctx don't change,
> because this is a feature of the brigade.
It is not a feature of the brigade. The brigade holds data. The code that is
constructing and manipulating the brigade says where it is supposed to go.
You're sending it to disk in the above examples. Apache sends it to filters.
In the cache example, the code may determine the brigade should be sent to a
pipe. That's the code doing the decision, independent of what the brigade
thinks. And the code may not decide what to do with the brigade until
certain other triggers have been met (e.g. brigade is too big). The code
makes the choice; the brigade just holds some data.
> > Look at it from a code flow standpoint: every time you call apr_brigade_*
> > (in the ap_r* and ap_f* functions, which are our only examples so far),
> > you're setting the flush and ctx variables in the brigade. Doesn't that just
> > scream "parameter!" to you?
>
> No. It screams that Apache is using brigades incorrectly. If we did what
> Roy has suggested all along, basically, each filter has a bucket brigade
> that it uses to store the information, then the flush function and the ctx
> wouldn't change for the lifetime of the filter (and thus the lifetime of
> the brigade).
That is never what Roy said. Don't go there.
A brigade has always been a data structure to carry information through a
series of filters. Each filter applies its processing at each step to that
brigade, then passes it to the next one.
>...
> Having said all of that, as I have said multiple times recently, I am not
> arguing anymore. I will make this change, just to get the correct
> solution into Apache. If I make this change, can I finally commit this
> patch?
I'm hoping that you'll understand, rather than give in. The abstractions in
the code are important, especially when we're basing so much of our data
flow around them. My intent is to explain where the abstraction in the
current patch breaks down, and offer suggestions on how to improve it. I
feel that I'm not explaining this well, because (while it may be obvious to
me) it hasn't been clicking into place with others. I don't know how to
explain better, despite trying; I've offered to do the code as a way to
demonstrate what I mean.
In this case, we have contention for the flush/ctx values in the brigade,
and it makes them hard to work with unless you can be aware of what other
pieces of code are doing. That awareness of others is "coupling" and is the
surest sign of future bugs. We modularize code to reduce coupling. Keeping
the flush func/ctx out of the brigade reduces coupling between the codes
that write into the brigade -- they don't have to be concerned with trying
to figure out if somebody changed the func/ctx on them.
The brigade buffering portion of the patch is fine to check in as-is, if
you're fine with changing it in a second step. If you want to do it
beforehand, then fine.
I have an issue with the ap_r* implementation in the patch. I believe we can
solve the problem without r->bb, thus avoiding ordering problems or
requirements that people always use r->bb. I'd ask that you leave out that
portion, and I can reimplement ap_r* and OLD_WRITE in terms of the new
brigade buffering stuff.
The ap_f* functions seem fine in concept, and the header filter stuff, too.
If there are any problems in there, we can deal with them post-patch. I'm
only concerned with getting the abstractions/concepts set up properly to
start with.
Were there any other pieces to the patch beyond that?
[ 1) brigade buffering. 2) ap_r* revamp. 3) ap_f*. 4) header filter ]
thx,
-g
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] final brigade buffering
Posted by rb...@covalent.net.
> We should not require the user to have to track all the ins and outs of the
> APIs like that. There is just *way* too much coupling.
>
> As I read it, you're saying that I cannot use apr_brigade_* in my code. But
> that doesn't make much sense... I want to work directly with brigades for
> their speed benefit.
Not at all. I am saying that you should feel free to use apr_brigade_* in
your code, but you should know what that means. Basically, I am fixing a
general problem with apr_brigade_*, and fixing the specific problem for
Apache. I am asking programmers to know _1_ API, the ap_f* API. The
other API's, ap_r* and apr_brigade_* are irrelevant to Apache
programmers. The ap_r* is for backwards compatability, so we must have
it, but it is just a small wrapper around ap_f*.
The apr_brigade_* stuff is more general, and requires a bit more work for
the programmer. This is very similar to the way that we had the BUFF code
in 1.3, and nobody ever wrote to the BUFF, because everybody used ap_r*.
> I'll code up a patch that demonstrates this flushing stuff, and which
> doesn't have these interoperation/coupling problems.
Why? I know what you want to do, you want to move the flush function and
the ctx pointer out of the brigade and into the parameter list. This
doesn't require a patch, it is a difference of opinion, and we can safely
ignore this until the code is actually committed, at which point, we could
easily change it.
> "... expects the programmer to think about what they want to do" ... I don't
> believe we need to place that burden on them.
>
> Flushing *does* depend on where you are. If you're in a handler or in a
> filter, the flush must operate very different (specifically: where do you
> send the brigade). You're covering that up by forcing people to use ap_r*
> and ap_f*. That is simply a bandaid over the real problem.
Flushing has nothing to do with where you are. It has to do with the
brigade you are operating on. In Apache, that brigade changes based on
where the brigade is. That is in Apache.
Take a cache as an example, let's assume I create a cache that is
basically a bucket brigade, one element per bucket. When that bucket
brigade becomes full, I want to flush to disk. How I flush to disk
doesn't change, the ctx doesn't change. This is because the brigade has a
specified behavior that it does when the brigade gets too full. I can
safely fill out some fields in the brigade after I create it, and
everything just works forever.
Another example. I am writing my word processor that uses a bucket
brigade, one word per bucket. I create the brigade and tell it how to
flush to disk. Again, the flush function and the flush ctx don't change,
because this is a feature of the brigade.
> Look at it from a code flow standpoint: every time you call apr_brigade_*
> (in the ap_r* and ap_f* functions, which are our only examples so far),
> you're setting the flush and ctx variables in the brigade. Doesn't that just
> scream "parameter!" to you?
No. It screams that Apache is using brigades incorrectly. If we did what
Roy has suggested all along, basically, each filter has a bucket brigade
that it uses to store the information, then the flush function and the ctx
wouldn't change for the lifetime of the filter (and thus the lifetime of
the brigade). Just because we are changing the purpose of the brigade
multiple times per request, doesn't mean we are doing it correctly. In
our code, each brigade changes it's role during the request
processing. When the request starts, the brigade's job is to get the data
from the handler to filter1, then it is to get the data from filter1 to
filter2, etc... We have this problem, because we force the brigade to
change jobs, the fact that this shows up when the brigade changes location
is a side-effect of how we use brigades, not an inherent property of
them.
> The problem is not specific to Apache. If somebody else uses brigades, and
> they have a flush function which wants to use apr_brigade_*, then they fall
> into the same trap. I don't think we want to say, "APRUTIL has this problem
> with flush functions that can't use apr_brigade_*, but hey... we worked
> around it in Apache."
I really have no idea what this means. The flush function is used by
apr_brigade_*. The only problem they may have, is that if they change the
job of the brigade, then they will have to deal with the consequences.
> Expect a small patch later this evening which just deals with the brigade
> buffering stuff.
The other advantage to this, is that it means that all the functions have
a very similar footprint.
Having said all of that, as I have said multiple times recently, I am not
arguing anymore. I will make this change, just to get the correct
solution into Apache. If I make this change, can I finally commit this
patch?
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] final brigade buffering
Posted by Greg Stein <gs...@lyra.org>.
We should not require the user to have to track all the ins and outs of the
APIs like that. There is just *way* too much coupling.
As I read it, you're saying that I cannot use apr_brigade_* in my code. But
that doesn't make much sense... I want to work directly with brigades for
their speed benefit.
I'll code up a patch that demonstrates this flushing stuff, and which
doesn't have these interoperation/coupling problems.
"... expects the programmer to think about what they want to do" ... I don't
believe we need to place that burden on them.
Flushing *does* depend on where you are. If you're in a handler or in a
filter, the flush must operate very different (specifically: where do you
send the brigade). You're covering that up by forcing people to use ap_r*
and ap_f*. That is simply a bandaid over the real problem.
Look at it from a code flow standpoint: every time you call apr_brigade_*
(in the ap_r* and ap_f* functions, which are our only examples so far),
you're setting the flush and ctx variables in the brigade. Doesn't that just
scream "parameter!" to you?
The problem is not specific to Apache. If somebody else uses brigades, and
they have a flush function which wants to use apr_brigade_*, then they fall
into the same trap. I don't think we want to say, "APRUTIL has this problem
with flush functions that can't use apr_brigade_*, but hey... we worked
around it in Apache."
Expect a small patch later this evening which just deals with the brigade
buffering stuff.
Cheers,
-g
On Thu, Feb 08, 2001 at 05:06:33PM -0800, rbb@covalent.net wrote:
>
> You were perfectly clear, but you are mixing APIs. There are two Apache
> API's and one APR-util API.
>
> The two Apache API's are:
>
> ap_r*
> ap_f*
>
> The one APR-util API is:
>
> apr_brigade_*
>
> It is perfectly possible for module writers to call any of the three at
> any time, but they have to know what they are doing.
>
> Our ap_f* calls setup the ctx correctly for when they call apr_brigade_*,
> so any module author using the Apache API doesn't have to worry about it
> at all. The APR-util API expect's the programmer to think about what they
> want to do, and write the code accordingly.
>
> The problem with the code below, is that you are looking to use the APR
> functions without really thinking about how they work. Our brigades are
> used for filters, so how we flush depends on which filter we are in. In
> the grand scheme of things, brigades are data stores, and how you flush
> depends on the properties of the brigade, not on where you are.
>
> Yes, if you write code like what you have below, you will not get what you
> expect. But, you are using the wrong tool for the job. For what you
> want, you don't want to use the brigade functions, you want to use the
> Apache specific functions.
>
> Ryan
>
>
> > Hrm. I'm sorry that I wasn't clear enough... it is pretty clear to me about
> > the need, so I obviously missed a point in explaining it well enough. Let's
> > use code this time. That is quite a bit more rigorous than english :-)
> >
> > Consider this handler and filter (and that the filter is on the chain):
> >
> > void my_handler(...)
> > {
> > bb = brigade_create()
> > bb->flush = my_flush;
> > bb->flush_ctx = r->output_filters;
> >
> > apr_brigade_write(bb, "some big-ass string that causes a flush");
> > apr_brigade_write(bb, "another big-ass string");
> > }
> >
> > apr_status_t my_flush(bb, ctx)
> > {
> > return ap_pass_brigade(ctx, bb);
> > }
> >
> > apr_status_t some_filter(f, bb)
> > {
> > bb->flush = some_filter_flush;
> > bb->flush_ctx = f->next;
> >
> > apr_brigade_write(bb, "small or big, this doesn't matter");
> > }
> >
> > apr_status_t some_filter_flush(bb, ctx)
> > {
> > return ap_pass_brigade(ctx, bb);
> > }
> >
> >
> > The above code is subtly broken because of the flush function and context
> > are placed into the brigade. The values are very much context-dependent. By
> > placing them into the brigade itself, it is creating a situation very
> > similar to globals: you have contention for the values that should be in
> > there. Each caller of apr_brigade_write needs a different set, because they
> > have different needs on *how* to get the brigade flushed.
> >
> > Since each caller has their own needs, it makes the most sense to place them
> > directly into the call itself (as parameters). Otherwise, every single call
> > will be preceded by setting flush_func/flush_ctx. In that case, it will be
> > immediately apparent that they should be paremeters :-)
> >
> > I'd suggest that all the brigade write functions have a standardized
> > prototype along the lines of:
> >
> > apr_status_t apr_brigade_whatever(bb, flush_func, flush_ctx, ...params)
> >
> > flush_func can be NULL; if so, then the brigade buffering may need to copy
> > the values into a heap bucket (regardless of size).
> >
> >
> > For those that haven't seen the bug in the above code yet, consider the
> > *second* call to apr_brigade_write in the handler. Because the first call
> > caused a flush through the filter stack, and some_filter wrote over the
> > flush values, then the second write in the handler gets flushed to the wrong
> > location.
> >
> > Cheers,
> > -g
> >
> > --
> > Greg Stein, http://www.lyra.org/
> >
> >
>
>
> _______________________________________________________________________________
> Ryan Bloom rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> -------------------------------------------------------------------------------
>
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] final brigade buffering
Posted by rb...@covalent.net.
You were perfectly clear, but you are mixing APIs. There are two Apache
API's and one APR-util API.
The two Apache API's are:
ap_r*
ap_f*
The one APR-util API is:
apr_brigade_*
It is perfectly possible for module writers to call any of the three at
any time, but they have to know what they are doing.
Our ap_f* calls setup the ctx correctly for when they call apr_brigade_*,
so any module author using the Apache API doesn't have to worry about it
at all. The APR-util API expect's the programmer to think about what they
want to do, and write the code accordingly.
The problem with the code below, is that you are looking to use the APR
functions without really thinking about how they work. Our brigades are
used for filters, so how we flush depends on which filter we are in. In
the grand scheme of things, brigades are data stores, and how you flush
depends on the properties of the brigade, not on where you are.
Yes, if you write code like what you have below, you will not get what you
expect. But, you are using the wrong tool for the job. For what you
want, you don't want to use the brigade functions, you want to use the
Apache specific functions.
Ryan
> Hrm. I'm sorry that I wasn't clear enough... it is pretty clear to me about
> the need, so I obviously missed a point in explaining it well enough. Let's
> use code this time. That is quite a bit more rigorous than english :-)
>
> Consider this handler and filter (and that the filter is on the chain):
>
> void my_handler(...)
> {
> bb = brigade_create()
> bb->flush = my_flush;
> bb->flush_ctx = r->output_filters;
>
> apr_brigade_write(bb, "some big-ass string that causes a flush");
> apr_brigade_write(bb, "another big-ass string");
> }
>
> apr_status_t my_flush(bb, ctx)
> {
> return ap_pass_brigade(ctx, bb);
> }
>
> apr_status_t some_filter(f, bb)
> {
> bb->flush = some_filter_flush;
> bb->flush_ctx = f->next;
>
> apr_brigade_write(bb, "small or big, this doesn't matter");
> }
>
> apr_status_t some_filter_flush(bb, ctx)
> {
> return ap_pass_brigade(ctx, bb);
> }
>
>
> The above code is subtly broken because of the flush function and context
> are placed into the brigade. The values are very much context-dependent. By
> placing them into the brigade itself, it is creating a situation very
> similar to globals: you have contention for the values that should be in
> there. Each caller of apr_brigade_write needs a different set, because they
> have different needs on *how* to get the brigade flushed.
>
> Since each caller has their own needs, it makes the most sense to place them
> directly into the call itself (as parameters). Otherwise, every single call
> will be preceded by setting flush_func/flush_ctx. In that case, it will be
> immediately apparent that they should be paremeters :-)
>
> I'd suggest that all the brigade write functions have a standardized
> prototype along the lines of:
>
> apr_status_t apr_brigade_whatever(bb, flush_func, flush_ctx, ...params)
>
> flush_func can be NULL; if so, then the brigade buffering may need to copy
> the values into a heap bucket (regardless of size).
>
>
> For those that haven't seen the bug in the above code yet, consider the
> *second* call to apr_brigade_write in the handler. Because the first call
> caused a flush through the filter stack, and some_filter wrote over the
> flush values, then the second write in the handler gets flushed to the wrong
> location.
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: error?
Posted by James Sutherland <ja...@cam.ac.uk>.
On Thu, 8 Feb 2001, Michael Gleissner wrote:
> " Status: 302 Moved Location: http://server/cgi-bin/spam/fini.pl "
i.e. "You're looking in the wrong place: the correct URL is
http://server..." That's what your code should be producing.
> This is the error I get when I try to redirect from a perl script.
> Any idea what it means?
>
> This is the code for the redirect:
>
> use CGI;
>
> $co = new CGI;
>
> print $co->redirect('http://server/cgi-bin/spam/fini.pl');
>
> print $co->start_html,
>
> $co->end_html;
It means you've issued an HTTP redirect to that URL. What's wrong with
that?
James.
error?
Posted by Michael Gleissner <mg...@harper.cc.il.us>.
" Status: 302 Moved Location: http://server/cgi-bin/spam/fini.pl "
This is the error I get when I try to redirect from a perl script.
Any idea what it means?
This is the code for the redirect:
use CGI;
$co = new CGI;
print $co->redirect('http://server/cgi-bin/spam/fini.pl');
print $co->start_html,
$co->end_html;
Thanks,
******************************************
Michael Gleissner
Unix System Administrator
Office: A234
Extension: 6831
Re: [PATCH] final brigade buffering
Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 08, 2001 at 06:52:01AM -0800, rbb@covalent.net wrote:
>...
> One more thing about this. How we flush is not a feature of where we are,
> it is a feature of a brigade. Apache happens to use the same brigade in
> a multitidude of locations throughout the code. When I write my word
> processor using brigades, my flush function and context will always be the
> same, when the brigade gets to full, I will be flushing to disk, into a
> specific file.
>
> This also fits very well into the brigades that are stored in a filter's
> ctx field. Those brigades don't move, they stay in that f->ctx block, and
> thus the flush mechanism is different for them than for brigades that we
> pass down the stack, but it is always the same for that brigade.
>
> Brigades are data stores, and a data store doesn't care where they are
> when they flush data, they just care that they have to flush, and they
> flush correctly.
Hrm. I'm sorry that I wasn't clear enough... it is pretty clear to me about
the need, so I obviously missed a point in explaining it well enough. Let's
use code this time. That is quite a bit more rigorous than english :-)
Consider this handler and filter (and that the filter is on the chain):
void my_handler(...)
{
bb = brigade_create()
bb->flush = my_flush;
bb->flush_ctx = r->output_filters;
apr_brigade_write(bb, "some big-ass string that causes a flush");
apr_brigade_write(bb, "another big-ass string");
}
apr_status_t my_flush(bb, ctx)
{
return ap_pass_brigade(ctx, bb);
}
apr_status_t some_filter(f, bb)
{
bb->flush = some_filter_flush;
bb->flush_ctx = f->next;
apr_brigade_write(bb, "small or big, this doesn't matter");
}
apr_status_t some_filter_flush(bb, ctx)
{
return ap_pass_brigade(ctx, bb);
}
The above code is subtly broken because of the flush function and context
are placed into the brigade. The values are very much context-dependent. By
placing them into the brigade itself, it is creating a situation very
similar to globals: you have contention for the values that should be in
there. Each caller of apr_brigade_write needs a different set, because they
have different needs on *how* to get the brigade flushed.
Since each caller has their own needs, it makes the most sense to place them
directly into the call itself (as parameters). Otherwise, every single call
will be preceded by setting flush_func/flush_ctx. In that case, it will be
immediately apparent that they should be paremeters :-)
I'd suggest that all the brigade write functions have a standardized
prototype along the lines of:
apr_status_t apr_brigade_whatever(bb, flush_func, flush_ctx, ...params)
flush_func can be NULL; if so, then the brigade buffering may need to copy
the values into a heap bucket (regardless of size).
For those that haven't seen the bug in the above code yet, consider the
*second* call to apr_brigade_write in the handler. Because the first call
caused a flush through the filter stack, and some_filter wrote over the
flush values, then the second write in the handler gets flushed to the wrong
location.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] final brigade buffering
Posted by rb...@covalent.net.
> > Within the brigade buffering, I'm also concerned about the flush mechanism.
> > Let's say a handler creates the brigade. "flush" for him means to send it to
> > ap_pass_brigade(r->output_filters, bb). But for a filter, it means
> > ap_pass_brigade(f->next, bb). But the flush info is within the brigade
> > itself, rather than based on the context of when the brigade writing is
> > done. In that respect, I believe two optional params to each brigade writing
> > function is needed.
>
> BTW, this is just untrue. The flush semantics are they way they are,
> because we have no choice. We can not assume what the flush function will
> look like.
>
> In the Apache code, we setup the ctx from the ap_f* functions, which then
> call the apr_brigade_* functions. This idea actually came right off the
> development list, and it is implemented exactly as it was discussed here.
One more thing about this. How we flush is not a feature of where we are,
it is a feature of a brigade. Apache happens to use the same brigade in
a multitidude of locations throughout the code. When I write my word
processor using brigades, my flush function and context will always be the
same, when the brigade gets to full, I will be flushing to disk, into a
specific file.
This also fits very well into the brigades that are stored in a filter's
ctx field. Those brigades don't move, they stay in that f->ctx block, and
thus the flush mechanism is different for them than for brigades that we
pass down the stack, but it is always the same for that brigade.
Brigades are data stores, and a data store doesn't care where they are
when they flush data, they just care that they have to flush, and they
flush correctly.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] final brigade buffering
Posted by rb...@covalent.net.
> Within the brigade buffering, I'm also concerned about the flush mechanism.
> Let's say a handler creates the brigade. "flush" for him means to send it to
> ap_pass_brigade(r->output_filters, bb). But for a filter, it means
> ap_pass_brigade(f->next, bb). But the flush info is within the brigade
> itself, rather than based on the context of when the brigade writing is
> done. In that respect, I believe two optional params to each brigade writing
> function is needed.
BTW, this is just untrue. The flush semantics are they way they are,
because we have no choice. We can not assume what the flush function will
look like.
In the Apache code, we setup the ctx from the ap_f* functions, which then
call the apr_brigade_* functions. This idea actually came right off the
development list, and it is implemented exactly as it was discussed here.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] final brigade buffering
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Feb 07, 2001 at 07:59:35AM -0800, rbb@covalent.net wrote:
> Has anybody reviewed this yet? I believe I have covered all of the
> problems with the previous patches, and have done it in a way that is
> useful for any program to use. I am 99.9% sure that this also solves the
> problem for mod_perl, mod_snake, mod_proxy, and the other modules. I know
> this solves the Apache header problem with less of a hack.
>
> Since we aren't freezing the tree, I would like to get this committed.
I have done some basic review on the brigade buffering part, but not the
rest.
I'd like to see a patch with *just* the brigade buffering. This "all or
none" is a bit much, especially because I have a different solution that I'd
like to propose for the r->bb ordering thing.
Within the brigade buffering, I'm also concerned about the flush mechanism.
Let's say a handler creates the brigade. "flush" for him means to send it to
ap_pass_brigade(r->output_filters, bb). But for a filter, it means
ap_pass_brigade(f->next, bb). But the flush info is within the brigade
itself, rather than based on the context of when the brigade writing is
done. In that respect, I believe two optional params to each brigade writing
function is needed.
But my main point: can we break it down into separate patches, then work
through them in sequence?
(e.g. brigade buffering with comments; no apparent issues with the header
part; an alternative for r->bb ordering; then the filter writing stuff)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] final brigade buffering
Posted by rb...@covalent.net.
Has anybody reviewed this yet? I believe I have covered all of the
problems with the previous patches, and have done it in a way that is
useful for any program to use. I am 99.9% sure that this also solves the
problem for mod_perl, mod_snake, mod_proxy, and the other modules. I know
this solves the Apache header problem with less of a hack.
Since we aren't freezing the tree, I would like to get this committed.
Ryan
On Mon, 5 Feb 2001 rbb@covalent.net wrote:
>
> This is the final brigade buffering patch. What it basically does. We
> use a simple heap bucket to buffer the data. Whenever we fill a buffer,
> we automatically flush the entire brigade, using a flush function from the
> brigade itself. We don't copy unless we can copy the entire string into
> the brigade with a single copy.
>
> The only remaining complaint that I know of with this patch, is that it
> forces handlers to use r->bb as their brigade if they want to make sure
> that the data is never sent out of order. This also removes the OLD_WRITE
> buffer, and it begins to remove some of the other buffering logic we have
> added.
>
> This also allows modules like mod_perl to remove their special buffering
> logic. I would really like to get this committed. I will not even try to
> say that this code is bug free, but it has served a lot of pages very
> cleanly with this patch now.
>
> Since we are no longer freezing the tree, I would like to get this in
> soon.
>
> Ryan
>
>
> Index: include/http_protocol.h
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/include/http_protocol.h,v
> retrieving revision 1.51
> diff -u -d -b -w -u -r1.51 http_protocol.h
> --- include/http_protocol.h 2001/02/05 03:31:43 1.51
> +++ include/http_protocol.h 2001/02/06 05:19:31
> @@ -88,13 +88,13 @@
> /**
> * Send the minimal part of an HTTP response header.
> * @param r The current request
> - * @param buf The buffer to add the header to.
> + * @param bb The brigade to add the header to.
> * @warning Modules should be very careful about using this, and should
> * prefer ap_send_http_header(). Much of the HTTP/1.1 implementation
> * correctness depends on code in ap_send_http_header().
> - * @deffunc void ap_basic_http_header(request_rec *r, char *buf)
> + * @deffunc void ap_basic_http_header(request_rec *r, apr_bucket_brigade *bb)
> */
> -AP_DECLARE(void) ap_basic_http_header(request_rec *r, char *buf);
> +AP_DECLARE(void) ap_basic_http_header(request_rec *r, apr_bucket_brigade *bb);
>
> /**
> * Send the Status-Line and header fields for HTTP response
> @@ -310,7 +310,11 @@
> * @return The number of bytes sent
> * @deffunc int ap_rputc(int c, request_rec *r)
> */
> -AP_DECLARE(int) ap_rputc(int c, request_rec *r);
> +#define ap_rputc(c, r) \
> + if (!r->bb) { \
> + r->bb = apr_brigade_create(r->pool); \
> + } \
> + ap_fputc(r->output_filters, r->bb, c)
> /**
> * Output a string for the current request
> * @param str The string to output
> @@ -318,7 +322,11 @@
> * @return The number of bytes sent
> * @deffunc int ap_rputs(const char *str, request_rec *r)
> */
> -AP_DECLARE(int) ap_rputs(const char *str, request_rec *r);
> +#define ap_rputs(str, r) \
> + if (!r->bb) { \
> + r->bb = apr_brigade_create(r->pool); \
> + } \
> + ap_fputs(r->output_filters, r->bb, str)
> /**
> * Write a buffer for the current request
> * @param buf The buffer to write
> @@ -327,7 +335,11 @@
> * @return The number of bytes sent
> * @deffunc int ap_rwrite(const void *buf, int nbyte, request_rec *r)
> */
> -AP_DECLARE(int) ap_rwrite(const void *buf, int nbyte, request_rec *r);
> +#define ap_rwrite(str, n, r) \
> + if (!r->bb) { \
> + r->bb = apr_brigade_create(r->pool); \
> + } \
> + ap_fwrite(r->output_filters, r->bb, str, n)
> /**
> * Write an unspecified number of strings to the request
> * @param r The current request
> @@ -335,7 +347,11 @@
> * @return The number of bytes sent
> * @deffunc int ap_rvputs(request_rec *r, ...)
> */
> -AP_DECLARE_NONSTD(int) ap_rvputs(request_rec *r,...);
> +#define ap_rvputs(r, args...) \
> + if (!r->bb) { \
> + r->bb = apr_brigade_create(r->pool); \
> + } \
> + ap_fvputs(r->output_filters, r->bb, ##args)
> /**
> * Output data to the client in a printf format
> * @param r The current request
> @@ -344,7 +360,11 @@
> * @return The number of bytes sent
> * @deffunc int ap_vrprintf(request_rec *r, const char *fmt, va_list vlist)
> */
> -AP_DECLARE(int) ap_vrprintf(request_rec *r, const char *fmt, va_list vlist);
> +#define ap_vrprintf(r, fmt, vlist) \
> + if (!r->bb) { \
> + r->bb = apr_brigade_create(r->pool); \
> + } \
> + ap_vfprintf(r->output_filters, r->bb, fmt, vlist)
> /**
> * Output data to the client in a printf format
> * @param r The current request
> @@ -353,15 +373,22 @@
> * @return The number of bytes sent
> * @deffunc int ap_rprintf(request_rec *r, const char *fmt, ...)
> */
> -AP_DECLARE_NONSTD(int) ap_rprintf(request_rec *r, const char *fmt,...)
> - __attribute__((format(printf,2,3)));
> +#define ap_rprintf(r, fmt, args...) \
> + if (!r->bb) { \
> + r->bb = apr_brigade_create(r->pool); \
> + } \
> + ap_fprintf(r->output_filters, r->bb, fmt, ##args)
> /**
> * Flush all of the data for the current request to the client
> * @param r The current request
> * @return The number of bytes sent
> * @deffunc int ap_rflush(request_rec *r)
> */
> -AP_DECLARE(int) ap_rflush(request_rec *r);
> +#define ap_rflush(r) \
> + if (!r->bb) { \
> + r->bb = apr_brigade_create(r->pool); \
> + } \
> + ap_fflush(r->output_filters, r->bb)
>
> /**
> * Index used in custom_responses array for a specific error code
> Index: include/httpd.h
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/include/httpd.h,v
> retrieving revision 1.134
> diff -u -d -b -w -u -r1.134 httpd.h
> --- include/httpd.h 2001/02/05 12:55:11 1.134
> +++ include/httpd.h 2001/02/06 05:19:31
> @@ -86,6 +86,7 @@
> #include "apr_pools.h"
> #include "apr_time.h"
> #include "apr_network_io.h"
> +#include "apr_buckets.h"
>
> #ifdef CORE_PRIVATE
>
> @@ -798,6 +799,8 @@
> /** A flag to determine if the eos bucket has been sent yet
> * @defvar int eos_sent */
> int eos_sent;
> +
> + apr_bucket_brigade *bb;
>
> /* Things placed at the end of the record to avoid breaking binary
> * compatibility. It would be nice to remember to reorder the entire
> Index: include/util_filter.h
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/include/util_filter.h,v
> retrieving revision 1.38
> diff -u -d -b -w -u -r1.38 util_filter.h
> --- include/util_filter.h 2001/02/03 20:25:13 1.38
> +++ include/util_filter.h 2001/02/06 05:19:31
> @@ -393,6 +393,16 @@
> AP_DECLARE(apr_status_t) ap_save_brigade(ap_filter_t *f, apr_bucket_brigade **save_to,
> apr_bucket_brigade **b);
>
> +AP_DECLARE(apr_status_t) ap_fwrite(ap_filter_t *f, apr_bucket_brigade *bb,
> + const char *str, apr_size_t nbyte);
> +AP_DECLARE(apr_status_t) ap_fputs(ap_filter_t *f, apr_bucket_brigade *bb,
> + const char *str);
> +AP_DECLARE(apr_status_t) ap_fputc(ap_filter_t *f, apr_bucket_brigade *bb,
> + char str);
> +AP_DECLARE(apr_status_t) ap_fvputs(ap_filter_t *f, apr_bucket_brigade *bb, ...);
> +AP_DECLARE(apr_status_t) ap_fprintf(ap_filter_t *f, apr_bucket_brigade *bb, const char *fmt, ...);
> +
> +
> #ifdef __cplusplus
> }
> #endif
> Index: modules/http/http_core.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/http_core.c,v
> retrieving revision 1.254
> diff -u -d -b -w -u -r1.254 http_core.c
> --- modules/http/http_core.c 2001/02/05 18:28:47 1.254
> +++ modules/http/http_core.c 2001/02/06 05:19:35
> @@ -3593,8 +3593,6 @@
> AP_FTYPE_CONTENT);
> ap_register_output_filter("CHUNK", chunk_filter, AP_FTYPE_TRANSCODE);
> ap_register_output_filter("COALESCE", coalesce_filter, AP_FTYPE_CONTENT);
> - ap_register_output_filter("OLD_WRITE", ap_old_write_filter,
> - AP_FTYPE_CONTENT - 1);
> }
>
> AP_DECLARE_DATA module core_module = {
> Index: modules/http/http_protocol.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/http/http_protocol.c,v
> retrieving revision 1.289
> diff -u -d -b -w -u -r1.289 http_protocol.c
> --- modules/http/http_protocol.c 2001/02/01 21:54:18 1.289
> +++ modules/http/http_protocol.c 2001/02/06 05:19:38
> @@ -1587,13 +1587,15 @@
>
> static void end_output_stream(request_rec *r)
> {
> - apr_bucket_brigade *bb;
> apr_bucket *b;
>
> - bb = apr_brigade_create(r->pool);
> + if (!r->bb) {
> + r->bb = apr_brigade_create(r->pool);
> + }
> +
> b = apr_bucket_create_eos();
> - APR_BRIGADE_INSERT_TAIL(bb, b);
> - ap_pass_brigade(r->output_filters, bb);
> + APR_BRIGADE_INSERT_TAIL(r->bb, b);
> + ap_pass_brigade(r->output_filters, r->bb);
> }
>
> void ap_finalize_sub_req_protocol(request_rec *sub)
> @@ -1792,7 +1794,7 @@
>
> typedef struct header_struct {
> request_rec *r;
> - char *buf;
> + apr_bucket_brigade *bb;
> } header_struct;
>
> /* Send a single HTTP header field to the client. Note that this function
> @@ -1807,16 +1809,7 @@
>
> headfield = apr_pstrcat(h->r->pool, fieldname, ": ", fieldval, CRLF, NULL);
> ap_xlate_proto_to_ascii(headfield, strlen(headfield));
> - apr_cpystrn(h->buf, headfield, strlen(headfield) + 1);
> - h->buf += strlen(headfield);
> - return 1;
> -}
> -
> -static int compute_header_len(apr_size_t *length, const char *fieldname,
> - const char *fieldval)
> -{
> - /* The extra five are for ": " and CRLF, plus one for a '\0'. */
> - *length = *length + strlen(fieldname) + strlen(fieldval) + 6;
> + apr_brigade_write(h->bb, headfield, strlen(headfield) + 1);
> return 1;
> }
>
> @@ -1844,7 +1837,7 @@
> }
> }
>
> -static void basic_http_header(request_rec *r, char *buf, const char *protocol)
> +static void basic_http_header(request_rec *r, apr_bucket_brigade *bb, const char *protocol)
> {
> char *date = NULL;
> char *tmp;
> @@ -1857,14 +1850,13 @@
>
> tmp = apr_pstrcat(r->pool, protocol, " ", r->status_line, CRLF, NULL);
> ap_xlate_proto_to_ascii(tmp, strlen(tmp));
> - apr_cpystrn(buf, tmp, strlen(tmp) + 1);
> - buf += strlen(tmp);
> + apr_brigade_write(bb, tmp, strlen(tmp) + 1);
>
> date = apr_palloc(r->pool, APR_RFC822_DATE_LEN);
> apr_rfc822_date(date, r->request_time);
>
> h.r = r;
> - h.buf = buf;
> + h.bb = bb;
> form_header_field(&h, "Date", date);
> form_header_field(&h, "Server", ap_get_server_version());
>
> @@ -1872,12 +1864,12 @@
> apr_table_unset(r->headers_out, "Server");
> }
>
> -AP_DECLARE(void) ap_basic_http_header(request_rec *r, char *buf)
> +AP_DECLARE(void) ap_basic_http_header(request_rec *r, apr_bucket_brigade *bb)
> {
> const char *protocol;
>
> basic_http_header_check(r, &protocol);
> - basic_http_header(r, buf, protocol);
> + basic_http_header(r, bb, protocol);
> }
>
> /* Navigator versions 2.x, 3.x and 4.0 betas up to and including 4.0b2
> @@ -1897,18 +1889,19 @@
> * It is more expensive to check the User-Agent than it is to just add the
> * bytes, so we haven't used the BrowserMatch feature here.
> */
> -static void terminate_header(char *buf)
> +static void terminate_header(apr_bucket_brigade *bb)
> {
> - int len = strlen(buf);
> - char *headfield = buf + len;
> char *tmp = "X-Pad: avoid browser bug" CRLF;
> + char *crlf = CRLF;
> + apr_bucket *b = APR_BRIGADE_LAST(bb);
> + apr_bucket_shared *s = b->data;
>
> - if (len >= 255 && len <= 257) {
> - apr_cpystrn(headfield, tmp, strlen(tmp) + 1);
> - headfield += strlen(tmp);
> + if (s->end >= 255 && s->end <= 257) {
> + ap_xlate_proto_to_ascii(tmp, strlen(tmp));
> + apr_brigade_write(bb, tmp, strlen(tmp) + 1);
> }
> - apr_cpystrn(headfield, CRLF, strlen(CRLF) + 1);
> - ap_xlate_proto_to_ascii(buf + len, strlen(buf + len));
> + ap_xlate_proto_to_ascii(crlf, strlen(crlf));
> + apr_brigade_write(bb, crlf, strlen(crlf) + 1);
> }
>
> /*
> @@ -2154,42 +2147,28 @@
>
> int ap_send_http_options(request_rec *r)
> {
> - char *buff;
> - apr_bucket *b;
> - apr_bucket_brigade *bb;
> - apr_size_t len = 0;
> + apr_bucket_brigade *bb = apr_brigade_create(r->pool);
> header_struct h;
>
> if (r->assbackwards)
> return DECLINED;
> -
> - apr_table_do((int (*) (void *, const char *, const char *)) compute_header_len,
> - (void *) &len, r->headers_out, NULL);
>
> - /* Need to add a fudge factor so that the CRLF at the end of the headers
> - * and the basic http headers don't overflow this buffer.
> - */
> - len += strlen(ap_get_server_version()) + 100;
> - buff = apr_pcalloc(r->pool, len);
> - ap_basic_http_header(r, buff);
> + ap_basic_http_header(r, bb);
>
> apr_table_setn(r->headers_out, "Content-Length", "0");
> apr_table_setn(r->headers_out, "Allow", make_allow(r));
> ap_set_keepalive(r);
>
> h.r = r;
> - h.buf = buff;
> + h.bb = bb;
>
> apr_table_do((int (*) (void *, const char *, const char *)) form_header_field,
> (void *) &h, r->headers_out, NULL);
>
> - terminate_header(buff);
> + terminate_header(bb);
>
> r->bytes_sent = 0;
>
> - bb = apr_brigade_create(r->pool);
> - b = apr_bucket_create_pool(buff, strlen(buff), r->pool);
> - APR_BRIGADE_INSERT_TAIL(bb, b);
> ap_pass_brigade(r->output_filters, bb);
>
> return OK;
> @@ -2462,11 +2441,10 @@
> int i;
> char *date = NULL;
> request_rec *r = f->r;
> - char *buff, *buff_start;
> const char *clheader;
> const char *protocol;
> apr_bucket *e;
> - apr_bucket_brigade *b2;
> + apr_bucket_brigade *b2 = apr_brigade_create(r->pool);
> apr_size_t len = 0;
> header_struct h;
> header_filter_ctx *ctx = f->ctx;
> @@ -2483,7 +2461,7 @@
> }
>
> APR_BRIGADE_FOREACH(e, b) {
> - if (APR_BRIGADE_FIRST(b)->type == &ap_bucket_type_error) {
> + if (e->type == &ap_bucket_type_error) {
> ap_bucket_error *eb = e->data;
>
> ap_die(eb->status, r);
> @@ -2580,33 +2558,11 @@
> !strcmp(clheader, "0")) {
> apr_table_unset(r->headers_out, "Content-Length");
> }
> -
> - if (r->status == HTTP_NOT_MODIFIED) {
> - apr_table_do((int (*)(void *, const char *, const char *)) compute_header_len,
> - (void *) &len, r->headers_out,
> - "Connection",
> - "Keep-Alive",
> - "ETag",
> - "Content-Location",
> - "Expires",
> - "Cache-Control",
> - "Vary",
> - "Warning",
> - "WWW-Authenticate",
> - "Proxy-Authenticate",
> - NULL);
> - }
> - else {
> - apr_table_do((int (*) (void *, const char *, const char *)) compute_header_len,
> - (void *) &len, r->headers_out, NULL);
> - }
>
> - buff_start = buff = apr_pcalloc(r->pool, len);
> - basic_http_header(r, buff, protocol);
> - buff += strlen(buff);
> + basic_http_header(r, b2, protocol);
>
> h.r = r;
> - h.buf = buff;
> + h.bb = b2;
>
> if (r->status == HTTP_NOT_MODIFIED) {
> apr_table_do((int (*)(void *, const char *, const char *)) form_header_field,
> @@ -2628,13 +2584,10 @@
> (void *) &h, r->headers_out, NULL);
> }
>
> - terminate_header(buff);
> + terminate_header(b2);
>
> r->sent_bodyct = 1; /* Whatever follows is real body stuff... */
>
> - b2 = apr_brigade_create(r->pool);
> - e = apr_bucket_create_pool(buff_start, strlen(buff_start), r->pool);
> - APR_BRIGADE_INSERT_HEAD(b2, e);
> ap_pass_brigade(f->next, b2);
>
> if (r->header_only) {
> @@ -2985,244 +2938,6 @@
> return mm->size; /* XXX - change API to report apr_status_t? */
> }
> #endif /* APR_HAS_MMAP */
> -
> -typedef struct {
> - char *buf;
> - char *cur;
> - apr_size_t avail;
> -} old_write_filter_ctx;
> -
> -AP_CORE_DECLARE_NONSTD(apr_status_t) ap_old_write_filter(
> - ap_filter_t *f, apr_bucket_brigade *bb)
> -{
> - old_write_filter_ctx *ctx = f->ctx;
> -
> - AP_DEBUG_ASSERT(ctx);
> -
> - if (ctx->buf != NULL) {
> - apr_size_t nbyte = ctx->cur - ctx->buf;
> -
> - if (nbyte != 0) {
> - /* whatever is coming down the pipe (we don't care), we
> - can simply insert our buffered data at the front and
> - pass the whole bundle down the chain. */
> - apr_bucket *b = apr_bucket_create_heap(ctx->buf, nbyte, 0, NULL);
> - APR_BRIGADE_INSERT_HEAD(bb, b);
> - ctx->buf = NULL;
> - }
> - }
> -
> - return ap_pass_brigade(f->next, bb);
> -}
> -
> -static apr_status_t flush_buffer(request_rec *r, old_write_filter_ctx *ctx,
> - const char *extra, apr_size_t extra_len)
> -{
> - apr_bucket_brigade *bb = apr_brigade_create(r->pool);
> - apr_size_t nbyte = ctx->cur - ctx->buf;
> - apr_bucket *b = apr_bucket_create_heap(ctx->buf, nbyte, 0, NULL);
> -
> - APR_BRIGADE_INSERT_TAIL(bb, b);
> - ctx->buf = NULL;
> -
> - /* if there is extra data, then send that, too */
> - if (extra != NULL) {
> - b = apr_bucket_create_transient(extra, extra_len);
> - APR_BRIGADE_INSERT_TAIL(bb, b);
> - }
> -
> - return ap_pass_brigade(r->output_filters, bb);
> -}
> -
> -static apr_status_t buffer_output(request_rec *r,
> - const char *str, apr_size_t len)
> -{
> - ap_filter_t *f;
> - old_write_filter_ctx *ctx;
> - apr_size_t amt;
> - apr_status_t status;
> -
> - if (len == 0)
> - return APR_SUCCESS;
> -
> - /* ### future optimization: record some flags in the request_rec to
> - ### say whether we've added our filter, and whether it is first. */
> -
> - /* this will typically exit on the first test */
> - for (f = r->output_filters; f != NULL; f = f->next)
> - if (strcmp("OLD_WRITE", f->frec->name) == 0)
> - break;
> - if (f == NULL) {
> - /* our filter hasn't been added yet */
> - ctx = apr_pcalloc(r->pool, sizeof(*ctx));
> - ap_add_output_filter("OLD_WRITE", ctx, r, r->connection);
> - }
> -
> - /* if the first filter is not our buffering filter, then we have to
> - deliver the content through the normal filter chain */
> - if (strcmp("OLD_WRITE", r->output_filters->frec->name) != 0) {
> - apr_bucket_brigade *bb = apr_brigade_create(r->pool);
> - apr_bucket *b = apr_bucket_create_transient(str, len);
> - APR_BRIGADE_INSERT_TAIL(bb, b);
> -
> - return ap_pass_brigade(r->output_filters, bb);
> - }
> -
> - /* grab the context from our filter */
> - ctx = r->output_filters->ctx;
> -
> - /* if there isn't a buffer in the context yet, put one there. */
> - if (ctx->buf == NULL) {
> - /* use the heap so it will get free'd after being flushed */
> - ctx->avail = AP_MIN_BYTES_TO_WRITE;
> - ctx->buf = ctx->cur = malloc(ctx->avail);
> - }
> -
> - /* squeeze the data into the existing buffer */
> - if (len <= ctx->avail) {
> - memcpy(ctx->cur, str, len);
> - ctx->cur += len;
> - if ((ctx->avail -= len) == 0)
> - return flush_buffer(r, ctx, NULL, 0);
> - return APR_SUCCESS;
> - }
> -
> - /* the new content can't fit in the existing buffer */
> -
> - if (len >= AP_MIN_BYTES_TO_WRITE) {
> - /* it is really big. send what we have, and the new stuff. */
> - return flush_buffer(r, ctx, str, len);
> - }
> -
> - /* the new data is small. put some into the current buffer, flush it,
> - and then drop the remaining into a new buffer. */
> - amt = ctx->avail;
> - memcpy(ctx->cur, str, amt);
> - ctx->cur += amt;
> - ctx->avail = 0;
> - if ((status = flush_buffer(r, ctx, NULL, 0)) != APR_SUCCESS)
> - return status;
> -
> - ctx->buf = malloc(AP_MIN_BYTES_TO_WRITE);
> - memcpy(ctx->buf, str + amt, len - amt);
> - ctx->cur = ctx->buf + (len - amt);
> - ctx->avail = AP_MIN_BYTES_TO_WRITE - (len - amt);
> -
> - return APR_SUCCESS;
> -}
> -
> -AP_DECLARE(int) ap_rputc(int c, request_rec *r)
> -{
> - char c2 = (char)c;
> -
> - if (r->connection->aborted) {
> - return -1;
> - }
> -
> - if (buffer_output(r, &c2, 1) != APR_SUCCESS)
> - return -1;
> -
> - return c;
> -}
> -
> -AP_DECLARE(int) ap_rputs(const char *str, request_rec *r)
> -{
> - apr_size_t len;
> -
> - if (r->connection->aborted)
> - return -1;
> -
> - if (buffer_output(r, str, len = strlen(str)) != APR_SUCCESS)
> - return -1;
> -
> - return len;
> -}
> -
> -AP_DECLARE(int) ap_rwrite(const void *buf, int nbyte, request_rec *r)
> -{
> - if (r->connection->aborted)
> - return -1;
> -
> - if (buffer_output(r, buf, nbyte) != APR_SUCCESS)
> - return -1;
> -
> - return nbyte;
> -}
> -
> -AP_DECLARE(int) ap_vrprintf(request_rec *r, const char *fmt, va_list va)
> -{
> - char buf[4096];
> - apr_size_t written;
> -
> - if (r->connection->aborted)
> - return -1;
> -
> - /* ### fix this mechanism to allow more than 4K of output */
> - written = apr_vsnprintf(buf, sizeof(buf), fmt, va);
> - if (buffer_output(r, buf, written) != APR_SUCCESS)
> - return -1;
> -
> - return written;
> -}
> -
> -AP_DECLARE_NONSTD(int) ap_rprintf(request_rec *r, const char *fmt, ...)
> -{
> - va_list va;
> - int n;
> -
> - if (r->connection->aborted)
> - return -1;
> -
> - va_start(va, fmt);
> - n = ap_vrprintf(r, fmt, va);
> - va_end(va);
> -
> - return n;
> -}
> -
> -AP_DECLARE_NONSTD(int) ap_rvputs(request_rec *r, ...)
> -{
> - va_list va;
> - const char *s;
> - apr_size_t len;
> - apr_size_t written = 0;
> -
> - if (r->connection->aborted)
> - return -1;
> -
> - /* ### TODO: if the total output is large, put all the strings
> - ### into a single brigade, rather than flushing each time we
> - ### fill the buffer */
> - va_start(va, r);
> - while (1) {
> - s = va_arg(va, const char *);
> - if (s == NULL)
> - break;
> -
> - len = strlen(s);
> - if (buffer_output(r, s, len) != APR_SUCCESS) {
> - return -1;
> - }
> -
> - written += len;
> - }
> - va_end(va);
> -
> - return written;
> -}
> -
> -AP_DECLARE(int) ap_rflush(request_rec *r)
> -{
> - apr_bucket_brigade *bb;
> - apr_bucket *b;
> -
> - bb = apr_brigade_create(r->pool);
> - b = apr_bucket_create_flush();
> - APR_BRIGADE_INSERT_TAIL(bb, b);
> - if (ap_pass_brigade(r->output_filters, bb) != APR_SUCCESS)
> - return -1;
> - return 0;
> -}
>
> static const char *add_optional_notes(request_rec *r,
> const char *prefix,
> Index: server/util_filter.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/server/util_filter.c,v
> retrieving revision 1.43
> diff -u -d -b -w -u -r1.43 util_filter.c
> --- server/util_filter.c 2001/01/19 07:04:29 1.43
> +++ server/util_filter.c 2001/02/06 05:19:44
> @@ -265,3 +265,74 @@
> APR_BRIGADE_CONCAT(*saveto, *b);
> return APR_SUCCESS;
> }
> +
> +static apr_status_t brigade_flush(apr_bucket_brigade *bb, void *ctx)
> +{
> + ap_filter_t *f = ctx;
> +
> + return ap_pass_brigade(f, bb);
> +}
> +
> +AP_DECLARE(apr_status_t) ap_fwrite(ap_filter_t *f, apr_bucket_brigade *bb,
> + const char *str, apr_size_t nbyte)
> +{
> + apr_bucket *b;
> + apr_status_t status;
> +
> + if (nbyte == 0)
> + return 0;
> +
> + b = APR_BRIGADE_LAST(bb);
> +
> + bb->flush = brigade_flush;
> + bb->ctx = f->next;
> + status = apr_brigade_write(bb, str, nbyte);
> +
> + /* We are just checking to see if we just converted the
> + * buffer into a new bucket and put it at the end of the brigade. If
> + * we did, we want to pass the brigade to the next filter. If not,
> + * we just keep going. This allows us to use the network to limit how
> + * much data we send at any one time.
> + */
> + if (b != APR_BRIGADE_LAST(bb)) {
> + ap_pass_brigade(f->next, bb);
> + }
> + return status;
> +}
> +
> +AP_DECLARE(apr_status_t) ap_fputs(ap_filter_t *f, apr_bucket_brigade *bb,
> + const char *str)
> +{
> + int j = strlen(str);
> +
> + return ap_fwrite(f, bb, str, j);
> +}
> +
> +AP_DECLARE(apr_status_t) ap_fputc(ap_filter_t *f, apr_bucket_brigade *bb,
> + char str)
> +{
> + return ap_fwrite(f, bb, &str, 1);
> +}
> +
> +AP_DECLARE(apr_status_t) ap_fvputs(ap_filter_t *f, apr_bucket_brigade *bb, ...)
> +{
> + apr_size_t written;
> + va_list va;
> +
> + va_start(va, bb);
> + written = apr_brigade_vputstrs(bb, va);
> + va_end(va);
> + return written;
> +}
> +
> +AP_DECLARE(apr_status_t) ap_fprintf(ap_filter_t *f, apr_bucket_brigade *bb, const char *fmt, ...)
> +{
> +
> + apr_size_t written;
> + va_list va;
> +
> + va_start(va, fmt);
> + written = apr_brigade_vprintf(bb, fmt, va);
> + va_end(va);
> + return written;
> +}
> Index: srclib/apr-util/buckets/apr_brigade.c
> ===================================================================
> RCS file: /home/cvs/apr-util/buckets/apr_brigade.c,v
> retrieving revision 1.3
> diff -u -d -b -w -u -r1.3 apr_brigade.c
> --- srclib/apr-util/buckets/apr_brigade.c 2001/01/19 07:01:59 1.3
> +++ srclib/apr-util/buckets/apr_brigade.c 2001/02/06 05:20:12
> @@ -56,6 +56,7 @@
> #include "apr_lib.h"
> #include "apr_pools.h"
> #include "apr_tables.h"
> +#include "apr_buckets.h"
> #include "apr_errno.h"
>
> #include <stdlib.h>
> @@ -66,8 +67,6 @@
> #include <sys/uio.h>
> #endif
>
> -#include "apr_buckets.h"
> -
> static apr_status_t apr_brigade_cleanup(void *data)
> {
> apr_bucket_brigade *b = data;
> @@ -97,8 +96,9 @@
> {
> apr_bucket_brigade *b;
>
> - b = apr_palloc(p, sizeof(*b));
> + b = apr_pcalloc(p, sizeof(*b));
> b->p = p;
> +
> APR_RING_INIT(&b->list, apr_bucket, link);
>
> apr_register_cleanup(b->p, b, apr_brigade_cleanup, apr_brigade_cleanup);
> @@ -185,12 +185,45 @@
> return vec - orig;
> }
>
> +static int check_brigade_flush(const char **str,
> + apr_size_t *n, apr_bucket_brigade *bb)
> +{
> + apr_bucket *b = APR_BRIGADE_LAST(bb);
> +
> + if (APR_BRIGADE_EMPTY(bb)) {
> + if (*n > APR_BUCKET_BUFF_SIZE) {
> + apr_bucket *e = apr_bucket_create_transient(*str, *n);
> + APR_BRIGADE_INSERT_TAIL(bb, e);
> + return 1;
> + }
> + else {
> + return 0;
> + }
> + }
> +
> + if (APR_BUCKET_IS_HEAP(b)) {
> + apr_bucket_shared *s = b->data;
> + apr_bucket_heap *h = s->data;
> +
> + if (*n > (h->alloc_len - (s->end - s->start))) {
> + apr_bucket *e = apr_bucket_create_transient(*str, *n);
> + APR_BRIGADE_INSERT_TAIL(bb, e);
> + return 1;
> + }
> + }
> + else if (*n > APR_BUCKET_BUFF_SIZE) {
> + apr_bucket *e = apr_bucket_create_transient(*str, *n);
> + APR_BRIGADE_INSERT_TAIL(bb, e);
> + return 1;
> + }
> +
> + return 0;
> +}
> +
> APU_DECLARE(int) apr_brigade_vputstrs(apr_bucket_brigade *b, va_list va)
> {
> - apr_bucket *r;
> const char *x;
> int j, k;
> - apr_size_t i;
>
> for (k = 0;;) {
> x = va_arg(va, const char *);
> @@ -198,20 +231,91 @@
> break;
> j = strlen(x);
>
> - /* XXX: copy or not? let the caller decide? */
> - r = apr_bucket_create_heap(x, j, 1, &i);
> - if (i != j) {
> - /* Do we need better error reporting? */
> - return -1;
> + k += apr_brigade_write(b, x, j);
> }
> - k += i;
> + return k;
> +}
>
> - APR_BRIGADE_INSERT_TAIL(b, r);
> +APU_DECLARE(int) apr_brigade_putc(apr_bucket_brigade *b, const char c)
> +{
> + apr_size_t nbyte = 1;
> + const char *str = &c;
> +
> + if (check_brigade_flush(&str, &nbyte, b)) {
> + if (b->flush) {
> + return b->flush(b, b->ctx);
> + }
> }
> + else {
> + apr_bucket *buck = APR_BRIGADE_LAST(b);
> + apr_bucket_shared *s;
> + apr_bucket_heap *h;
> + char *buf;
>
> - return k;
> + if (!APR_BUCKET_IS_HEAP(buck) || APR_BRIGADE_EMPTY(b)) {
> + buf = malloc(APR_BUCKET_BUFF_SIZE);
> +
> + buck = apr_bucket_create_heap(buf, APR_BUCKET_BUFF_SIZE, 0, NULL);
> + s = buck->data;
> + s->start = s->end = 0;
> + h = s->data;
> +
> + APR_BRIGADE_INSERT_TAIL(b, buck);
> + }
> + else {
> + buf = h->base + s->end;
> +
> + s = buck->data;
> + h = s->data;
> + }
> + memcpy(buf, &c, 1);
> + s->end++;
> + }
> +
> + return 1;
> +}
> +
> +APU_DECLARE(int) apr_brigade_write(apr_bucket_brigade *b, const char *str, apr_size_t nbyte)
> +{
> + if (check_brigade_flush(&str, &nbyte, b)) {
> + if (b->flush) {
> + return b->flush(b, b->ctx);
> + }
> + }
> + else {
> + apr_bucket *buck = APR_BRIGADE_LAST(b);
> + apr_bucket_shared *s;
> + apr_bucket_heap *h;
> + char *buf;
> +
> + if (!APR_BUCKET_IS_HEAP(buck) || APR_BRIGADE_EMPTY(b)) {
> + buf = malloc(APR_BUCKET_BUFF_SIZE);
> +
> + buck = apr_bucket_create_heap(buf, APR_BUCKET_BUFF_SIZE, 0, NULL);
> + s = buck->data;
> + s->start = s->end = 0;
> + h = s->data;
> +
> + APR_BRIGADE_INSERT_TAIL(b, buck);
> + }
> + else {
> + s = buck->data;
> + h = s->data;
> +
> + buf = h->base + s->end;
> }
>
> + memcpy(buf, str, nbyte);
> + s->end += nbyte;
> + }
> + return nbyte;
> +}
> +
> +APU_DECLARE(int) apr_brigade_puts(apr_bucket_brigade *b, const char *str)
> +{
> + return apr_brigade_write(b, str, strlen(str));
> +}
> +
> APU_DECLARE_NONSTD(int) apr_brigade_putstrs(apr_bucket_brigade *b, ...)
> {
> va_list va;
> @@ -240,13 +344,9 @@
> * directly into a bucket. I'm being lazy right now. RBB
> */
> char buf[4096];
> - apr_bucket *r;
> - int res;
>
> - res = apr_vsnprintf(buf, 4096, fmt, va);
> -
> - r = apr_bucket_create_heap(buf, strlen(buf), 1, NULL);
> - APR_BRIGADE_INSERT_TAIL(b, r);
> + apr_vsnprintf(buf, 4096, fmt, va);
>
> - return res;
> + return apr_brigade_puts(b, buf);
> }
> +
> Index: srclib/apr-util/include/apr_buckets.h
> ===================================================================
> RCS file: /home/cvs/apr-util/include/apr_buckets.h,v
> retrieving revision 1.68
> diff -u -d -b -w -u -r1.68 apr_buckets.h
> --- srclib/apr-util/include/apr_buckets.h 2001/01/27 14:52:55 1.68
> +++ srclib/apr-util/include/apr_buckets.h 2001/02/06 05:20:14
> @@ -78,6 +78,8 @@
> * @package Bucket Brigades
> */
>
> +#define APR_BUCKET_BUFF_SIZE 9000
> +
> typedef enum {APR_BLOCK_READ, APR_NONBLOCK_READ} apr_read_type_e;
>
> /*
> @@ -237,6 +239,8 @@
> * the destroying function is responsible for killing the cleanup.
> */
> apr_pool_t *p;
> + void *ctx;
> + apr_status_t (*flush)(apr_bucket_brigade *bb, void *ctx);
> /** The buckets in the brigade are on this list. */
> /*
> * XXX: the apr_bucket_list structure doesn't actually need a name tag
> @@ -533,7 +537,7 @@
> * modified, it is only used to free the bucket.
> */
> char *base;
> - /** how much memory was allocated. This may not be necessary */
> + /** how much memory was allocated */
> size_t alloc_len;
> };
>
> @@ -629,8 +633,7 @@
> struct iovec *vec, int nvec);
>
> /**
> - * This function writes a list of strings into a bucket brigade. We just
> - * allocate a new heap bucket for each string.
> + * This function writes a list of strings into a bucket brigade.
> * @param b The bucket brigade to add to
> * @param va A list of strings to add
> * @return The number of bytes added to the brigade
> @@ -639,8 +642,34 @@
> APU_DECLARE(int) apr_brigade_vputstrs(apr_bucket_brigade *b, va_list va);
>
> /**
> + * This function writes an string into a bucket brigade.
> + * @param b The bucket brigade to add to
> + * @param str The string to add
> + * @return The number of bytes added to the brigade
> + * @deffunc int apr_brigade_write(ap_bucket_brigade *b, const char *str)
> + */
> +APU_DECLARE(int) apr_brigade_write(apr_bucket_brigade *b, const char *str, apr_size_t nbyte);
> +
> +/**
> + * This function writes an string into a bucket brigade.
> + * @param b The bucket brigade to add to
> + * @param str The string to add
> + * @return The number of bytes added to the brigade
> + * @deffunc int apr_brigade_puts(ap_bucket_brigade *b, const char *str)
> + */
> +APU_DECLARE(int) apr_brigade_puts(apr_bucket_brigade *b, const char *str);
> +
> +/**
> + * This function writes a character into a bucket brigade.
> + * @param b The bucket brigade to add to
> + * @param c The character to add
> + * @return The number of bytes added to the brigade
> + * @deffunc int apr_brigade_putc(apr_bucket_brigade *b, const char c)
> + */
> +APU_DECLARE(int) apr_brigade_putc(apr_bucket_brigade *b, const char c);
> +
> +/**
> * This function writes an unspecified number of strings into a bucket brigade.
> - * We just allocate a new heap bucket for each string.
> * @param b The bucket brigade to add to
> * @param ... The strings to add
> * @return The number of bytes added to the brigade
> @@ -649,7 +678,7 @@
> APU_DECLARE_NONSTD(int) apr_brigade_putstrs(apr_bucket_brigade *b, ...);
>
> /**
> - * Evaluate a printf and put the resulting string into a bucket at the end
> + * Evaluate a printf and put the resulting string at the end
> * of the bucket brigade.
> * @param b The brigade to write to
> * @param fmt The format of the string to write
> @@ -661,7 +690,7 @@
> const char *fmt, ...);
>
> /**
> - * Evaluate a printf and put the resulting string into a bucket at the end
> + * Evaluate a printf and put the resulting string at the end
> * of the bucket brigade.
> * @param b The brigade to write to
> * @param fmt The format of the string to write
>
>
> _______________________________________________________________________________
> Ryan Bloom rbb@apache.org
> 406 29th St.
> San Francisco, CA 94131
> -------------------------------------------------------------------------------
>
>
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------