This weekend I finally got around to implementing multivalued
key iterators in apreq2. Here is a backport of the each() code
to APR::Table, with apr-ext/table.t tests included. Rather
than lauching into a longwined explanation of the hows and
whys of this patch (see http://marc.theaimsgroup.com/?t=105478383300002&r=1&w=2
for the priors), please look over those new apr-ext/table.t tests to see what behavior this patch provides.
Excellent, Joe!
I haven't delved into details but it looks fine to me, sans a few indentation and style issue, that I pointed out below.
Thanks in advance for looking this over- I'll be happy to explain it in more detail if anyone is interested.
I think it'd be beneficial to document the logic in the code that implements it.
And also update the APR::Table manpage :)
Thanks.
Index: t/apr-ext/table.t =================================================================== RCS file: /home/cvspublic/modperl-2.0/t/apr-ext/table.t,v retrieving revision 1.1 diff -u -r1.1 table.t --- t/apr-ext/table.t 16 Jun 2004 03:55:48 -0000 1.1 +++ t/apr-ext/table.t 12 Jul 2004 18:54:24 -0000 @@ -1,15 +1,32 @@ use Apache::Test; - +use Apache::TestUtil;
please also add:
use strict; use warnings FATAL => 'all';
(which should have been there in first place) and please fix the test accordingly :) (missing my declarations)
+while (($a,$b) = each %$table) {
+ my $key = ("first", "second")[$i % 2];
+ my $val = ++$i;
+ ok t_cmp $a, $key, "table each: key test";
+ ok t_cmp $b, $val, "table each: value test";
+ ok t_cmp $table->get($a), $val, "table each: get test";
+ ok t_cmp tied(%$table)->get($a), $val, "table each: tied get test";
+}
But if you don't mind leave that test alone and instead extend t/response/TestAPR/table.pm, since that's where it all tested. As mentioned before, I'd love to see apr-ext/ and non-apr-ext somehow re-using the same tests, instead of duplicating and spreading them all over, making it hard to maintain.
If it's too much of a bother, I can do that move.
Index: src/modules/perl/modperl_common_util.c ===================================================================
else {
@@ -67,7 +67,17 @@
"(expecting an %s derived object)", classname);
}
- return NULL;
+ return &PL_sv_undef;
+}
new line here please
+MP_INLINE void *modperl_hash_tied_object(pTHX_ + const char *classname,
+ SV *tsv)
+{
+ SV *rv = modperl_hash_tied_object_rv(aTHX_ classname, tsv);
+ if (SvROK(rv))
+ return (void *)SvIVX(SvRV(rv));
+ else
+ return NULL;
{} are a must everywhere in conditionals, even if there is only one statement following.
Index: xs/APR/Table/APR__Table.h[...]
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/APR/Table/APR__Table.h,v
retrieving revision 1.11
diff -u -r1.11 APR__Table.h
--- xs/APR/Table/APR__Table.h 4 Mar 2004 06:01:10 -0000 1.11
+++ xs/APR/Table/APR__Table.h 12 Jul 2004 18:54:26 -0000
@@ -13,7 +13,6 @@
* limitations under the License.
*/
-#define mpxs_APR__Table_FETCH apr_table_get
#define mpxs_APR__Table_STORE apr_table_set
#define mpxs_APR__Table_DELETE apr_table_unset
#define mpxs_APR__Table_CLEAR apr_table_clear
@@ -122,54 +121,74 @@
+ if (GIMME_V == G_SCALAR) {
+ if (i > 0 && i <= arr->nelts + && !strcasecmp(key, elts[i-1].key))
+ {
opening { must be on the same line as the conditional statement.
-- __________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
