You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@attglobal.net> on 2003/06/23 14:02:26 UTC
Re: cvs commit: apr/tables apr_tables.c
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
/home/trawick/regressapr/oldlogs/aprmake.stderr and
/home/trawick/regressapr/logs/aprmake.stderr are different!
------------------/home/trawick/regressapr/logs/aprmake.stderr.diff---------------
0a1,4
> apr_tables.c: In function `table_mergesort':
> apr_tables.c:1050: warning: unused variable `lowest'
> apr_tables.c: At top level:
> apr_tables.c:1201: warning: no previous prototype for `apr_table_cat'
--------------end of
/home/trawick/regressapr/logs/aprmake.stderr.diff-------------
------------------/home/trawick/regressapr/oldlogs/aprmake.stderr---------------
--------------end of
/home/trawick/regressapr/oldlogs/aprmake.stderr-------------
------------------/home/trawick/regressapr/logs/aprmake.stderr---------------
apr_tables.c: In function `table_mergesort':
apr_tables.c:1050: warning: unused variable `lowest'
apr_tables.c: At top level:
apr_tables.c:1201: warning: no previous prototype for `apr_table_cat'
--------------end of
/home/trawick/regressapr/logs/aprmake.stderr-------------
------------------/home/trawick/regressapr/logs/run_testtable.stdout---------------
Partial APR Tests:
Table: ........F
9 tests run: 8 passed, 1 failed, 0 not implemented.
Failed tests in Table:
1) table_overlap: expected <8> but was <7>
--------------end of
/home/trawick/regressapr/logs/run_testtable.stdout-------------
------------------/home/trawick/regressapr/logs/run_testtable.stderr---------------
--------------end of
/home/trawick/regressapr/logs/run_testtable.stderr-------------
Re: cvs commit: apr/tables apr_tables.c
Posted by Brian Pane <br...@cnet.com>.
I'll take a look at this today.
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
>
> /home/trawick/regressapr/oldlogs/aprmake.stderr and
> /home/trawick/regressapr/logs/aprmake.stderr are different!
> ------------------/home/trawick/regressapr/logs/aprmake.stderr.diff---------------
> 0a1,4
> > apr_tables.c: In function `table_mergesort':
> > apr_tables.c:1050: warning: unused variable `lowest'
> > apr_tables.c: At top level:
> > apr_tables.c:1201: warning: no previous prototype for `apr_table_cat'
> --------------end of
> /home/trawick/regressapr/logs/aprmake.stderr.diff-------------
> ------------------/home/trawick/regressapr/oldlogs/aprmake.stderr---------------
> --------------end of
> /home/trawick/regressapr/oldlogs/aprmake.stderr-------------
> ------------------/home/trawick/regressapr/logs/aprmake.stderr---------------
> apr_tables.c: In function `table_mergesort':
> apr_tables.c:1050: warning: unused variable `lowest'
> apr_tables.c: At top level:
> apr_tables.c:1201: warning: no previous prototype for `apr_table_cat'
> --------------end of
> /home/trawick/regressapr/logs/aprmake.stderr-------------
> ------------------/home/trawick/regressapr/logs/run_testtable.stdout---------------
> Partial APR Tests:
> Table: ........F
>
> 9 tests run: 8 passed, 1 failed, 0 not implemented.
>
> Failed tests in Table:
> 1) table_overlap: expected <8> but was <7>
>
> --------------end of
> /home/trawick/regressapr/logs/run_testtable.stdout-------------
> ------------------/home/trawick/regressapr/logs/run_testtable.stderr---------------
> --------------end of
> /home/trawick/regressapr/logs/run_testtable.stderr-------------
Re: cvs commit: apr/tables apr_tables.c
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Jeff Trawick <tr...@attglobal.net> writes:
[...]
> Partial APR Tests:
> Table: ........F
>
> 9 tests run: 8 passed, 1 failed, 0 not implemented.
>
> Failed tests in Table:
> 1) table_overlap: expected <8> but was <7>
With current cvs, that particular subtest (<8> vs <7>) of
table_overlap passes, but a later subtest fails for me. This
patch seems to fix my problem; I don't know if it will fix
yours:
Index: tables/apr_tables.c
===================================================================
RCS file: /home/cvspublic/apr/tables/apr_tables.c,v
retrieving revision 1.48
diff -u -r1.48 apr_tables.c
--- tables/apr_tables.c 22 Jun 2003 21:50:25 -0000 1.48
+++ tables/apr_tables.c 23 Jun 2003 12:31:15 -0000
@@ -1176,9 +1176,8 @@
(*sort_next)->key = NULL;
} while (++sort_next <= dup_last);
}
- else {
- last = sort_next;
- }
+
+ last = sort_next;
}
/* Shift elements to the left to fill holes left by removing duplicates */
Index: test/testtable.c
===================================================================
RCS file: /home/cvspublic/apr/test/testtable.c,v
retrieving revision 1.9
diff -u -r1.9 testtable.c
--- test/testtable.c 1 Jan 2003 00:01:56 -0000 1.9
+++ test/testtable.c 23 Jun 2003 12:31:15 -0000
@@ -174,7 +174,7 @@
val = apr_table_get(t1, "a");
CuAssertStrEquals(tc, val, "1");
val = apr_table_get(t1, "b");
- CuAssertStrEquals(tc, val, "2");
+ CuAssertStrEquals(tc, val, "2.");
val = apr_table_get(t1, "c");
CuAssertStrEquals(tc, val, "3");
val = apr_table_get(t1, "d");
--
Joe Schaefer
Re: cvs commit: apr/tables apr_tables.c
Posted by Brian Pane <br...@cnet.com>.
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