You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Joe Orton <jo...@redhat.com> on 2003/10/29 22:52:46 UTC
[PATCH] simplify apr_pools.c a little
I wrote this when tracking down that psprintf bug, I think it makes the
code a little clearer than trying to follow the pointer juggling
everywhere. Any objections to committing it? (object code is
byte-for-byte identical before and after)
apr_pools.c | 63 ++++++++++++++++++++++++------------------------------------
1 files changed, 26 insertions(+), 37 deletions(-)
Index: memory/unix/apr_pools.c
===================================================================
RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
retrieving revision 1.200
diff -u -r1.200 apr_pools.c
--- memory/unix/apr_pools.c 28 Sep 2003 16:43:44 -0000 1.200
+++ memory/unix/apr_pools.c 29 Oct 2003 21:48:53 -0000
@@ -615,6 +615,21 @@
}
#endif /* defined(NETWARE) */
+/* Node list management helper macros; list_insert() inserts 'node'
+ * before 'point'. */
+#define list_insert(node, point) do { \
+ node->ref = point->ref; \
+ *node->ref = node; \
+ node->next = point; \
+ point->ref = &node->next; \
+} while (0)
+
+/* list_remove() removes 'node' from its list. */
+#define list_remove(node) do { \
+ *node->ref = node->next; \
+ node->next->ref = node->ref; \
+} while (0)
+
/*
* Memory allocation
*/
@@ -638,8 +653,7 @@
node = active->next;
if (size < (apr_size_t)(node->endp - node->first_avail)) {
- *node->ref = node->next;
- node->next->ref = node->ref;
+ list_remove(node);
}
else {
if ((node = allocator_alloc(pool->allocator, size)) == NULL) {
@@ -655,10 +669,7 @@
mem = node->first_avail;
node->first_avail += size;
- node->ref = active->ref;
- *node->ref = node;
- node->next = active;
- active->ref = &node->next;
+ list_insert(node, active);
pool->active = node;
@@ -675,13 +686,8 @@
}
while (free_index < node->free_index);
- *active->ref = active->next;
- active->next->ref = active->ref;
-
- active->ref = node->ref;
- *active->ref = active;
- active->next = node;
- node->ref = &active->next;
+ list_remove(active);
+ list_insert(active, node);
return mem;
}
@@ -944,13 +950,9 @@
node = active->next;
if (!ps->got_a_new_node
&& size < (apr_size_t)(node->endp - node->first_avail)) {
- *node->ref = node->next;
- node->next->ref = node->ref;
- node->ref = active->ref;
- *node->ref = node;
- node->next = active;
- active->ref = &node->next;
+ list_remove(node);
+ list_insert(node, active);
node->free_index = 0;
@@ -967,13 +969,8 @@
}
while (free_index < node->free_index);
- *active->ref = active->next;
- active->next->ref = active->ref;
-
- active->ref = node->ref;
- *active->ref = active;
- active->next = node;
- node->ref = &active->next;
+ list_remove(active);
+ list_insert(active, node);
}
node = pool->active;
@@ -1058,10 +1055,7 @@
node->free_index = 0;
- node->ref = active->ref;
- *node->ref = node;
- node->next = active;
- active->ref = &node->next;
+ list_insert(node, active);
pool->active = node;
@@ -1079,13 +1073,8 @@
}
while (free_index < node->free_index);
- *active->ref = active->next;
- active->next->ref = active->ref;
-
- active->ref = node->ref;
- *active->ref = active;
- active->next = node;
- node->ref = &active->next;
+ list_remove(active);
+ list_insert(active, node);
return strp;
}
RE: [PATCH] simplify apr_pools.c a little
Posted by Sander Striker <st...@apache.org>.
> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: Thursday, October 30, 2003 11:32 AM
> On Wed, Oct 29, 2003 at 09:52:46PM +0000, Joe Orton wrote:
> > I wrote this when tracking down that psprintf bug, I think it makes the
> > code a little clearer than trying to follow the pointer juggling
> > everywhere. Any objections to committing it? (object code is
> > byte-for-byte identical before and after)
>
> +1. The ref stuff is totally opaque.
+1 for the simplification. I should have used macros like that in the
first place. There, committed. Thanks Joe.
> I really don't understand the benefit of the double-indirection and the
> resulting complexity.
There are less checks to do. Since memory allocation is one
of the core operations (in that there are so many of them), eliminating
cycles counts (IMO). Ofcourse we'd need to do some serious profiling
to see if there is really speedup in the double-indirection. For now,
with the new macros, the code should be readable enough not to warrant
rewrite with 'clearer', more verbose, list logic.
Sander
Re: [PATCH] simplify apr_pools.c a little
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Oct 29, 2003 at 09:52:46PM +0000, Joe Orton wrote:
> I wrote this when tracking down that psprintf bug, I think it makes the
> code a little clearer than trying to follow the pointer juggling
> everywhere. Any objections to committing it? (object code is
> byte-for-byte identical before and after)
+1. The ref stuff is totally opaque. I really don't understand the benefit
of the double-indirection and the resulting complexity.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] simplify apr_pools.c a little
Posted by Cliff Woolley <jw...@virginia.edu>.
On Wed, 29 Oct 2003, Joe Orton wrote:
> I wrote this when tracking down that psprintf bug, I think it makes the
> code a little clearer than trying to follow the pointer juggling
> everywhere. Any objections to committing it? (object code is
> byte-for-byte identical before and after)
I like it... +1. Seems to give a little bit of a "future proof" feel to
it, also.
--Cliff