Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Subject: Re: [PATCH] Update to Brian's patch to allocate brigades out of
	the bucket allocator
From: Brian Pane <brian.pane@cnet.com>
To: dev@httpd.apache.org
Cc: APR Development List <dev@apr.apache.org>
In-Reply-To: <JHEPKCEMGPKFFDHHDDKDOENAGGAA.bill@wstoddard.com>
References: <JHEPKCEMGPKFFDHHDDKDOENAGGAA.bill@wstoddard.com>
Content-Type: text/plain
Organization: 
Message-Id: <1040409251.1205.28.camel@desktop>
Mime-Version: 1.0
Date: 20 Dec 2002 10:34:11 -0800
Content-Transfer-Encoding: 7bit

On Fri, 2002-12-20 at 07:32, Bill Stoddard wrote:
> If a pool is passed to apr_brigade_create, the brigade is allocated out of the
> pool. If the pool is NULL, the brigade is allocated out of the bucket allocator.
> We don't want a pool pointer hanging around in a brigade allocated out of the
> bucket allocator. That's just asking for trouble.  This patch makes how the
> brigade is allocated, either out of the pool or out of the allocator, explicit.

+1

I'm not instinctively worried about having a pool pointer in
a structure allocated out of the bucket allocator.  We do the
same thing safely in other contexts--mmap buckets, for example--
by being careful about coordinating the bucket vs. pool cleanup
steps.  But Bill's approach is safe and conservative, and it
will work even with existing code that re-uses destroyed brigades.

Brian

> 
> Index: apr_brigade.c
> ===================================================================
> RCS file: /home/cvs/apr-util/buckets/apr_brigade.c,v
> retrieving revision 1.55
> diff -u -r1.55 apr_brigade.c
> --- apr_brigade.c	17 Dec 2002 19:16:39 -0000	1.55
> +++ apr_brigade.c	20 Dec 2002 15:17:58 -0000
> @@ -95,7 +95,9 @@
>          apr_pool_cleanup_kill(b->p, b, brigade_cleanup);
>      }
>      rv = apr_brigade_cleanup(b);
> -    apr_bucket_free(b);
> +    if (!b->p) {
> +        apr_bucket_free(b);
> +    }
>      return rv;
>  }
> 
> @@ -104,7 +106,12 @@
>  {
>      apr_bucket_brigade *b;
> 
> -    b = apr_bucket_alloc(sizeof(*b), list);
> +    if (p) {
> +        b = apr_palloc(p, sizeof(*b));
> +    }
> +    else {
> +        b = apr_bucket_alloc(sizeof(*b), list);
> +    }
>      b->p = p;
>      b->bucket_alloc = list;
>