Index: CHANGES =================================================================== --- CHANGES (revision 484588) +++ CHANGES (working copy) @@ -1,6 +1,11 @@ -*- coding: utf-8 -*- Changes with Apache 2.2.4 + *) mod_mem_cache: Convert mod_mem_cache to use APR memory pool functions + by creating a root pool for object persistence across requests. This + also eliminates the need for custom serialization code. + [Davi Arnaut ] + *) mod_cgi and mod_cgid: Don't use apr_status_t error return from input filters as HTTP return value from the handler. PR#31579. Index: modules/cache/mod_mem_cache.c =================================================================== --- modules/cache/mod_mem_cache.c (revision 484588) +++ modules/cache/mod_mem_cache.c (working copy) @@ -58,17 +58,11 @@ CACHE_TYPE_MMAP } cache_type_e; -typedef struct { - char* hdr; - char* val; -} cache_header_tbl_t; - typedef struct mem_cache_object { + apr_pool_t *pool; cache_type_e type; - apr_ssize_t num_header_out; - apr_ssize_t num_req_hdrs; - cache_header_tbl_t *header_out; - cache_header_tbl_t *req_hdrs; /* for Vary negotiation */ + apr_table_t *header_out; + apr_table_t *req_hdrs; /* for Vary negotiation */ apr_size_t m_len; void *m; apr_os_file_t fd; @@ -210,19 +204,6 @@ { mem_cache_object_t *mobj = obj->vobj; - /* TODO: - * We desperately need a more efficient way of allocating objects. We're - * making way too many malloc calls to create a fully populated - * cache object... - */ - - /* Cleanup the cache_object_t */ - if (obj->key) { - free((void*)obj->key); - } - - free(obj); - /* Cleanup the mem_cache_object_t */ if (mobj) { if (mobj->type == CACHE_TYPE_HEAP && mobj->m) { @@ -235,18 +216,9 @@ close(mobj->fd); #endif } - if (mobj->header_out) { - if (mobj->header_out[0].hdr) - free(mobj->header_out[0].hdr); - free(mobj->header_out); - } - if (mobj->req_hdrs) { - if (mobj->req_hdrs[0].hdr) - free(mobj->req_hdrs[0].hdr); - free(mobj->req_hdrs); - } - free(mobj); } + + apr_pool_destroy(mobj->pool); } static apr_status_t decrement_refcount(void *arg) { @@ -334,9 +306,10 @@ static int create_entity(cache_handle_t *h, cache_type_e type_e, request_rec *r, const char *key, apr_off_t len) { + apr_status_t rv; + apr_pool_t *pool; cache_object_t *obj, *tmp_obj; mem_cache_object_t *mobj; - apr_size_t key_len; if (len == -1) { /* Caching a streaming response. Assume the response is @@ -370,25 +343,21 @@ } } - /* Allocate and initialize cache_object_t */ - obj = calloc(1, sizeof(*obj)); - if (!obj) { + rv = apr_pool_create(&pool, NULL); + + if (rv != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_WARNING, rv, r->server, + "mem_cache: Failed to create memory pool."); return DECLINED; } - key_len = strlen(key) + 1; - obj->key = malloc(key_len); - if (!obj->key) { - cleanup_cache_object(obj); - return DECLINED; - } - memcpy((void*)obj->key, key, key_len); + /* Allocate and initialize cache_object_t */ + obj = apr_pcalloc(pool, sizeof(*obj)); + obj->key = apr_pstrdup(pool, key); + /* Allocate and init mem_cache_object_t */ - mobj = calloc(1, sizeof(*mobj)); - if (!mobj) { - cleanup_cache_object(obj); - return DECLINED; - } + mobj = apr_pcalloc(pool, sizeof(*mobj)); + mobj->pool = pool; /* Finish initing the cache object */ apr_atomic_set32(&obj->refcount, 1); @@ -539,64 +508,7 @@ return OK; } -static apr_status_t serialize_table(cache_header_tbl_t **obj, - apr_ssize_t *nelts, - apr_table_t *table) -{ - const apr_array_header_t *elts_arr = apr_table_elts(table); - apr_table_entry_t *elts = (apr_table_entry_t *) elts_arr->elts; - apr_ssize_t i; - apr_size_t len = 0; - apr_size_t idx = 0; - char *buf; - *nelts = elts_arr->nelts; - if (*nelts == 0 ) { - *obj=NULL; - return APR_SUCCESS; - } - *obj = malloc(sizeof(cache_header_tbl_t) * elts_arr->nelts); - if (NULL == *obj) { - return APR_ENOMEM; - } - for (i = 0; i < elts_arr->nelts; ++i) { - len += strlen(elts[i].key); - len += strlen(elts[i].val); - len += 2; /* Extra space for NULL string terminator for key and val */ - } - - /* Transfer the headers into a contiguous memory block */ - buf = malloc(len); - if (!buf) { - *obj = NULL; - return APR_ENOMEM; - } - - for (i = 0; i < *nelts; ++i) { - (*obj)[i].hdr = &buf[idx]; - len = strlen(elts[i].key) + 1; /* Include NULL terminator */ - memcpy(&buf[idx], elts[i].key, len); - idx+=len; - - (*obj)[i].val = &buf[idx]; - len = strlen(elts[i].val) + 1; - memcpy(&buf[idx], elts[i].val, len); - idx+=len; - } - return APR_SUCCESS; -} -static int unserialize_table( cache_header_tbl_t *ctbl, - int num_headers, - apr_table_t *t ) -{ - int i; - - for (i = 0; i < num_headers; ++i) { - apr_table_addn(t, ctbl[i].hdr, ctbl[i].val); - } - - return APR_SUCCESS; -} /* Define request processing hook handlers */ /* remove_url() * Notes: @@ -629,17 +541,12 @@ static apr_status_t recall_headers(cache_handle_t *h, request_rec *r) { - int rc; mem_cache_object_t *mobj = (mem_cache_object_t*) h->cache_obj->vobj; - h->req_hdrs = apr_table_make(r->pool, mobj->num_req_hdrs); - h->resp_hdrs = apr_table_make(r->pool, mobj->num_header_out); + h->req_hdrs = apr_table_copy(r->pool, mobj->req_hdrs); + h->resp_hdrs = apr_table_copy(r->pool, mobj->header_out); - rc = unserialize_table(mobj->req_hdrs, mobj->num_req_hdrs, h->req_hdrs); - rc = unserialize_table(mobj->header_out, mobj->num_header_out, - h->resp_hdrs); - - return rc; + return OK; } static apr_status_t recall_body(cache_handle_t *h, apr_pool_t *p, apr_bucket_brigade *bb) @@ -669,7 +576,6 @@ { cache_object_t *obj = h->cache_obj; mem_cache_object_t *mobj = (mem_cache_object_t*) obj->vobj; - int rc; apr_table_t *headers_out; /* @@ -679,12 +585,7 @@ * - The original response headers (for returning with a cached response) * - The body of the message */ - rc = serialize_table(&mobj->req_hdrs, - &mobj->num_req_hdrs, - r->headers_in); - if (rc != APR_SUCCESS) { - return rc; - } + mobj->req_hdrs = apr_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, @@ -698,13 +599,8 @@ } headers_out = apr_table_overlay(r->pool, headers_out, r->err_headers_out); + mobj->header_out = apr_table_copy(mobj->pool, headers_out); - rc = serialize_table(&mobj->header_out, &mobj->num_header_out, - headers_out); - if (rc != APR_SUCCESS) { - return rc; - } - /* Init the info struct */ obj->info.status = info->status; if (info->date) { @@ -812,13 +708,10 @@ /* Caching a streamed response. Reallocate a buffer of the * correct size and copy the streamed response into that * buffer */ - char *buf = malloc(obj->count); - if (!buf) { + mobj->m = realloc(mobj->m, obj->count); + if (!mobj->m) { return APR_ENOMEM; } - memcpy(buf, mobj->m, obj->count); - free(mobj->m); - mobj->m = buf; /* Now comes the crufty part... there is no way to tell the * cache that the size of the object has changed. We need