Remove the returned-length "w" parameter from apr_bucket_heap_create()

and apr_bucket_heap_make().  It was useless, since the length is invariant
from the length passed in and from the resulting bucket's b->length.  This
takes care of a long-standing issue first brought up in February and
discussed on the dev@apr list.  (Issue #2 from the "Bucket API Cleanup
Issues" thread.)

See http://marc.theaimsgroup.com/?l=apr-dev&m=98324983126666&w=2

Reviewed by:    Ryan Bloom (concept)

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@91866 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/modules/experimental/mod_case_filter_in.c b/modules/experimental/mod_case_filter_in.c
index 0975145606f0604ce855c9e1fd0c19eac20849b8..1a8ceb19968cea5359b050c4132f35c4bfbeb82d 100644 (file)
--- a/modules/experimental/mod_case_filter_in.c
+++ b/modules/experimental/mod_case_filter_in.c
@@ -147,7 +147,7 @@ static apr_status_t CaseFilterInFilter(ap_filter_t *f,
        for(n=0 ; n < len ; ++n)
            buf[n]=toupper(data[n]);
 
-       pbktOut=apr_bucket_heap_create(buf,len,0,NULL);
+       pbktOut = apr_bucket_heap_create(buf, len, 0);
        APR_BRIGADE_INSERT_TAIL(pbbOut,pbktOut);
        apr_bucket_delete(pbktIn);
     }

diff --git a/modules/experimental/mod_charset_lite.c b/modules/experimental/mod_charset_lite.c
index e4ea92de69747cb743ee4eac91bdefefed98ff1e..fa3b46083300b074319c51a107d6616575fee2ba 100644 (file)
--- a/modules/experimental/mod_charset_lite.c
+++ b/modules/experimental/mod_charset_lite.c
@@ -91,6 +91,7 @@
 
 /* XXX this works around an issue with the heap bucket: apr_bucket_heap_create will 
  *     copy only the first 4096 bytes
+ * XXX: this comment is just plain wrong, or at least it is now. --jcw 11/01
  */
 #undef INPUT_XLATE_BUF_SIZE         /* XXX */
 #define INPUT_XLATE_BUF_SIZE (4096) /* XXX must match DEFAULT_BUCKET_SIZE */
@@ -1070,8 +1071,7 @@ static int xlate_in_filter(ap_filter_t *f, apr_bucket_brigade *bb,
             apr_bucket *e;
 
             e = apr_bucket_heap_create(ctx->tmp, 
-                                      INPUT_XLATE_BUF_SIZE - buffer_size, 1, 
-                                      NULL);
+                                      INPUT_XLATE_BUF_SIZE - buffer_size, 1);
             /* make sure we insert at the head, because there may be
              * an eos bucket already there, and the eos bucket should 
              * come after the data

diff --git a/modules/filters/mod_include.c b/modules/filters/mod_include.c
index 051671d2735a908cee43ba428b44e2b0b9d1b1d0..368fb2dbd3d31dc0a42e2a96e43504ef4abb918e 100644 (file)
--- a/modules/filters/mod_include.c
+++ b/modules/filters/mod_include.c
@@ -1154,7 +1154,7 @@ static int handle_echo(include_ctx_t *ctx, apr_bucket_brigade **bb,
     char       *tag_val   = NULL;
     const char *echo_text = NULL;
     apr_bucket  *tmp_buck;
-    apr_size_t e_len, e_wrt;
+    apr_size_t e_len;
     enum {E_NONE, E_URL, E_ENTITY} encode;
 
     encode = E_ENTITY;
@@ -1187,8 +1187,8 @@ static int handle_echo(include_ctx_t *ctx, apr_bucket_brigade **bb,
                     }
 
                     e_len = strlen(echo_text);
-                    tmp_buck = apr_bucket_heap_create(echo_text, e_len, 1, 
-                                                      &e_wrt);
+                    /* XXX: it'd probably be nice to use a pool bucket here */
+                    tmp_buck = apr_bucket_heap_create(echo_text, e_len, 1);
                 }
                 else {
                     tmp_buck = apr_bucket_immortal_create("(none)", 
@@ -1382,7 +1382,7 @@ static int handle_fsize(include_ctx_t *ctx, apr_bucket_brigade **bb,
     char *tag     = NULL;
     char *tag_val = NULL;
     apr_finfo_t  finfo;
-    apr_size_t  s_len, s_wrt;
+    apr_size_t  s_len;
     apr_bucket   *tmp_buck;
     char parsed_string[MAX_STRING_LEN];
 
@@ -1402,6 +1402,11 @@ static int handle_fsize(include_ctx_t *ctx, apr_bucket_brigade **bb,
                 ap_ssi_parse_string(r, tag_val, parsed_string, 
                                     sizeof(parsed_string), 0);
                 if (!find_file(r, "fsize", tag, parsed_string, &finfo)) {
+                    /* XXX: if we *know* we're going to have to copy the
+                     * thing off of the stack anyway, why not palloc buff
+                     * instead of sticking it on the stack; then we can just
+                     * use a pool bucket and skip the copy
+                     */
                     char buff[50];
 
                     if (!(ctx->flags & FLAG_SIZE_IN_BYTES)) {
@@ -1425,7 +1430,7 @@ static int handle_fsize(include_ctx_t *ctx, apr_bucket_brigade **bb,
                         s_len = pos;
                     }
 
-                    tmp_buck = apr_bucket_heap_create(buff, s_len, 1, &s_wrt);
+                    tmp_buck = apr_bucket_heap_create(buff, s_len, 1);
                     APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
                     if (*inserted_head == NULL) {
                         *inserted_head = tmp_buck;
@@ -1448,7 +1453,7 @@ static int handle_flastmod(include_ctx_t *ctx, apr_bucket_brigade **bb,
     char *tag     = NULL;
     char *tag_val = NULL;
     apr_finfo_t  finfo;
-    apr_size_t  t_len, t_wrt;
+    apr_size_t  t_len;
     apr_bucket   *tmp_buck;
     char parsed_string[MAX_STRING_LEN];
 
@@ -1473,7 +1478,10 @@ static int handle_flastmod(include_ctx_t *ctx, apr_bucket_brigade **bb,
                     t_val = ap_ht_time(r->pool, finfo.mtime, ctx->time_str, 0);
                     t_len = strlen(t_val);
 
-                    tmp_buck = apr_bucket_heap_create(t_val, t_len, 1, &t_wrt);
+                    /* XXX: t_val was already pstrdup'ed into r->pool by
+                     * ap_ht_time. no sense copying it again to the heap.
+                     * should just use a pool bucket */
+                    tmp_buck = apr_bucket_heap_create(t_val, t_len, 1);
                     APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
                     if (*inserted_head == NULL) {
                         *inserted_head = tmp_buck;
@@ -2290,14 +2298,13 @@ static int parse_expr(request_rec *r, const char *expr, int *was_error,
 #define LOG_COND_STATUS(cntx, t_buck, h_ptr, ins_head, tag_text)           \
 {                                                                          \
     char *cond_txt = "**** X     conditional_status=\"0\"\n";              \
-    apr_size_t c_wrt;                                                      \
                                                                            \
     if (cntx->flags & FLAG_COND_TRUE) {                                    \
         cond_txt[31] = '1';                                                \
     }                                                                      \
     memcpy(&cond_txt[5], tag_text, sizeof(tag_text));                      \
-    t_buck = apr_bucket_heap_create(cond_txt, sizeof(cond_txt), 1, &c_wrt); \
-    APR_BUCKET_INSERT_BEFORE(h_ptr, t_buck);                                \
+    t_buck = apr_bucket_heap_create(cond_txt, sizeof(cond_txt), 1);        \
+    APR_BUCKET_INSERT_BEFORE(h_ptr, t_buck);                               \
                                                                            \
     if (ins_head == NULL) {                                                \
         ins_head = t_buck;                                                 \
@@ -2305,10 +2312,9 @@ static int parse_expr(request_rec *r, const char *expr, int *was_error,
 }
 #define DUMP_PARSE_EXPR_DEBUG(t_buck, h_ptr, d_buf, ins_head)            \
 {                                                                        \
-    apr_size_t b_wrt;                                                    \
     if (d_buf[0] != '\0') {                                              \
-        t_buck = apr_bucket_heap_create(d_buf, strlen(d_buf), 1, &b_wrt); \
-        APR_BUCKET_INSERT_BEFORE(h_ptr, t_buck);                          \
+        t_buck = apr_bucket_heap_create(d_buf, strlen(d_buf), 1);        \
+        APR_BUCKET_INSERT_BEFORE(h_ptr, t_buck);                         \
                                                                          \
         if (ins_head == NULL) {                                          \
             ins_head = t_buck;                                           \
@@ -2381,10 +2387,9 @@ static int handle_if(include_ctx_t *ctx, apr_bucket_brigade **bb,
                 expr = tag_val;
 #ifdef DEBUG_INCLUDE
                 if (1) {
-                    apr_size_t d_len = 0, d_wrt = 0;
+                    apr_size_t d_len = 0;
                     d_len = sprintf(debug_buf, "**** if expr=\"%s\"\n", expr);
-                    tmp_buck = apr_bucket_heap_create(debug_buf, d_len, 1, 
-                                                      &d_wrt);
+                    tmp_buck = apr_bucket_heap_create(debug_buf, d_len, 1);
                     APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
 
                     if (*inserted_head == NULL) {
@@ -2464,10 +2469,9 @@ static int handle_elif(include_ctx_t *ctx, apr_bucket_brigade **bb,
                 expr = tag_val;
 #ifdef DEBUG_INCLUDE
                 if (1) {
-                    apr_size_t d_len = 0, d_wrt = 0;
+                    apr_size_t d_len = 0;
                     d_len = sprintf(debug_buf, "**** elif expr=\"%s\"\n", expr);
-                    tmp_buck = apr_bucket_heap_create(debug_buf, d_len, 1, 
-                                                      &d_wrt);
+                    tmp_buck = apr_bucket_heap_create(debug_buf, d_len, 1);
                     APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
 
                     if (*inserted_head == NULL) {
@@ -2613,7 +2617,7 @@ static int handle_printenv(include_ctx_t *ctx, apr_bucket_brigade **bb,
             const apr_table_entry_t *elts = (const apr_table_entry_t *)arr->elts;
             int i;
             const char *key_text, *val_text;
-            apr_size_t   k_len, v_len, t_wrt;
+            apr_size_t   k_len, v_len;
 
             *inserted_head = NULL;
             for (i = 0; i < arr->nelts; ++i) {
@@ -2626,8 +2630,15 @@ static int handle_printenv(include_ctx_t *ctx, apr_bucket_brigade **bb,
                 k_len = strlen(key_text);
                 v_len = strlen(val_text);
 
+                /* XXX: this isn't a very efficient way to do this.  Buckets
+                 * aren't optimized for single-byte allocations.  All of
+                 * this stuff is getting copied anyway, so it'd be better to
+                 * pstrcat them into a single pool buffer and use a single
+                 * pool bucket.  Less alloc calls, easier to send out to the
+                 * network.
+                 */
                 /*  Key_text                                               */
-                tmp_buck = apr_bucket_heap_create(key_text, k_len, 1, &t_wrt);
+                tmp_buck = apr_bucket_heap_create(key_text, k_len, 1);
                 APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
                 if (*inserted_head == NULL) {
                     *inserted_head = tmp_buck;
@@ -2636,7 +2647,7 @@ static int handle_printenv(include_ctx_t *ctx, apr_bucket_brigade **bb,
                 tmp_buck = apr_bucket_immortal_create("=", 1);
                 APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
                 /*              Value_text                                 */
-                tmp_buck = apr_bucket_heap_create(val_text, v_len, 1, &t_wrt);
+                tmp_buck = apr_bucket_heap_create(val_text, v_len, 1);
                 APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
                 /*                        newline...                       */
                 tmp_buck = apr_bucket_immortal_create("\n", 1);

diff --git a/modules/filters/mod_include.h b/modules/filters/mod_include.h
index 2105e1c3ab826ed3d4cc28cae4e692ccb583a35e..190b61ba1505ba7c58edb0dfccea9d86dec1ccf4 100644 (file)
--- a/modules/filters/mod_include.h
+++ b/modules/filters/mod_include.h
@@ -179,9 +179,9 @@ typedef struct include_filter_ctx {
 
 #define CREATE_ERROR_BUCKET(cntx, t_buck, h_ptr, ins_head)        \
 {                                                                 \
-    apr_size_t e_wrt;                                             \
+    /* XXX: it'd probably be nice to use a pool bucket here */    \
     t_buck = apr_bucket_heap_create(cntx->error_str,              \
-                             strlen(cntx->error_str), 1, &e_wrt); \
+                             strlen(cntx->error_str), 1);         \
     APR_BUCKET_INSERT_BEFORE(h_ptr, t_buck);                      \
                                                                   \
     if (ins_head == NULL) {                                       \

diff --git a/modules/generators/mod_cgi.c b/modules/generators/mod_cgi.c
index f5344187da22cd64b468c61f04eb428e251559b1..9dbb950c2ef4ffed75ae831655439f8a07c3ffb7 100644 (file)
--- a/modules/generators/mod_cgi.c
+++ b/modules/generators/mod_cgi.c
@@ -771,20 +771,26 @@ static int include_cgi(char *s, request_rec *r, ap_filter_t *next,
 
     rr_status = ap_run_sub_req(rr);
     if (ap_is_HTTP_REDIRECT(rr_status)) {
-        apr_size_t len_loc, h_wrt;
+        apr_size_t len_loc;
         const char *location = apr_table_get(rr->headers_out, "Location");
 
         location = ap_escape_html(rr->pool, location);
         len_loc = strlen(location);
 
+        /* XXX: if most of this stuff is going to get copied anyway,
+         * it'd be more efficient to pstrcat it into a single pool buffer
+         * and a single pool bucket */
+
         tmp_buck = apr_bucket_immortal_create("<A HREF=\"", sizeof("<A HREF=\""));
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
-        tmp2_buck = apr_bucket_heap_create(location, len_loc, 1, &h_wrt);
+        tmp2_buck = apr_bucket_heap_create(location, len_loc, 1);
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
+        /* XXX: this looks like a bug: should be sizeof - 1 */
         tmp2_buck = apr_bucket_immortal_create("\">", sizeof("\">"));
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
-        tmp2_buck = apr_bucket_heap_create(location, len_loc, 1, &h_wrt);
+        tmp2_buck = apr_bucket_heap_create(location, len_loc, 1);
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
+        /* XXX: this looks like a bug: should be sizeof - 1 */
         tmp2_buck = apr_bucket_immortal_create("</A>", sizeof("</A>"));
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
 

diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c
index f5e34a81f4c2f96ad0bb83f2e1639da28cefd320..00c7fbb609ea1e48db64b9d1d6dac6b568597ab3 100644 (file)
--- a/modules/generators/mod_cgid.c
+++ b/modules/generators/mod_cgid.c
@@ -1092,20 +1092,26 @@ static int include_cgi(char *s, request_rec *r, ap_filter_t *next,
 
     rr_status = ap_run_sub_req(rr);
     if (ap_is_HTTP_REDIRECT(rr_status)) {
-        apr_size_t len_loc, h_wrt;
+        apr_size_t len_loc;
         const char *location = apr_table_get(rr->headers_out, "Location");
 
         location = ap_escape_html(rr->pool, location);
         len_loc = strlen(location);
 
+        /* XXX: if most of this stuff is going to get copied anyway,
+         * it'd be more efficient to pstrcat it into a single pool buffer
+         * and a single pool bucket */
+
         tmp_buck = apr_bucket_immortal_create("<A HREF=\"", sizeof("<A HREF=\""));
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp_buck);
-        tmp2_buck = apr_bucket_heap_create(location, len_loc, 1, &h_wrt);
+        tmp2_buck = apr_bucket_heap_create(location, len_loc, 1);
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
+        /* XXX: this looks like a bug: should be sizeof - 1 */
         tmp2_buck = apr_bucket_immortal_create("\">", sizeof("\">"));
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
-        tmp2_buck = apr_bucket_heap_create(location, len_loc, 1, &h_wrt);
+        tmp2_buck = apr_bucket_heap_create(location, len_loc, 1);
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
+        /* XXX: this looks like a bug: should be sizeof - 1 */
         tmp2_buck = apr_bucket_immortal_create("</A>", sizeof("</A>"));
         APR_BUCKET_INSERT_BEFORE(head_ptr, tmp2_buck);
 

diff --git a/modules/proxy/proxy_http.c b/modules/proxy/proxy_http.c
index cc67421d356e666d657a34cc35c83b58100f484e..40659e6fbe510e84131189851247ed0a78e7c385 100644 (file)
--- a/modules/proxy/proxy_http.c
+++ b/modules/proxy/proxy_http.c
@@ -782,7 +782,7 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
         /* Is it an HTTP/0.9 response? If so, send the extra data */
         if (backasswards) {
             apr_ssize_t cntr = len;
-            e = apr_bucket_heap_create(buffer, cntr, 0, NULL);
+            e = apr_bucket_heap_create(buffer, cntr, 0);
             APR_BRIGADE_INSERT_TAIL(bb, e);
         }