On Wed, Sep 12, 2001 at 09:57:16AM -0700, Doug MacEachern wrote:
> On Tue, 11 Sep 2001, Philippe M . Chiasson wrote:
>
> > As promised, APR::Table->do()
>
> cool!
>
> > A few things worth noting:
> >
> > * [OT] make source_scan gives me a huge diff and I have to manually weed out
>what's not
> > related to my stuff before sending, annoying ... why ?
>
> you don't need to send xs/tables diffs, i can just regenerate them here.
> >
> > APR::Table->do("my_sub") || APR::Table->do(sub {...});
> >
> > * Namespace issue. I am not sure about _mpxs_APR__Table_do_callback_data... it's
> > clear, but it's long. But I did it because that data shouldn't be used by
>anybody
> > but mpxs_APR__Table_do. Better suggestions ?
>
> you can pick something shorter, something like mpxs_table_cb_data_t is
> fine.
>
> > * Still doesn't feel like the right way to put it in
>xs/maps/apr_functions.map....but works
>
> see previous comment:
> apr_table_do | mpxs_
>
> to setup the call to mpxs_apr_table_do
>
> > * Filtering
> > * In 1.3, used to populate a table with the extra arguments passed to do and
>filter
> > in the C callback, just before calling the perl callback.
> > * apr_table_vdo supports filtering, but it accepts a va_list as argument and
>uses
> > va_arg to fetch the elements. Question, is there a way to create something
>that
> > apr_table_vdo would swallow as a va_list and on wich va_arg(vp, char *)
>would work?
> > If so, please tell me, as it would skip many operations and speed things
>up a bit.
>
> i think this addressed in your second patch (haven't looked yet).
Right now, my patch implements filtering on the modperl side with apr_tables to
hold the filter list, but the best solution would be to call vdo directly, but
I've read in many places that there is no way to fake a va_list to pass along
to vdo.
Possible solution would be to reenginer the vdo function in apr to accept
a char * array and change apr_table_do to generate that array from the va_list,
that way, we could call it directly with a filter list and not have to do it
again. Is it a good idea ? souldn't be that hard to do without breaking httpd ;p
> > +#define _mpxs_APR__Table_do_callback_prototype (int (*)(void *, const char *,
>const char *))
>
> this should be a typedef.
Yeah, I always have to look around for the right typdef syntax for function pointers.
>
> > +static MP_INLINE
> > +void mpxs_APR__Table_do(apr_table_t *table, SV *sub)
> > +{
> > + dTHX; /*XXX*/
>
> dTHX; should be avoided whenever possible. if you setup the .map as
> discussed:
> apr_table_do | mpxs_
1: Why dTHX should be avoided whenever possible ?
2: If so, I can remove it from the mpxs_apr_table_do entry point, but I'll have
to use it in the callback function anyways, unless I can somehow stash these
variables into the callback data struct, but I am _really_ not sure about that.
> mpxs_apr_table_do
> will be passed aTHX
>
> there's still minor style issues:
> if(...){
> vs.
> if (...) {
>
> and
> ("one","two","three")
> vs.
> ("one", "two", "three")
Damn, damn, damn ;-!
> otherwise, looking good!
Hope this one looks better...
--
Philippe M. Chiasson <[EMAIL PROTECTED]>
Extropia's Resident System Guru
http://www.eXtropia.com/
Had to create %s unexpectedly : A routine asked for a
symbol from a symbol table that ought to have existed
already, but for some reason it didn't, and had to be
created on an emergency basis to prevent a core dump.
-- perldiag(1)
perl -e '$$=\${gozer};{$_=unpack(P26,pack(L,$$));/^Just Another Perl
Hacker!\n$/&&print||$$++&&redo}'
--- /dev/null Sat Mar 24 12:37:44 2001
+++ xs/APR/Table/APR__Table.h Fri Sep 14 23:56:17 2001
@@ -0,0 +1,75 @@
+typedef struct {
+ SV *cv;
+ apr_table_t *filter; /*XXX: or maybe a mgv ? */
+} mpxs_table_do_cb_data_t;
+
+typedef int (*_mpxs_apr_table_do_cb_p)(void *, const char *, const char *);
+
+static int _mpxs_apr_table_do_cb(mpxs_table_do_cb_data_t *tdc_data, const char *key,
+const char *val)
+{
+ dTHX; /*XXX*/
+ dSP;
+ int rv=0;
+
+ /* Skip completely if something is wrong */
+ if ((!tdc_data) || (!tdc_data->cv) || (!key) || (!val))
+ return 0;
+
+ /* Skip entries if not in our filter list */
+ if (tdc_data->filter) {
+ if (!apr_table_get(tdc_data->filter,key)) {
+ return 1;
+ }
+ }
+
+ ENTER;
+ SAVETMPS;
+ PUSHMARK(sp);
+ XPUSHs(sv_2mortal(newSVpv(key,0)));
+ XPUSHs(sv_2mortal(newSVpv(val,0)));
+ PUTBACK;
+ rv = call_sv(tdc_data->cv, 0);
+ SPAGAIN;
+ rv = (1 == rv) ? POPi : 1;
+ PUTBACK;
+ FREETMPS;
+ LEAVE;
+
+ /* rv of 0 aborts the traversal */
+ return rv;
+}
+
+static MP_INLINE
+void mpxs_apr_table_do(pTHX_ I32 items, SV **MARK, SV **SP)
+{
+ dAX; /*XXX*/
+
+ apr_table_t *table;
+ SV *sub;
+ mpxs_table_do_cb_data_t tdc_data;
+
+ mpxs_usage_va_2(table,sub, "$table->do(sub,[@filter])");
+
+ tdc_data.cv=sub;
+ tdc_data.filter=NULL;
+
+ if (items > 2) {
+ STRLEN len;
+ tdc_data.filter = apr_table_make(table->a.pool, items-2);
+ while ( MARK <= SP ) {
+ apr_table_set(tdc_data.filter, SvPV(*MARK,len), "1");
+ MARK++;
+ }
+ }
+
+ /*XXX: would be nice to be able to call apr_table_vdo directly,
+ but I don't think it's possible to create/populate something
+ that smells like a va_list with our list of filters specs
+ */
+
+ apr_table_do( (_mpxs_apr_table_do_cb_p) _mpxs_apr_table_do_cb, (void *)
+&tdc_data, table, NULL);
+
+ /* Free tdc_data.filter or wait for the pool to go away? */
+
+ return;
+}
Index: lib/ModPerl/TypeMap.pm
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/lib/ModPerl/TypeMap.pm,v
retrieving revision 1.9
diff -u -I'$Id' -I'$Revision' -r1.9 TypeMap.pm
--- lib/ModPerl/TypeMap.pm 2001/09/10 05:35:10 1.9
+++ lib/ModPerl/TypeMap.pm 2001/09/14 15:57:46
@@ -396,6 +396,7 @@
Apache__RequestRec => 'r',
Apache__Server => 'server',
Apache__Connection => 'connection',
+ APR__Table => 'table',
APR__UUID => 'uuid',
apr_status_t => 'status',
);
Index: t/response/TestAPR/table.pm
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/t/response/TestAPR/table.pm,v
retrieving revision 1.1
diff -u -I'$Id' -I'$Revision' -r1.1 table.pm
--- t/response/TestAPR/table.pm 2001/09/12 16:40:27 1.1
+++ t/response/TestAPR/table.pm 2001/09/14 15:57:46
@@ -8,10 +8,13 @@
use Apache::Const -compile => 'OK';
use APR::Table ();
+my $filter_count;
+my $TABLE_SIZE = 20;
+
sub handler {
my $r = shift;
- plan $r, tests => 5;
+ plan $r, tests => 9;
my $table = APR::Table::make($r->pool, 16);
@@ -25,7 +28,52 @@
ok not defined $table->get('foo');
+ for(1..$TABLE_SIZE)
+ {
+ $table->set(chr($_+97),$_);
+ }
+
+ #Simple filtering
+ $filter_count = 0;
+ $table->do("my_filter");
+ ok $filter_count == $TABLE_SIZE;
+
+ #Filtering aborting in the middle
+ $filter_count = 0;
+ $table->do("my_filter_stop");
+ ok $filter_count == int($TABLE_SIZE)/2;
+
+ #Filtering with anon sub
+ $filter_count=0;
+ $table->do(sub {
+ my ($key,$value) = @_;
+ $filter_count++;
+ die "arguments I recieved are bogus($key,$value)" unless $key eq
+chr($value+97);
+ return 1;
+ }
+ );
+ ok $filter_count == $TABLE_SIZE;
+
+ $filter_count=0;
+ $table->do("my_filter","c","b","e");
+ ok $filter_count == 3;
+
Apache::OK;
+}
+
+sub my_filter {
+ my ($key,$value) = @_;
+ $filter_count++;
+ die "arguments I recieved are bogus($key,$value)" unless $key eq chr($value+97);
+ return 1;
+}
+
+sub my_filter_stop {
+ my ($key,$value) = @_;
+ $filter_count++;
+ die "arguments I recieved are bogus($key,$value)" unless $key eq chr($value+97);
+ return 0 if ($filter_count == int($TABLE_SIZE)/2);
+ return 1;
}
1;
Index: todo/api.txt
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/todo/api.txt,v
retrieving revision 1.2
diff -u -I'$Id' -I'$Revision' -r1.2 api.txt
--- todo/api.txt 2001/09/08 18:26:46 1.2
+++ todo/api.txt 2001/09/14 15:57:46
@@ -9,8 +9,6 @@
$r->headers_out->{KEY} is not currently supported
might want to make this optional, disabled by default
-missing: APR::Table->do
-
$r->finfo:
need apr_finfo_t <-> struct stat conversion (might already be there,
haven't looked close enough yet)
Index: xs/maps/apr_functions.map
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/xs/maps/apr_functions.map,v
retrieving revision 1.18
diff -u -I'$Id' -I'$Revision' -r1.18 apr_functions.map
--- xs/maps/apr_functions.map 2001/09/10 06:42:51 1.18
+++ xs/maps/apr_functions.map 2001/09/14 15:57:46
@@ -183,7 +183,7 @@
apr_table_overlay | | base, overlay, p
apr_table_add
-apr_table_addn
- apr_table_do
+ apr_table_do | mpxs_ | ...
apr_table_get
apr_table_merge
-apr_table_mergen
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/xs/tables/current/ModPerl/FunctionTable.pm,v
retrieving revision 1.24
diff -u -I'$Id' -I'$Revision' -r1.24 FunctionTable.pm
--- xs/tables/current/ModPerl/FunctionTable.pm 2001/09/13 04:41:44 1.24
+++ xs/tables/current/ModPerl/FunctionTable.pm 2001/09/14 15:57:47
@@ -3244,6 +3244,28 @@
]
},
{
+ 'return_type' => 'void',
+ 'name' => 'mpxs_apr_table_do',
+ 'args' => [
+ {
+ 'type' => 'PerlInterpreter *',
+ 'name' => 'my_perl'
+ },
+ {
+ 'type' => 'I32',
+ 'name' => 'items'
+ },
+ {
+ 'type' => 'SV **',
+ 'name' => 'mark'
+ },
+ {
+ 'type' => 'SV **',
+ 'name' => 'sp'
+ }
+ ]
+ },
+ {
'return_type' => 'char *',
'name' => 'mpxs_APR__URI_port',
'args' => [
PGP signature
