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 Schaefer <jo...@sunstarsys.com> on 2005/03/14 16:25:54 UTC
[PATCH] Fix apr_table_overlap when pools aren't the same
Here's a semi-tested patch to fix apr_table_overlap when
the pools differ. The current code wipes out the first
argument's table when the pools differ, which is clearly
inconsistent with the documented behavior (my fault probably,
because I think this bug is due to an old patch of mine).
I say "semi-tested" because the patch I wanted to submit
would change all the POOL_DEBUG ifdefs in apr_tables.c
to APR_POOL_DEBUG, but when I tested that against httpd's trunk
there were lots of core files being generated. When I tried to
write a patch for httpd, I noticed that apr_pool_join doesn't
work (not implemented by apr), so at the moment I'm not sure
if apr has abandoned the goal of being strict about pool ancenstry
in the table ops.
Anyways, here's the patch. I hope it helps.
Index: tables/apr_tables.c
===================================================================
--- tables/apr_tables.c (revision 157131)
+++ tables/apr_tables.c (working copy)
@@ -1190,16 +1190,20 @@
{
const int m = a->a.nelts;
const int n = b->a.nelts;
- apr_pool_t *p = b->a.pool;
if (m + n == 0) {
return;
}
- /* copy (extend) a using b's pool */
- if (a->a.pool != p) {
- make_array_core(&a->a, p, m+n, sizeof(apr_table_entry_t), 0);
+#ifdef POOL_DEBUG
+ /* we don't copy keys and values, so it's necessary that b->a.pool
+ * have a life span at least as long as a->a.pool
+ */
+ if (!apr_pool_is_ancestor(b->a.pool, a->a.pool)) {
+ fprintf(stderr, "overlap_table: b's pool is not an ancestor of a's\n");
+ abort();
}
+#endif
apr_table_cat(a, b);
Index: test/testtable.c
===================================================================
--- test/testtable.c (revision 157131)
+++ test/testtable.c (working copy)
@@ -117,9 +117,14 @@
static void table_overlap(abts_case *tc, void *data)
{
const char *val;
- apr_table_t *t1 = apr_table_make(p, 1);
- apr_table_t *t2 = apr_table_make(p, 1);
+ apr_pool_t *subp;
+ apr_table_t *t1;
+ apr_table_t *t2;
+ apr_pool_create(&subp, p);
+ t1 = apr_table_make(subp, 1);
+ t2 = apr_table_make(p, 1);
+
apr_table_addn(t1, "a", "0");
apr_table_addn(t1, "g", "7");
apr_table_addn(t2, "a", "1");
--
Joe Schaefer
Re: [PATCH] Fix apr_table_overlap when pools aren't the same
Posted by Joe Orton <jo...@redhat.com>.
On Wed, May 04, 2005 at 10:21:30AM -0400, Joe Schaefer wrote:
> Joe Orton <jo...@redhat.com> writes:
>
> [...]
>
> > After filling in the gaps in the pools code it does seem to work OK.
> > Do you see any problems with the trunk?
>
> IIRC there were tons of segfaults during a run of the perl-framework
> tests when I compiled everything with --enable-pool-debug. But I
> didn't have a working apr_pool_join implementation at the time, so
> now I'll have to go back and check (unless you've already done that :-).
Yup, I did that much at least.
joe
Re: [PATCH] Fix apr_table_overlap when pools aren't the same
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Orton <jo...@redhat.com> writes:
[...]
> After filling in the gaps in the pools code it does seem to work OK.
> Do you see any problems with the trunk?
IIRC there were tons of segfaults during a run of the perl-framework
tests when I compiled everything with --enable-pool-debug. But I
didn't have a working apr_pool_join implementation at the time, so
now I'll have to go back and check (unless you've already done that :-).
--
Joe Schaefer
Re: [PATCH] Fix apr_table_overlap when pools aren't the same
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Mar 14, 2005 at 10:25:54AM -0500, Joe Schaefer wrote:
> Here's a semi-tested patch to fix apr_table_overlap when
> the pools differ. The current code wipes out the first
> argument's table when the pools differ, which is clearly
> inconsistent with the documented behavior (my fault probably,
> because I think this bug is due to an old patch of mine).
Thanks a lot, I've committed this.
> I say "semi-tested" because the patch I wanted to submit
> would change all the POOL_DEBUG ifdefs in apr_tables.c
> to APR_POOL_DEBUG, but when I tested that against httpd's trunk
> there were lots of core files being generated. When I tried to
> write a patch for httpd, I noticed that apr_pool_join doesn't
> work (not implemented by apr), so at the moment I'm not sure
> if apr has abandoned the goal of being strict about pool ancenstry
> in the table ops.
After filling in the gaps in the pools code it does seem to work OK. Do
you see any problems with the trunk?
Regards,
joe