On Thu, Sep 20, 2001 at 09:37:07AM -0700, Doug MacEachern wrote:
> On Thu, 20 Sep 2001, Philippe M . Chiasson wrote:
>
> > After a fun week of machines crashing all over me, here is my first attempt at
> > a tied implementation of APR::Tables.
> >
> > Important to note that this will most probably be partly rewritten as I don't
>believe
> > it's "the right way" to do it. It does work for me though.
>
> cool. one reason i held off on this, was because i don't think
> APR::Table-s should be tied by default. but hadn't thought of a way to
> turn that on, but something along the lines of:
> use APR::Table tie => 1;
>
> or if they are tied by default:
> use APR::Table tie => 0;
> would turn it off.
Hrm.. interesting. But looking at the underlying implementation, I am really
curious at the speed trade-off. Having them tied all the time completely
transparently simplifies a lot of things, like $r->headers_out() doesn't have
to do anything 'magical' to return a tied hash. just return a apr_table_t *
and everything else is taken care of.
I'll investigate a bit more about that.
Wich makes me thing about a question I had. If I wanted to implement a piece
of, say APR::Table->foo() in pure perl, would it be possible ? I don't think
it is since the .xs and .pm are auto generated ?
>
> > The few things that I believe are not done right are:
> >
> > 1. the tie and untie functions should be macros
>
> because of ? speed? you could just MP_INLINE those functions.
Sure, that's easy enough to change...
>
> > 2. the T_APRTABLEOBJ typemap stuff is just nasty. The only thing I want to be
>able to do
> > is to define input/output typemap for an APR::Table, and right now I had to
>hardcode quite
> > a few things
>
> what did you have to hard code? APR::Table? that is an xsubpp variable,
> $Package i think.
Not what I mean is this :
in xs/typemap:
OUTPUT
T_APRTABLEOBJ
$arg = _mpxs_apr_table_tie(aTHX_ $arg, $var);
INPUT
T_APRTABLEOBJ
$var = _mpxs_apr_table_untie(aTHX_ $arg);
And adding
my %typemap = (
'Apache::RequestRec' => 'T_APACHEOBJ',
'apr_time_t' => 'T_APR_TIME',
> 'APR::Table' => 'T_APRTABLEOBJ',
);
in ModPerl/WrapXS.pm
It feels a bit to obscure and complicated to me ... I assumed there was a simpler
way that I didn't know of...
>
> > 3. All the mp_xs_sv2_* macros are completely separate from the typemap entries,
>and I think they
> > should be somehow tied together. Right now I have to
> > #define mp_xs_sv2_APR__Table(sv) \
> > _mpxs_apr_table_untie(aTHX_ sv)
> > if I want to use mpxs_usage_va_2 and friends
>
> you should only need mpxs_usage_foo if a function has a variable list
> argument (like $r->print(...)). anything else should be taken care of by
> the prototype that is generated in the .xs, like the current
> APR::Table::{get,set,etc} currently are.
I know, but since I am playing around with the typemap, I had to do:
#define mp_xs_sv2_APR__Table(sv) _mpxs_apr_table_untie(aTHX_ sv)
Otherwise it's automagically assumed that the underlying pointer is hidden
in a simple scalar, not a blessed thing...
Would be nice if the typemap or the mp_xs_sv2* macros would all use a common base.
Either make sure typemap entries only use mp_xs_sv2* macros, or the other way around.
>
> > 4. XS aliases. if apr_table_get and apr_table_FETCH are identical, there should
>be a way to simply
> > alias one to the other instead of duplicating the XS glue
> > newXS("APR::Table::FETCH", XS_APR__Table_get, file);
> > is all that's required.
>
> see Apache__RequestIO.h:
> #define mpxs_Apache__RequestRec_PRINT mpxs_Apache__RequestRec_print
>
> and modperl_functions.map:
> int:DEFINE_PRINT | | ...
I see, thanks.
> support for ALIAS would be nicer though.
Way better instead of duplicating code that doesn't have to.
>
> > 5. FIRSTKEY, NEXTKEY not implemented yet, as I have to figure out a place I can
>stash the state of an iterator somwehere.
>
> should be able todo what 1.x does. see APR::URI for an example of
> subclassing a data structure.
I am not sure I get that... Care to point me a little bit more in the right direction ?
> one note that i mentioned in another thread, no "_private" names please,
> i.e. no _ prefix. implemention of your patch is looking good so far, but
> the style is way off, please fix :-)
I forgot about the _ prefix, sorry about that. I wish there was a way to identify
a specific function as 100% private and make sure it's completely ignored by the
make source_scan
As for the style, I must confess since I knew this patch would not be final,
I didn't bother going thru every single line double-checking the style ;-p
I am lazy and not used to the ASF style quite yet.
I really wanted you to look at the overall implementation and point me in the right
direction.... I knew I was wrong in many places.
Following patch attempts should follow the style guide 99%, I promise. Keep reminding
me about it and I might even start altering the way I 'normally' code ;)
> and check that the patch will compile with 5.6.1, i think you'll at least
> need to add to modperl_perl_includes.h:
> #ifndef PERL_MAGIC_tied
> # define PERL_MAGIC_tied 'P'
> #endif
Oh, I thought we were relying on some fixes only in perl-devel, so I was only
testing against 5.7.2-rsync... Is it supposed to work flawlessly with 5.6.1 ?
If so, I will make sure I test against it.
So, thanks for the feedback, as this was what I was mostly interested in. Really
surprised that it actually works, but I wanted to know as soon as possible how far
from the right path I was.
I am on the road this week-end, but I will probably be able to have a "take 2" patch
avaliable on monday.
Gozer out to bed now.
>
>
>
>
>
>
>
--
+----------------------------------------------------+
| Philippe M. Chiasson <[EMAIL PROTECTED]> |
+----------------------------------------------------+
| F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5 |
+----------------------------------------------------+
gethostent not implemented : Your C library apparently
doesn't implement gethostent(), probably because if it did,
it'd feel morally obligated to return every hostname on the
Internet.
-- perldiag(1)
perl -e '$$=\${gozer};{$_=unpack(P26,pack(L,$$));/^Just Another Perl
Hacker!\n$/&&print||$$++&&redo}'
PGP signature
