httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject [PATCH] final part of the ap_r* solution
Date Mon, 12 Feb 2001 18:17:03 GMT

This is the final portion of the ap_r* patch.  All this does is remove the
OLD_WRITE filter, and replace the ap_r* calls with macros that use tha
ap_f* calls.  I would really like to finish this solution finally.

The only argument that I have heard against this patch, is that it allows
somebody who is using two different brigades to get the data out of
order.  In order for this to happen, somebody has to use both the ap_r*
calls and the native brigade calls, and they have to use some brigade
other than r->bb.

The other option is to re-write the OLD_WRITE filter to use the ap_f*
calls.  I dislike this option, because it means that we are solving the
same problem in two ways (brigade buffering and OLD_WRITE), and it takes
the control away from the module author.

If we re-write ap_r* to use ap_f*, then the module author has complete
control over when this does and does not work.  It is very easy to
document the one failure condition described above.  The failure in this
case is the data gets out of order.

If we continue to use the OLD_WRITE filter, then the failure condition
becomes that my handler is written correctly, but some other module has
inserted it's filter in front of the OLD_WRITE filter.  This takes control
away from the module author, and puts it in the hands of any other 3rd
party module.  The failure in this case is that the data is sent in a lot
of very small packets

The two failure conditions are different, and it can be argued which is
more important.  Can we please just come to a decision?

Ryan

Index: include/http_protocol.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/http_protocol.h,v
retrieving revision 1.52
diff -u -d -b -w -u -r1.52 http_protocol.h
--- include/http_protocol.h	2001/02/09 07:17:52	1.52
+++ include/http_protocol.h	2001/02/12 17:52:15
@@ -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_fputstrs(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.136
diff -u -d -b -w -u -r1.136 httpd.h
--- include/httpd.h	2001/02/12 15:44:36	1.136
+++ include/httpd.h	2001/02/12 17:52:15
@@ -82,6 +82,7 @@
 #include "apr_pools.h"
 #include "apr_time.h"
 #include "apr_network_io.h"
+#include "apr_buckets.h"
 
 #include "pcreposix.h"
 
@@ -751,6 +752,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: modules/http/http_core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_core.c,v
retrieving revision 1.258
diff -u -d -b -w -u -r1.258 http_core.c
--- modules/http/http_core.c	2001/02/11 00:10:30	1.258
+++ modules/http/http_core.c	2001/02/12 17:52:32
@@ -87,7 +87,6 @@
 
 #include "mod_core.h"
 
-
 /* LimitXMLRequestBody handling */
 #define AP_LIMIT_UNSET                  ((long) -1)
 #define AP_DEFAULT_LIMIT_XML_BODY       ((size_t)1000000)
@@ -3586,8 +3585,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.296
diff -u -d -b -w -u -r1.296 http_protocol.c
--- modules/http/http_protocol.c	2001/02/11 01:09:02	1.296
+++ modules/http/http_protocol.c	2001/02/12 17:52:33
@@ -1580,13 +1580,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_eos_create();
-    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)
@@ -2931,244 +2933,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_heap_create(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_heap_create(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_transient_create(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_transient_create(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_flush_create();
-    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,


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------




Mime
View raw message