Return-Path: 
 <dev-return-1566-apmail-apr-dev-archive=apr.apache.org@apr.apache.org>
Delivered-To: apmail-apr-dev-archive@apr.apache.org
Received: (qmail 79573 invoked by uid 500); 1 Mar 2001 02:20:46 -0000
Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
List-Post: <mailto:dev@apr.apache.org>
List-Help: <mailto:dev-help@apr.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@apr.apache.org>
List-Subscribe: <mailto:dev-subscribe@apr.apache.org>
Delivered-To: mailing list dev@apr.apache.org
Received: (qmail 79558 invoked from network); 1 Mar 2001 02:20:45 -0000
X-Authentication-Warning: node1.unix.Virginia.EDU: jcw5q owned process doing
 -bs
Date: Wed, 28 Feb 2001 21:20:49 -0500 (EST)
From: Cliff Woolley <cliffwoolley@yahoo.com>
X-X-Sender: <jcw5q@node1.unix.Virginia.EDU>
To: Greg Stein <gstein@lyra.org>
cc: dev@apr.apache.org
MMDF-Warning: Parse error in original version of preceding line at
 mail.virginia.edu
Subject: Re: Bucket API cleanup issues
In-Reply-To: <20010228145938.D2297@lyra.org>
Message-ID: <Pine.A41.4.32.0102282111190.53408-100000@node1.unix.Virginia.EDU>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII
X-Spam-Rating: h31.sny.collab.net 1.6.2 0/1000/N

On Wed, 28 Feb 2001, Greg Stein wrote:

> 1) it is hard to tell that "p" and "h" refer to the same bucket.
>    "p->heap.foo" is more obvious than "h->foo".

Fine by me.

> 2) Nit: I'm getting tired of these single letters for the bucket stuff. "a"
>    as a variable for a bucket? "e"? Blarg. Using "h" and "p" fall right into
>    that camp; especially p, given its use as a pool or char pointer in so
>    many other contexts. I might suggest "bh" and "bp".

We've more or less standardized on one letter meaning a bucket, though...
fixing this would be a massive change.  For now, I think we need to stick
with one letter for consistency.  I'd be just as happy if that letter were
'b' (I don't know where a and e came from, but they're definitely there),
or at least a letter mnemonic to the type of bucket it is (as I've done
here).

> 3) You have two "base" members. That will be confusing as hell over the long
>    haul. I'd recommend using just heap.base. Guess how to detect when a pool
>    bucket is no longer in a pool? Right in one! Just set pool to NULL. And
>    *please* put some comments to this effect in the apr_bucket_pool
>    structure declaration(!)

Done.

> 4) And no... I don't think we need to check malloc() results. If NULL is
>    returned, then we'll segfault right away. That's fine in my book since
>    we'd otherwise just call abort(). (if you really want to check, then call
>    abort() on error or a pool's abort function)

Yeah... I just put that comment in because there are comments just like it
throughout the buckets code where there's a possibility of failure that
we're not checking for, in case we decide at some point down the road to
go back and put in checks for them.  <shrug>

> That's it for now. I'll re-review when you check it in.

Thanks.

--Cliff