You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2015/10/07 00:23:24 UTC
svn commit: r1707159 - /httpd/httpd/trunk/server/eor_bucket.c
Author: ylavic
Date: Tue Oct 6 22:23:24 2015
New Revision: 1707159
URL: http://svn.apache.org/viewvc?rev=1707159&view=rev
Log:
eor_bucket: follow up to r1707084: use an inner shared bucket.
Modified:
httpd/httpd/trunk/server/eor_bucket.c
Modified: httpd/httpd/trunk/server/eor_bucket.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/eor_bucket.c?rev=1707159&r1=1707158&r2=1707159&view=diff
==============================================================================
--- httpd/httpd/trunk/server/eor_bucket.c (original)
+++ httpd/httpd/trunk/server/eor_bucket.c Tue Oct 6 22:23:24 2015
@@ -21,15 +21,15 @@
static apr_status_t eor_bucket_cleanup(void *data)
{
- apr_bucket *b = (apr_bucket *)data;
- request_rec *r = (request_rec *)b->data;
+ request_rec **rp = data;
- if (r != NULL) {
+ if (*rp) {
+ request_rec *r = *rp;
/*
* If eor_bucket_destroy is called after us, this prevents
* eor_bucket_destroy from trying to destroy the pool again.
*/
- b->data = NULL;
+ *rp = NULL;
/* Update child status and log the transaction */
ap_update_child_status(r->connection->sbh, SERVER_BUSY_LOG, r);
ap_run_log_transaction(r);
@@ -50,11 +50,13 @@ static apr_status_t eor_bucket_read(apr_
AP_DECLARE(apr_bucket *) ap_bucket_eor_make(apr_bucket *b, request_rec *r)
{
- b->length = 0;
- b->start = 0;
- b->data = r;
- b->type = &ap_bucket_type_eor;
+ apr_bucket *h;
+ h = apr_bucket_alloc(sizeof(*h), b->list);
+ h->data = r;
+
+ b = apr_bucket_shared_make(b, h, 0, 0);
+ b->type = &ap_bucket_type_eor;
return b;
}
@@ -66,7 +68,9 @@ AP_DECLARE(apr_bucket *) ap_bucket_eor_c
APR_BUCKET_INIT(b);
b->free = apr_bucket_free;
b->list = list;
+ b = ap_bucket_eor_make(b, r);
if (r) {
+ apr_bucket *h = b->data;
/*
* Register a cleanup for the request pool as the eor bucket could
* have been allocated from a different pool then the request pool
@@ -76,38 +80,31 @@ AP_DECLARE(apr_bucket *) ap_bucket_eor_c
* We need to use a pre-cleanup here because a module may create a
* sub-pool which is still needed during the log_transaction hook.
*/
- apr_pool_pre_cleanup_register(r->pool, (void *)b, eor_bucket_cleanup);
+ apr_pool_pre_cleanup_register(r->pool, &h->data, eor_bucket_cleanup);
}
- return ap_bucket_eor_make(b, r);
+ return b;
}
static void eor_bucket_destroy(void *data)
{
- request_rec *r = (request_rec *)data;
+ apr_bucket *h = data;
- if (r) {
- /* eor_bucket_cleanup will be called when the pool gets destroyed */
- apr_pool_destroy(r->pool);
+ if (apr_bucket_shared_destroy(h)) {
+ request_rec *r = h->data;
+ if (r) {
+ /* eor_bucket_cleanup will be called when the pool gets destroyed */
+ apr_pool_destroy(r->pool);
+ }
+ apr_bucket_free(h);
}
}
-static apr_status_t eor_bucket_copy(apr_bucket *a, apr_bucket **b)
-{
- *b = apr_bucket_alloc(sizeof(**b), a->list); /* XXX: check for failure? */
- **b = *a;
-
- /* We don't want the request to be destroyed more than once. */
- (*b)->data = NULL;
-
- return APR_SUCCESS;
-}
-
AP_DECLARE_DATA const apr_bucket_type_t ap_bucket_type_eor = {
"EOR", 5, APR_BUCKET_METADATA,
eor_bucket_destroy,
eor_bucket_read,
apr_bucket_setaside_noop,
apr_bucket_split_notimpl,
- eor_bucket_copy
+ apr_bucket_shared_copy
};
Re: svn commit: r1707159 - /httpd/httpd/trunk/server/eor_bucket.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Oct 7, 2015 at 9:10 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Wed, Oct 7, 2015 at 8:45 PM, Ruediger Pluem <rp...@apache.org> wrote:
>>
>>
>> On 10/07/2015 12:23 AM, ylavic@apache.org wrote:
>>>
>>> static void eor_bucket_destroy(void *data)
>>> {
>>> - request_rec *r = (request_rec *)data;
>>> + apr_bucket *h = data;
>>
>> IMHO this should be
>>
>> apr_bucket_refcount *h;
>
> Good catch!
>
> Actually, we even need:
>
> typedef struct {
> apr_bucket_refcount refcount;
> request_rec *data;
> } apr_bucket_eor;
>
> and use this type everywhere for h.
Commited in r1707362.
>
> Thanks,
> Yann.
Re: svn commit: r1707159 - /httpd/httpd/trunk/server/eor_bucket.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Oct 7, 2015 at 8:45 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
>
> On 10/07/2015 12:23 AM, ylavic@apache.org wrote:
>>
>> static void eor_bucket_destroy(void *data)
>> {
>> - request_rec *r = (request_rec *)data;
>> + apr_bucket *h = data;
>
> IMHO this should be
>
> apr_bucket_refcount *h;
Good catch!
Actually, we even need:
typedef struct {
apr_bucket_refcount refcount;
request_rec *data;
} apr_bucket_eor;
and use this type everywhere for h.
Thanks,
Yann.
Re: svn commit: r1707159 - /httpd/httpd/trunk/server/eor_bucket.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10/07/2015 12:23 AM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Tue Oct 6 22:23:24 2015
> New Revision: 1707159
>
> URL: http://svn.apache.org/viewvc?rev=1707159&view=rev
> Log:
> eor_bucket: follow up to r1707084: use an inner shared bucket.
>
> Modified:
> httpd/httpd/trunk/server/eor_bucket.c
>
> Modified: httpd/httpd/trunk/server/eor_bucket.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/eor_bucket.c?rev=1707159&r1=1707158&r2=1707159&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/eor_bucket.c (original)
> +++ httpd/httpd/trunk/server/eor_bucket.c Tue Oct 6 22:23:24 2015
> @@ -21,15 +21,15 @@
>
> static apr_status_t eor_bucket_cleanup(void *data)
> {
> - apr_bucket *b = (apr_bucket *)data;
> - request_rec *r = (request_rec *)b->data;
> + request_rec **rp = data;
>
> - if (r != NULL) {
> + if (*rp) {
> + request_rec *r = *rp;
> /*
> * If eor_bucket_destroy is called after us, this prevents
> * eor_bucket_destroy from trying to destroy the pool again.
> */
> - b->data = NULL;
> + *rp = NULL;
> /* Update child status and log the transaction */
> ap_update_child_status(r->connection->sbh, SERVER_BUSY_LOG, r);
> ap_run_log_transaction(r);
> @@ -50,11 +50,13 @@ static apr_status_t eor_bucket_read(apr_
>
> AP_DECLARE(apr_bucket *) ap_bucket_eor_make(apr_bucket *b, request_rec *r)
> {
> - b->length = 0;
> - b->start = 0;
> - b->data = r;
> - b->type = &ap_bucket_type_eor;
> + apr_bucket *h;
IMHO this should be
apr_bucket_refcount *h;
>
> + h = apr_bucket_alloc(sizeof(*h), b->list);
> + h->data = r;
> +
> + b = apr_bucket_shared_make(b, h, 0, 0);
> + b->type = &ap_bucket_type_eor;
> return b;
> }
>
> @@ -66,7 +68,9 @@ AP_DECLARE(apr_bucket *) ap_bucket_eor_c
> APR_BUCKET_INIT(b);
> b->free = apr_bucket_free;
> b->list = list;
> + b = ap_bucket_eor_make(b, r);
> if (r) {
> + apr_bucket *h = b->data;
> /*
> * Register a cleanup for the request pool as the eor bucket could
> * have been allocated from a different pool then the request pool
> @@ -76,38 +80,31 @@ AP_DECLARE(apr_bucket *) ap_bucket_eor_c
> * We need to use a pre-cleanup here because a module may create a
> * sub-pool which is still needed during the log_transaction hook.
> */
> - apr_pool_pre_cleanup_register(r->pool, (void *)b, eor_bucket_cleanup);
> + apr_pool_pre_cleanup_register(r->pool, &h->data, eor_bucket_cleanup);
> }
> - return ap_bucket_eor_make(b, r);
> + return b;
> }
>
> static void eor_bucket_destroy(void *data)
> {
> - request_rec *r = (request_rec *)data;
> + apr_bucket *h = data;
IMHO this should be
apr_bucket_refcount *h;
Regards
Rüdiger