On Feb 9, 2008, at 12:46 AM, Ruediger Pluem wrote:

Or we keep the old slightly odd names.

I currently see no essential need for these changes to be backportable. So
I would prefer cleaning up the names. But there might be a compromise:

1. Make ap_cache_cacheable_hdrs a macro that is defined as ap_cache_cacheable_hdrs_out
2. Leave ap_cache_cacheable_hdrs_out as is.
3. Create ap_cache_cacheable_headers_out / ap_cache_cacheable_headers_in

This should only require a minor bump.

Bit of grepping yields very few google hits on people using it. And with the #define() we'll have the issue going forward as well. How about below. And we wack the ap_cache_cacheable_hdrs_out()
function on the next major bump.

Dw.

Index: cache_util.c
===================================================================
--- cache_util.c        (revision 620145)
+++ cache_util.c        (working copy)
@@ -571,10 +571,10 @@
     return apr_pstrdup(p, hashfile);
 }

-/* Create a new table consisting of those elements from an input
+/* Create a new table consisting of those elements from an
  * headers table that are allowed to be stored in a cache.
  */
-CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t *pool, +CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers(apr_pool_t *pool, apr_table_t *t,
                                                         server_rec *s)
 {
@@ -582,12 +582,17 @@
     char **header;
     int i;

+    if (t == NULL) {
+       return apr_table_make(pool, 10);
+    };
+
     /* Make a copy of the headers, and remove from
      * the copy any hop-by-hop headers, as defined in Section
      * 13.5.1 of RFC 2616
      */
     apr_table_t *headers_out;
     headers_out = apr_table_copy(pool, t);
+
     apr_table_unset(headers_out, "Connection");
     apr_table_unset(headers_out, "Keep-Alive");
     apr_table_unset(headers_out, "Proxy-Authenticate");
@@ -608,3 +613,46 @@
     }
     return headers_out;
 }
+
+/* Legacy call - functionally equivalent to ap_cache_cacheable_headers.
+ * @warning this call will possibly be removed on the next API bump.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t *pool,
+                                                        apr_table_t *t,
+                                                        server_rec *s)
+{
+    return ap_cache_cacheable_headers(p,t,s);
+}
+
+
+/* Create a new table consisting of those elements from an input
+ * headers table that are allowed to be stored in a cache.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_in(request_rec * r)
+{
+       return ap_cache_cacheable_hdrs(r->pool, r->headers_in, r->server);
+}
+
+/* Create a new table consisting of those elements from an output
+ * headers table that are allowed to be stored in a cache;
+ * ensure there is a content type and capture any errors.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_out(request_rec * r)
+{
+       apr_table_t *headers_out;
+
+       headers_out = ap_cache_cacheable_hdrs(r->pool, r->headers_out,
+                                                  r->server);
+
+        if (!apr_table_get(headers_out, "Content-Type")
+            && r->content_type) {
+            apr_table_setn(headers_out, "Content-Type",
+                           ap_make_content_type(r, r->content_type));
+        }
+
+        headers_out = apr_table_overlay(r->pool, headers_out,
+                                        r->err_headers_out);
+
+       return headers_out;
+}
+
Index: mod_cache.c
===================================================================
--- mod_cache.c (revision 620145)
+++ mod_cache.c (working copy)
@@ -752,10 +752,12 @@
          * However, before doing that, we need to first merge in
          * err_headers_out and we also need to strip any hop-by-hop
          * headers that might have snuck in.
+         *
+        * XX check -- we're not patching up content-type.
          */
         r->headers_out = apr_table_overlay(r->pool, r->headers_out,
                                            r->err_headers_out);
- r->headers_out = ap_cache_cacheable_hdrs_out(r->pool, r- >headers_out, + r->headers_out = ap_cache_cacheable_headers(r->pool, r- >headers_out,
                                                      r->server);
         apr_table_clear(r->err_headers_out);

Index: mod_disk_cache.c
===================================================================
--- mod_disk_cache.c    (revision 620145)
+++ mod_disk_cache.c    (working copy)
@@ -922,17 +922,8 @@
     if (r->headers_out) {
         apr_table_t *headers_out;

- headers_out = ap_cache_cacheable_hdrs_out(r->pool, r- >headers_out,
-                                                  r->server);
+        headers_out = ap_cache_cacheable_headers_out(r);

-        if (!apr_table_get(headers_out, "Content-Type")
-            && r->content_type) {
-            apr_table_setn(headers_out, "Content-Type",
-                           ap_make_content_type(r, r->content_type));
-        }
-
-        headers_out = apr_table_overlay(r->pool, headers_out,
-                                        r->err_headers_out);
         rv = store_table(dobj->hfd, headers_out);
         if (rv != APR_SUCCESS) {
             return rv;
@@ -944,8 +935,8 @@
     if (r->headers_in) {
         apr_table_t *headers_in;

- headers_in = ap_cache_cacheable_hdrs_out(r->pool, r- >headers_in,
-                                                 r->server);
+        headers_in = ap_cache_cacheable_header_in(r);
+
         rv = store_table(dobj->hfd, headers_in);
         if (rv != APR_SUCCESS) {
             return rv;
Index: mod_mem_cache.c
===================================================================
--- mod_mem_cache.c     (revision 620145)
+++ mod_mem_cache.c     (working copy)
@@ -605,17 +605,8 @@
     mobj->req_hdrs = deep_table_copy(mobj->pool, r->headers_in);

     /* Precompute how much storage we need to hold the headers */
-    headers_out = ap_cache_cacheable_hdrs_out(r->pool, r->headers_out,
-                                              r->server);
+    headers_out = ap_cache_cacheable_headers_out(r);

-    /* If not set in headers_out, set Content-Type */
-    if (!apr_table_get(headers_out, "Content-Type")
-        && r->content_type) {
-        apr_table_setn(headers_out, "Content-Type",
-                       ap_make_content_type(r, r->content_type));
-    }
-
- headers_out = apr_table_overlay(r->pool, headers_out, r- >err_headers_out);
     mobj->header_out = deep_table_copy(mobj->pool, headers_out);

     /* Init the info struct */
Index: mod_cache.h
===================================================================
--- mod_cache.h (revision 620145)
+++ mod_cache.h (working copy)
@@ -288,9 +288,27 @@
                                     const char *key, char **val);
CACHE_DECLARE(const char *)ap_cache_tokstr(apr_pool_t *p, const char *list, const char **str);

-/* Create a new table consisting of those elements from a request_rec's
- * headers_out that are allowed to be stored in a cache
+* Create a new table consisting of those elements from an
+ * headers table that are allowed to be stored in a cache.
  */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers(apr_pool_t *pool,
+                                                        apr_table_t *t,
+                                                        server_rec *s);
+
+/* Create a new table consisting of those elements from an input
+ * headers table that are allowed to be stored in a cache.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_in(request_rec * r);
+
+/* Create a new table consisting of those elements from an output
+ * headers table that are allowed to be stored in a cache;
+ * ensure there is a content type and capture any errors.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_out(request_rec * r);
+
+/* Legacy call - functionally equivalent to ap_cache_cacheable_headers.
+ * @warning this call will possibly be removed on the next API bump.
+ */
CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t *pool, apr_table_t *t, server_rec *s);

Reply via email to