You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Sander Striker <st...@samba-tng.org> on 2001/06/08 03:01:04 UTC
RE: cvs commit: apr/memory/unix apr_sms.c apr_sms_std.c apr_sms_tracking.c
> dreid 01/06/07 17:44:52
>
> Modified: include apr_sms.h
> memory/unix apr_sms.c apr_sms_std.c apr_sms_tracking.c
> Log:
> Some tidying up of the locking code.
> Move where we allocate the pool from and clean it up.
> Change the pre_destroy to be an apr_status_t
Yes, I should clarify that. We currently don't care what it returns,
but we might later on so it seemed ok to change that (now that we
only have a few sms implementations).
> Move the lock to the individual structures and add a lock_destroy in
> the pre_destroy function.
> Some changing of parameter names to make them shorter.
Oh, yes, I actually want to do s/mem_sys/sms/ aswell, but that can
be done later...
Sander
RE: cvs commit: apr/memory/unix apr_sms.c apr_sms_std.c apr_sms_tracking.c
Posted by Sander Striker <st...@samba-tng.org>.
[..]
> > Yes, but you are missing something here. You would have to create
> > a list header to nest in the apr_sms_t structure. With the ref
> > concept you don't need that, because it can point to parent->child.
>
> I'm not sure that I follow. We have a complete set of
> navigational pointers:
> parent, child, prev, next. With those, I'm not sure what you mean
> by a "list
> header".
>
> Insertion as a child of "this" is:
>
> new->parent = this;
> new->next = this->child;
> if (new->next != NULL)
> new->next->prev = new;
> this->child = new;
>
> Removal is:
>
> if (this->prev != NULL)
> this->prev->next = this->next;
> if (this->next != NULL)
> this->next->prev = this->prev;
> if (parent->child == this)
> parent->child = this->next;
>
> Is it that you are you saying that the third condition of the removal is
> unnecessary with the "ref" scheme?
Yes, that's exactly what I'm saying.
> > And although for removing from a list it is almost the same
> > number of operations, for inserting into a list you have more.
> > Anyhow, same idea, different approach.
>
> If you don't mind, could you elaborate? I'm confused on what you
> mean here.
Since we are always inserting at the head of the list it has less
of an impact. Didn't occur to me before, but does now.
> > Since the only place this
> > is used is inside the sms framework code could we please leave it
> > be for now? I'd like to focus on some other stuff first :-)
>
> You can focus on whatever you'd like. While you're doing that, we'll sneak
> in and change the names, make tweaks to the code, etc. Remember: the code
> can be attacked by more than one person :-)
Ack. I know that. There are multiple people working on sms actually :-)
Go ahead on the name change. I think noone likes extra typing :-)
Sander
Re: cvs commit: apr/memory/unix apr_sms.c apr_sms_std.c apr_sms_tracking.c
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Jun 08, 2001 at 11:09:39AM +0200, Sander Striker wrote:
> Greg Stein wrote:
> > On Fri, Jun 08, 2001 at 12:32:04AM -0400, Cliff Woolley wrote:
>...
> > > "sms->parent" would be ideal, but then again "sms->ref" wouldn't
> > > make much sense
> >
> > They should be:
> >
> > ->parent
> > ->next_sibling (->next and ->prev might be okay, but I like _sibling on
> > ->prev_sibling there for clarification that this is a tree)
> > ->child
> >
> > The "ref" concept is wrong. What is really happening is a standard
> > doubly-linked list. However, it adds an indirection and different
> > terminology. As a result, a new reader must crawl through a bunch of code
> > just to say "oh! a doubly-linked list!"
>
> To say it is wrong is a bit blunt don't you agree? I've used this scheme
> many times before and it has never failed me. Lets call it different...
Yes (sorry), and sure...
> > Screw that. Just make it a true doubly-linked list. Keep everybody happy.
> >
> > Removal is simply:
> >
> > if (sms->prev_sibling != NULL)
> > sms->prev_sibling->next_sibling = sms->next_sibling;
> > if (sms->next_sibling != NULL)
> > sms->next_sibling->prev_sibling = sms->prev_sibling;
> >
> > Done. Nothing fancy or complicated, and everybody will know what
> > is going on upon first glance.
>
> Yes, but you are missing something here. You would have to create
> a list header to nest in the apr_sms_t structure. With the ref
> concept you don't need that, because it can point to parent->child.
I'm not sure that I follow. We have a complete set of navigational pointers:
parent, child, prev, next. With those, I'm not sure what you mean by a "list
header".
Insertion as a child of "this" is:
new->parent = this;
new->next = this->child;
if (new->next != NULL)
new->next->prev = new;
this->child = new;
Removal is:
if (this->prev != NULL)
this->prev->next = this->next;
if (this->next != NULL)
this->next->prev = this->prev;
if (parent->child == this)
parent->child = this->next;
Is it that you are you saying that the third condition of the removal is
unnecessary with the "ref" scheme?
> And although for removing from a list it is almost the same
> number of operations, for inserting into a list you have more.
> Anyhow, same idea, different approach.
If you don't mind, could you elaborate? I'm confused on what you mean here.
> Since the only place this
> is used is inside the sms framework code could we please leave it
> be for now? I'd like to focus on some other stuff first :-)
You can focus on whatever you'd like. While you're doing that, we'll sneak
in and change the names, make tweaks to the code, etc. Remember: the code
can be attacked by more than one person :-)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
RE: cvs commit: apr/memory/unix apr_sms.c apr_sms_std.c apr_sms_tracking.c
Posted by Sander Striker <st...@samba-tng.org>.
> On Fri, Jun 08, 2001 at 12:32:04AM -0400, Cliff Woolley wrote:
> > On Fri, 8 Jun 2001, Sander Striker wrote:
> >
> > > Oh, yes, I actually want to do s/mem_sys/sms/ aswell, but that can
> >
> > I'm hoping that includes the foo_mem_sys's that are in the apr_sms_t
> > structure? "sms->parent_mem_sys" etc seems both redunant and too much
> > typing.
>
> Exactly. Completely agree.
>
> > "sms->parent" would be ideal, but then again "sms->ref" wouldn't
> > make much sense
>
> They should be:
>
> ->parent
> ->next_sibling (->next and ->prev might be okay, but I like _sibling on
> ->prev_sibling there for clarification that this is a tree)
> ->child
>
> The "ref" concept is wrong. What is really happening is a standard
> doubly-linked list. However, it adds an indirection and different
> terminology. As a result, a new reader must crawl through a bunch of code
> just to say "oh! a doubly-linked list!"
To say it is wrong is a bit blunt don't you agree? I've used this scheme
many times before and it has never failed me. Lets call it different...
> Screw that. Just make it a true doubly-linked list. Keep everybody happy.
>
> Removal is simply:
>
> if (sms->prev_sibling != NULL)
> sms->prev_sibling->next_sibling = sms->next_sibling;
> if (sms->next_sibling != NULL)
> sms->next_sibling->prev_sibling = sms->prev_sibling;
>
> Done. Nothing fancy or complicated, and everybody will know what
> is going on upon first glance.
Yes, but you are missing something here. You would have to create
a list header to nest in the apr_sms_t structure. With the ref
concept you don't need that, because it can point to parent->child.
And although for removing from a list it is almost the same
number of operations, for inserting into a list you have more.
Anyhow, same idea, different approach. Since the only place this
is used is inside the sms framework code could we please leave it
be for now? I'd like to focus on some other stuff first :-)
About the renaming:
parent_mem_sys -> parent
child_mem_sys -> child
sibling_mem_sys -> sibling
ref_mem_sys -> ref /* since ref_mem_sys was a confusing name
* anyway. Add a comment for now to
explain
* what it does.
*/
accounting_mem_sys -> acct
Sander
Re: cvs commit: apr/memory/unix apr_sms.c apr_sms_std.c apr_sms_tracking.c
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Jun 08, 2001 at 12:32:04AM -0400, Cliff Woolley wrote:
> On Fri, 8 Jun 2001, Sander Striker wrote:
>
> > Oh, yes, I actually want to do s/mem_sys/sms/ aswell, but that can
>
> I'm hoping that includes the foo_mem_sys's that are in the apr_sms_t
> structure? "sms->parent_mem_sys" etc seems both redunant and too much
> typing.
Exactly. Completely agree.
> "sms->parent" would be ideal, but then again "sms->ref" wouldn't
> make much sense
They should be:
->parent
->next_sibling (->next and ->prev might be okay, but I like _sibling on
->prev_sibling there for clarification that this is a tree)
->child
The "ref" concept is wrong. What is really happening is a standard
doubly-linked list. However, it adds an indirection and different
terminology. As a result, a new reader must crawl through a bunch of code
just to say "oh! a doubly-linked list!"
Screw that. Just make it a true doubly-linked list. Keep everybody happy.
Removal is simply:
if (sms->prev_sibling != NULL)
sms->prev_sibling->next_sibling = sms->next_sibling;
if (sms->next_sibling != NULL)
sms->next_sibling->prev_sibling = sms->prev_sibling;
Done. Nothing fancy or complicated, and everybody will know what is going on
upon first glance.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
RE: cvs commit: apr/memory/unix apr_sms.c apr_sms_std.c apr_sms_tracking.c
Posted by Cliff Woolley <cl...@yahoo.com>.
On Fri, 8 Jun 2001, Sander Striker wrote:
> Oh, yes, I actually want to do s/mem_sys/sms/ aswell, but that can
I'm hoping that includes the foo_mem_sys's that are in the apr_sms_t
structure? "sms->parent_mem_sys" etc seems both redunant and too much
typing. "sms->parent" would be ideal, but then again "sms->ref" wouldn't
make much sense, so for consistency I could live with "sms->parent_sms".
=-)
PS: If this post makes no sense, please tell me.
--Cliff
--------------------------------------------------------------
Cliff Woolley
cliffwoolley@yahoo.com
Charlottesville, VA