[PATCH] buckets: appending to a bucket_brigade (untested)

[Cliff Woolley <JW...@wlu.edu>]

Ryan:

    This implements ap_bucket_brigade_append_list() and
ap_bucket_brigade_append_bucket().  Those names are awfully long, but
they follow your scheme.  ap_brigade_append_list() and
ap_brigade_append_bucket() might be better names.  These are untested
functions, but I figured it'd be easy enough for you to check them out.

    Also, I notice that there are a bunch of memory leaks when you
destroy various things... often you destroy the data without destroying
the struct that points to it.  ap_bucket_brigade_destroy() seems to call
the list destruction function without freeing the brigade struct itself.
 ap_destroy_bucket_list()  [should that be called
ap_bucket_list_destroy() ???] destroys the bucket pointed to by each
list node, but not the node structs themselves, etc.  Possibly other
situations like this exist as well... I'll try and check.

    Another instance which I just realized I didn't fix in this patch is
down in ap_brigade_vputstrs() where you do  r = ap_bucket_new(...) .  I
think that should be below your if (x==NULL) break; statement so that
you don't create a bucket and then lose track of it if there's nothing
to put in it.

   PS: part of this patch is context diff instead of unidiff because
that actually makes it easier to tell what's going on in this case...
hope that doesn't cause you merging headaches.

--Cliff

Cliff Woolley
Central Systems Software Administrator
Washington and Lee University
http://www.wlu.edu/~jwoolley/

Work: (540) 463-8089
Pager: (540) 462-2303

Re: [PATCH] buckets: appending to a bucket_brigade (untested)

[rb...@covalent.net]

Cliff, GREAT!!!!!!  Thanks for the patch.  I'm unlikely to review and
apply this tonight, but I should get to it tomorrow.

I had a vague idea that there were memory leaks, but I haven't really
looked into them yet.  I want to get the basic code working to prove the
design, and then go back and clean it all up.

Ryan

On Wed, 12 Jul 2000, Cliff Woolley wrote:

> Ryan:
> 
>     This implements ap_bucket_brigade_append_list() and
> ap_bucket_brigade_append_bucket().  Those names are awfully long, but
> they follow your scheme.  ap_brigade_append_list() and
> ap_brigade_append_bucket() might be better names.  These are untested
> functions, but I figured it'd be easy enough for you to check them out.
> 
>     Also, I notice that there are a bunch of memory leaks when you
> destroy various things... often you destroy the data without destroying
> the struct that points to it.  ap_bucket_brigade_destroy() seems to call
> the list destruction function without freeing the brigade struct itself.
>  ap_destroy_bucket_list()  [should that be called
> ap_bucket_list_destroy() ???] destroys the bucket pointed to by each
> list node, but not the node structs themselves, etc.  Possibly other
> situations like this exist as well... I'll try and check.
> 
>     Another instance which I just realized I didn't fix in this patch is
> down in ap_brigade_vputstrs() where you do  r = ap_bucket_new(...) .  I
> think that should be below your if (x==NULL) break; statement so that
> you don't create a bucket and then lose track of it if there's nothing
> to put in it.
> 
>    PS: part of this patch is context diff instead of unidiff because
> that actually makes it easier to tell what's going on in this case...
> hope that doesn't cause you merging headaches.
> 
> --Cliff
> 
> Cliff Woolley
> Central Systems Software Administrator
> Washington and Lee University
> http://www.wlu.edu/~jwoolley/
> 
> Work: (540) 463-8089
> Pager: (540) 462-2303
> 


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------