From dev-return-9999-apmail-apr-dev-archive=apr.apache.org@apr.apache.org Tue Jun 24 07:28:19 2003 Return-Path: Delivered-To: apmail-apr-dev-archive@apr.apache.org Received: (qmail 3700 invoked by uid 500); 24 Jun 2003 07:28:18 -0000 Mailing-List: contact dev-help@apr.apache.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Delivered-To: mailing list dev@apr.apache.org Received: (qmail 3680 invoked from network); 24 Jun 2003 07:28:18 -0000 Subject: Re: cvs commit: apr/tables apr_tables.c From: Brian Pane To: dev@apr.apache.org In-Reply-To: <3EF6EC52.1020106@attglobal.net> References: <20030622215026.13619.qmail@icarus.apache.org> <3EF6EC52.1020106@attglobal.net> Content-Type: text/plain Organization: Message-Id: <1056439902.7004.41.camel@desktop> Mime-Version: 1.0 X-Mailer: Ximian Evolution 1.2.2 (1.2.2-5) Date: 24 Jun 2003 00:31:42 -0700 Content-Transfer-Encoding: 7bit X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N 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: /** *
 * 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