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/01/25 22:42:35 UTC
[PATCH] ap_fwrite, ap_fputs
This is a small start, not a full implementation. The general idea is
that apr_brigade_* functions should not be used by programers, they are
for Apache's use. Module writers will use either ap_rwrite or ap_fwrite
and their siblings. I have a few concerns with this patch, but it is the
general idea, the details are very flexible.
1) ap_fwrite is a horrible name, because fwrite has some very specific
meanings. Somebody else can come up with a better name, because I'm not
very good at that. :-)
2) I dislike ap_f* requiring a brigade and a filter. The solution is to
put the brigade in the filter structure itself. This is doable, but I am
not sure if it is a good idea or not.
Also, ap_r* would beome a very small macro like:
#define ap_rwrite(r, str, nbyte) ap_fwrite(r->output_filters, r->bb, \
str, nbyte
Comments, thoughts? I suspect that we will do the following functions to
start with, and add others as needed: ap_fwrite, ap_fputs, ap_fputc,
ap_fprintf, ap_fsendfile, ap_fsend_mmap.
Index: include/util_filter.h
===================================================================
RCS file: /home/cvs/httpd-2.0/include/util_filter.h,v
retrieving revision 1.36
diff -u -d -b -w -u -r1.36 util_filter.h
--- include/util_filter.h 2001/01/22 21:57:56 1.36
+++ include/util_filter.h 2001/01/25 21:03:48
@@ -403,6 +403,13 @@
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);
+
#ifdef __cplusplus
}
#endif
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/01/25 21:04:56
@@ -265,3 +265,43 @@
APR_BRIGADE_CONCAT(*saveto, *b);
return APR_SUCCESS;
}
+
+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);
+
+ status = apr_brigade_write(bb, str, nbyte);
+
+ /* Okay, so this sucks, but we don't have many options. What we
+ * are doing here is 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);
+}
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/01/25 21:07:55
@@ -99,6 +99,9 @@
b = apr_palloc(p, sizeof(*b));
b->p = p;
+
+ b->start = b->end = NULL;
+
APR_RING_INIT(&b->list, apr_bucket, link);
apr_register_cleanup(b->p, b, apr_brigade_cleanup, apr_brigade_cleanup);
@@ -185,12 +188,41 @@
return vec - orig;
}
+static int check_brigade_flush(char *start, char **end, const char *str,
+ apr_size_t *n, apr_bucket_brigade *b)
+{
+ if (*n > (APR_BUCKET_BUFF_SIZE - (*end - start))) {
+ if (start) {
+ memcpy(*end, str, APR_BUCKET_BUFF_SIZE - (*end - start));
+ *n -= (APR_BUCKET_BUFF_SIZE - (*end - start));
+ *end += (APR_BUCKET_BUFF_SIZE - (*end - start));
+ apr_brigade_flush(b);
+ }
+ if (*n > APR_BUCKET_BUFF_SIZE) {
+ apr_bucket *e = apr_bucket_create_transient(str, *n);
+ APR_BRIGADE_INSERT_TAIL(b, e);
+ return 1;
+ }
+ }
+ return 0;
+}
+
+APU_DECLARE(void) apr_brigade_flush(apr_bucket_brigade *b)
+{
+ apr_bucket *e;
+ apr_size_t i = b->end - b->start;
+
+ if (i) {
+ e = apr_bucket_create_heap(b->start, i, 0, NULL);
+ APR_BRIGADE_INSERT_TAIL(b, e);
+ b->start = b->end = NULL;
+ }
+}
+
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 +230,52 @@
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;
+
+ if (check_brigade_flush(b->start, &b->end, &c, &nbyte, b)) {
+ return 1;
}
- return k;
+ if (!b->start) {
+ b->start = b->end = malloc(APR_BUCKET_BUFF_SIZE);
+ }
+
+ memcpy(b->end, &c, 1);
+ b->end++;
+
+ return 1;
+}
+
+APU_DECLARE(int) apr_brigade_write(apr_bucket_brigade *b, const char *str, apr_size_t nbyte)
+{
+ apr_size_t n = nbyte;
+
+ if (check_brigade_flush(b->start, &b->end, str, &nbyte, b)) {
+ return n;
}
+ if (!b->start) {
+ b->start = b->end = malloc(APR_BUCKET_BUFF_SIZE);
+ }
+
+ memcpy(b->end, str, nbyte);
+ b->end += nbyte;
+
+ return n;
+}
+
+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 +304,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.67
diff -u -d -b -w -u -r1.67 apr_buckets.h
--- srclib/apr-util/include/apr_buckets.h 2001/01/23 22:24:06 1.67
+++ srclib/apr-util/include/apr_buckets.h 2001/01/25 21:08:12
@@ -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;
+ char *start;
+ char *end;
/** The buckets in the brigade are on this list. */
/*
* XXX: the apr_bucket_list structure doesn't actually need a name tag
@@ -628,9 +632,10 @@
APU_DECLARE(int) apr_brigade_to_iovec(apr_bucket_brigade *b,
struct iovec *vec, int nvec);
+APU_DECLARE(void) apr_brigade_flush(apr_bucket_brigade *b);
+
/**
- * 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 +644,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 +680,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 +692,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] ap_fwrite, ap_fputs
Posted by rb...@covalent.net.
On Thu, 25 Jan 2001, Greg Marr wrote:
> At 06:20 PM 01/25/2001, rbb@covalent.net wrote:
> > > Why not build this test into the status code, or into another
> > > parameter? The apr_brigade_* functions know when the buffer was
> > > converted into a bucket, so why not let them tell you?
> >
> >Too much information to pass back. I actually tried that at one
> >point, and couldn't find a way to do it that I really liked. In
> >reality, this test is relatively clean.
>
> It was the "this sucks" 6 line comment before this check in every
> function that prompted the search for another option. :)
I know. That was more a ping to myself to remember to look into this
again, and possibly to just move that into a macro. :-)
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] ap_fwrite, ap_fputs
Posted by Greg Marr <gr...@alum.wpi.edu>.
At 06:20 PM 01/25/2001, rbb@covalent.net wrote:
> > Why not build this test into the status code, or into another
> > parameter? The apr_brigade_* functions know when the buffer was
> > converted into a bucket, so why not let them tell you?
>
>Too much information to pass back. I actually tried that at one
>point, and couldn't find a way to do it that I really liked. In
>reality, this test is relatively clean.
It was the "this sucks" 6 line comment before this check in every
function that prompted the search for another option. :)
--
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"
Re: [PATCH] ap_fwrite, ap_fputs
Posted by rb...@covalent.net.
> >+
> >+ /* Okay, so this sucks, but we don't have many options. What we
> >+ * are doing here is 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);
> >+ }
>
> Why not build this test into the status code, or into another
> parameter? The apr_brigade_* functions know when the buffer was
> converted into a bucket, so why not let them tell you?
Too much information to pass back. I actually tried that at one point,
and couldn't find a way to do it that I really liked. In reality, this
test is relatively clean.
I may take another stab at getting the check function to report if it
flushed the brigade or not, but not right away.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: [PATCH] ap_fwrite, ap_fputs
Posted by Greg Marr <gr...@alum.wpi.edu>.
At 04:42 PM 01/25/2001, rbb@covalent.net wrote:
>+ status = apr_brigade_write(bb, str, nbyte);
>+
>+ /* Okay, so this sucks, but we don't have many options. What we
>+ * are doing here is 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);
>+ }
Why not build this test into the status code, or into another
parameter? The apr_brigade_* functions know when the buffer was
converted into a bucket, so why not let them tell you?
--
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"