apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <brian.p...@cnet.com>
Subject Re: cvs commit: apr/tables apr_tables.c
Date Tue, 24 Jun 2003 07:31:42 GMT
On Mon, 2003-06-23 at 05:02, Jeff Trawick wrote:
> brianp@apache.org wrote:
> > brianp      2003/06/22 14:50:25
> > 
> >   Modified:    include  apr_tables.h
> >                tables   apr_tables.c
> >   Log:
> >   Add an apr_table_compress() function to reconcile duplicate
> >   entries in a table, and replace the red-black trees in
> >   apr_table_overlap() with a less complex mergesort
> 
> If it were just the warnings I'd handle them, but there is some deeper 
> problem.  try apr/test/testall -v testtable

There were two problems showing up in testtable.  One
was a legitimate bug in the new table_compress code;
I've committed a fix for that. The other was a bug
in the test case.  Here's the state of things just
before the apr_table_overlap() call in testtable:

Breakpoint 1, table_overlap (tc=0x80c36a0) at testtable.c:171
171         apr_table_overlap(t1, t2, APR_OVERLAP_TABLES_SET);
(gdb) dump_table(t1)
[0] 'a'='0'
[1] 'g'='7'
(gdb) dump_table(t2)
[0] 'a'='1'
[1] 'b'='2'
[2] 'c'='3'
[3] 'b'='2.0'
[4] 'd'='4'
[5] 'e'='5'
[6] 'b'='2.'
[7] 'f'='6'
(gdb) next
173         CuAssertIntEquals(tc, apr_table_elts(t1)->nelts, 7);
(gdb) dump_table(t1)
[0] 'a'='1'
[1] 'g'='7'
[2] 'b'='2.'
[3] 'c'='3'
[4] 'd'='4'
[5] 'e'='5'
[6] 'f'='6'

Note that the new table code is doing the right thing here;
the value of 'b' is '2.'--the value of the last occurrence
of that key.

That's consistent with the documentation in the header
file:

/**
 *<PRE>
 * Conceptually, apr_table_overlap does this:
 *
 *  apr_array_header_t *barr = apr_table_elts(b);
 *  apr_table_entry_t *belt = (apr_table_entry_t *)barr->elts;
 *  int i;
 *
 *  for (i = 0; i < barr->nelts; ++i) {
 *      if (flags & APR_OVERLAP_TABLES_MERGE) {
 *          apr_table_mergen(a, belt[i].key, belt[i].val);
 *      }
 *      else {
 *          apr_table_setn(a, belt[i].key, belt[i].val);
 *      }
 *  }
 *
 *  Except that it is more efficient (less space and cpu-time)
especially
 *  when b has many elements.


The testtable code complains because it expects the value of
'b' to be '2' after the overlap.  The test only passed because
the bug in the test case matched a bug in the old table overlap
code.

I'm committing a fix for testtable now.

Brian



Mime
View raw message