You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2003/06/05 05:28:48 UTC
[Fwd: Re: [rfc] APR::Table & polymorphic values]
Fwd
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
>
>>Thanks for the drawing. Now I don't understand how do you get to the
>>apreq_value_t pointer from table_entry->val. Do you just subtract the
>>size of the apreq_value_t struct and based on the knowledge that the
>>memory allocation is continues, you magically retrieve the whole struct?
>
>
> Essentially, yes. That's one of the things the offsetof macro is
> good for. For specifics, see how apreq_attr_to_type is defined in apreq.h.
>
>
>>I think I understand the trick now. it wasn't obvious at all ;)
>
>
> Yeah, it took me a month to get used to it. You seem to have figured it
> out in far less time :-).
Thanks Joe ;) I had to ask Eric Cholet about the data[1] trick, which ensures
a continuous memory. I didn't know that. and the C FAQ entry 2.6 is far from
being clear about that.
>>and since newSVpv copies the string, you no longer can get to the
>>beginning of the struct. Now I understand why you wanted to mark it as
>>SvFAKE (that would prevent the copy).
>>
>>Now that I understand this thing I can think intelligently ;) For
>>example how about making SV into a dual-var, ala $!. it might not work
>>if IV slot is used to store the original pointer, since using val in
>>the numeric context will need to make use of this field. Same with NV?
>>I'm just thinking how can we save the overhead of messing with MAGIC
>>if we can.
>
>
> Yeah, me too. The patch just adds an sv_magic() call whenever newSVpv
> is called, which shouldn't be too costly (no magic lookups are caused
> by APR::Tables, just the overhead of adding magic to any returned scalars).
OK, my take on this is as follows: I don't think we should change the current
implementation in mod_perl because it's very non-generic. If tomorrow someone
wants to use APR::Table to do something else, they may want to change the core
implementation too, how do we say 'no' in that case?
I think it's OK to duplicate the implementation and change it the way APREQ
needs. Sort of sub-classing APR::Table. However if you think of a way to
rewrite the current implementation to make it easier to sub-class XS please
suggest how. Though it should happen without adding an extra overhead.
APR::Table is a very busy API, used by many mod_perl functions, so if we can
avoid adding any overhead at all, it's a good thing for an overall performance.
What do you think?
It's not that I don't want to change it, I'm just trying to keep the number of
mod_perl calls (memory usage) to bare minimum for the maximum performance.
>>So you say that due to the bug we can't sub-class get() to do newSVpv
>>while using the pointer to apreq_value_t and then setting SvCUR to the
>>real string?
>
>
> The bug I was referring to is tickled by the numeric tests (4,6,8,14) in
> modperl/env.t when I used this definition of mpxs_apr_table_str_2sv:
>
> MP_INLINE static SV *mpxs_apr_table_str_2sv(pTHX_ const char *str)
> {
> SV *sv;
> if (str == NULL)
> return &PL_sv_undef;
>
> sv = newSV(0);
> SvUPGRADE(sv, SVt_PV);
> SvCUR(sv) = strlen(str);
> SvPVX(sv) = (char *)str;
> SvPOK_on(sv);
> /* LEAVE SvLEN == 0 */
> return sv_2mortal(sv);
> }
>
> IMO, this would be (close to the) ideal approach, because a few
> nice things happen behind the scenes:
>
> SvLEN = 0 prevents perl from attempting to call Safefree()
> on the SvPVX during sv_free, which would cause a segfault
> since that memory was malloced from an apr_pool. SvLEN = 0 also
> prevents perl from "stealing" the SvPVX during assignment, (an
> optimization in sv_setsv). SvLEN = 0 causes assignment to induce a
> string copy, which is _perfectly fine_ since the apreq subclass'
> action can happen right on the return stack, *before* any perl
> assignment is made.
>
> The real problem with SvLEN=0 is sv_2iv, which would normally
> upgrade the SVt_PV to a numeric type, balks at having SvLEN = 0.
> sv_2iv winds up converting *any* such string to having a numeric 0
> IV. That breaks the numeric tests in modperl/env.t, at least with
> perl 5.8.0.
:(
Another idea I had is to append the 'str' pointer after \0. I guess that won't
survive copying and may introduce a leakage.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
> Thanks for the drawing. Now I don't understand how do you get to the
> apreq_value_t pointer from table_entry->val. Do you just subtract the
> size of the apreq_value_t struct and based on the knowledge that the
> memory allocation is continues, you magically retrieve the whole struct?
Essentially, yes. That's one of the things the offsetof macro is
good for. For specifics, see how apreq_attr_to_type is defined in apreq.h.
> I think I understand the trick now. it wasn't obvious at all ;)
Yeah, it took me a month to get used to it. You seem to have figured it
out in far less time :-).
> and since newSVpv copies the string, you no longer can get to the
> beginning of the struct. Now I understand why you wanted to mark it as
> SvFAKE (that would prevent the copy).
>
> Now that I understand this thing I can think intelligently ;) For
> example how about making SV into a dual-var, ala $!. it might not work
> if IV slot is used to store the original pointer, since using val in
> the numeric context will need to make use of this field. Same with NV?
> I'm just thinking how can we save the overhead of messing with MAGIC
> if we can.
Yeah, me too. The patch just adds an sv_magic() call whenever newSVpv
is called, which shouldn't be too costly (no magic lookups are caused
by APR::Tables, just the overhead of adding magic to any returned scalars).
> So you say that due to the bug we can't sub-class get() to do newSVpv
> while using the pointer to apreq_value_t and then setting SvCUR to the
> real string?
The bug I was referring to is tickled by the numeric tests (4,6,8,14) in
modperl/env.t when I used this definition of mpxs_apr_table_str_2sv:
MP_INLINE static SV *mpxs_apr_table_str_2sv(pTHX_ const char *str)
{
SV *sv;
if (str == NULL)
return &PL_sv_undef;
sv = newSV(0);
SvUPGRADE(sv, SVt_PV);
SvCUR(sv) = strlen(str);
SvPVX(sv) = (char *)str;
SvPOK_on(sv);
/* LEAVE SvLEN == 0 */
return sv_2mortal(sv);
}
IMO, this would be (close to the) ideal approach, because a few
nice things happen behind the scenes:
SvLEN = 0 prevents perl from attempting to call Safefree()
on the SvPVX during sv_free, which would cause a segfault
since that memory was malloced from an apr_pool. SvLEN = 0 also
prevents perl from "stealing" the SvPVX during assignment, (an
optimization in sv_setsv). SvLEN = 0 causes assignment to induce a
string copy, which is _perfectly fine_ since the apreq subclass'
action can happen right on the return stack, *before* any perl
assignment is made.
The real problem with SvLEN=0 is sv_2iv, which would normally
upgrade the SVt_PV to a numeric type, balks at having SvLEN = 0.
sv_2iv winds up converting *any* such string to having a numeric 0
IV. That breaks the numeric tests in modperl/env.t, at least with
perl 5.8.0.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Philippe M. Chiasson" <go...@cpan.org> writes:
[...]
> One thing I _do_ remember is that while (my($a, $b) = each %$table) { }
> did iterate over all key/value pairs when I initially implemented the
> TIE'ed stuff.. But once again, it's been a long time since I looked at
> that stuff.
It always iterated over the keys correctly, just not the (multi-)values.
The problem really lies in perl's tiehash implementation of each();
internally it's doing the moral equivalent of
sub each {
my $t = shift;
my $key = $t->NEXTKEY($prev_key) or return;
return wantarray ? ($key, $t->FETCH($key)) : $key;
}
IOW, each() really calls FETCH to get the value. As long as FETCH
doesn't work, each() in list context doesn't work either.
> But one thing is for sure, this patch (and this thread also, actually)
> is indeed quite ugly (no offense meant)...
Agreed. btw- I'm never offended by code critiques. Go ahead,
feel free to fire away :-)
> There _has_ to be a simpler to make this work...
Looking over things again, it seems we might be able to make
use of the xpvhv slots in the underlying HV (which are mostly
unused because of the tie magic). I just looked at hv.h and picked
xhv_array and xhv_eiter. I've no clue what they're used for
wrt tied hashes, but these implementations of NEXTKEY & FETCH
seems to work (all tests pass):
static MP_INLINE SV *mpxs_APR__Table_NEXTKEY(pTHX_ SV *tsv, SV *key)
{
apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
if (apr_is_empty_table(t)) {
return Nullsv;
}
if (mpxs_apr_table_iterix(tsv) < apr_table_elts(t)->nelts) {
apr_table_entry_t *e = ((apr_table_entry_t *)
apr_table_elts(t)->elts) + mpxs_apr_table_iterix(tsv)++;
STRLEN len = strlen(e->key);
SV *sv = newSVpv(e->key, 0);
XPVHV *xhv = (XPVHV *)SvANY(SvRV(tsv));
xhv->xhv_eiter = (HE *)e;
xhv->xhv_array = SvPVX(sv);
return sv;
}
mpxs_apr_table_iterix(tsv) = 0; /* done */
return &PL_sv_undef;
}
static MP_INLINE
const char *mpxs_APR__Table_FETCH(pTHX_ SV *tsv, SV *sv)
{
apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
MAGIC *mg;
XPVHV *xhv = (XPVHV *)SvANY(SvRV(tsv));
if (!t) {
return NULL;
}
if (xhv->xhv_array == SvPV_nolen(sv))
{
apr_table_entry_t *e = (apr_table_entry_t *)xhv->xhv_eiter;
fprintf(stderr, "ITERATOR CALL: %s => %s\n", e->key, e->val);
return e->val;
}
else {
return apr_table_get(t, SvPV_nolen(sv));
}
}
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Fri, 2003-06-06 at 12:19, Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> > I'd hate to take the joy of adding these wonderful features away from
> > Philippe, who's now busy wrestling with mod_perl 1.28 release, but
> > once done will certainly love to do those. Right Philippe?
Right ;-) I've just finally decided to peek into this long thread and
I'll need some time to sort it all out in my busy head ;-)
> Curiosity got the better of me. Here's a patch that seems to
> work (all tests pass), but it sure ain't pretty. Hopefully
> it'll help once Philippe gets some free tuits.
I'll read over all this and provide hopefully usefull comments ;-)
One thing I _do_ remember is that while (my($a, $b) = each %$table) { }
did iterate over all key/value pairs when I initially implemented the
TIE'ed stuff.. But once again, it's been a long time since I looked at
that stuff.
But one thing is for sure, this patch (and this thread also, actually)
is indeed quite ugly (no offense meant)...
There _has_ to be a simpler to make this work...
I'll probably get around looking at this near the end of this week.
Gozer out.
> Index: t/response/TestAPR/table.pm
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/t/response/TestAPR/table.pm,v
> retrieving revision 1.5
> diff -u -r1.5 table.pm
> --- t/response/TestAPR/table.pm 11 Apr 2002 11:08:44 -0000 1.5
> +++ t/response/TestAPR/table.pm 6 Jun 2003 04:07:33 -0000
> @@ -15,7 +15,7 @@
> sub handler {
> my $r = shift;
>
> - plan $r, tests => 17;
> + plan $r, tests => 26;
>
> my $table = APR::Table::make($r->pool, $TABLE_SIZE);
>
> @@ -34,6 +34,14 @@
> $array[0] eq 'bar' &&
> $array[1] eq 'tar' &&
> $array[2] eq 'kar';
> +
> + my $c = 0;
> + while (my($a, $b) = each %$table) {
> + ok $a eq 'foo';
> + ok $b eq $array[$c];
> + ok not defined $table->{'bar'};
> + $c++;
> + }
>
> ok $table->unset('foo') || 1;
>
> Index: xs/APR/Table/APR__Table.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/xs/APR/Table/APR__Table.h,v
> retrieving revision 1.10
> diff -u -r1.10 APR__Table.h
> --- xs/APR/Table/APR__Table.h 4 Jun 2003 02:31:56 -0000 1.10
> +++ xs/APR/Table/APR__Table.h 6 Jun 2003 04:07:33 -0000
> @@ -1,4 +1,4 @@
> -#define mpxs_APR__Table_FETCH apr_table_get
> +//#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
> @@ -105,25 +105,45 @@
> ((apr_table_entry_t *) \
> apr_table_elts(t)->elts)[mpxs_apr_table_iterix(sv)++].key
>
> -static MP_INLINE const char *mpxs_APR__Table_NEXTKEY(pTHX_ SV *tsv, SV *key)
> +
> +static MP_INLINE SV *mpxs_APR__Table_NEXTKEY(pTHX_ SV *tsv, SV *key)
> {
> apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
> -
> if (apr_is_empty_table(t)) {
> - return NULL;
> + return Nullsv;
> }
>
> if (mpxs_apr_table_iterix(tsv) < apr_table_elts(t)->nelts) {
> - return mpxs_apr_table_nextkey(t, tsv);
> - }
> + apr_table_entry_t *e = ((apr_table_entry_t *)
> + apr_table_elts(t)->elts) + mpxs_apr_table_iterix(tsv)++;
> + STRLEN len = strlen(e->key);
> + SV *sv = newSV(0);
> +
> + /* XXX: really nasty hack: set the numeric value of the key
> + * to represent a pointer to the corresponding val.
> + * We mark SvEND with another copy of the val's address
> + * as a means of confirming SvIVX really repesents a
> + * pointer.
> + */
> + SvUPGRADE(sv, SVt_PVIV);
> + SvGROW(sv, len + 3*sizeof(IV) + 1);
> + memcpy(SvPVX(sv), e->key, len);
> + SvCUR_set(sv, len);
> + SvEND(sv)[0] = 0;
> + SvIVX(sv) = (IV) e->val;
> + ((IV *)SvEND(sv))[1] = SvIVX(sv) = (IV) e->val;
> + SvPOK_on(sv);
> + SvIOK_on(sv);
>
> - return NULL;
> + return sv;
> + }
> + mpxs_apr_table_iterix(tsv) = 0; /* done */
> + return &PL_sv_undef;
> }
>
> -static MP_INLINE const char *mpxs_APR__Table_FIRSTKEY(pTHX_ SV *tsv)
> +static MP_INLINE SV *mpxs_APR__Table_FIRSTKEY(pTHX_ SV *tsv)
> {
> mpxs_apr_table_iterix(tsv) = 0; /* reset iterator index */
> -
> return mpxs_APR__Table_NEXTKEY(aTHX_ tsv, Nullsv);
> }
>
> @@ -164,4 +184,30 @@
> }
> });
>
> +}
> +
> +static MP_INLINE
> +const char *mpxs_APR__Table_FETCH(pTHX_ SV *tsv, SV *sv)
> +{
> + apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
> + MAGIC *mg;
> + if (!t) {
> + return NULL;
> + }
> +
> + /* XXX: really nasty hack, part 2: check for
> + * an SV coming from mpxs_APR_TABLE_NEXT. If
> + * it is, we take the return value directly
> + * from SvIVX.
> + */
> +
> + if (SvPOK(sv) && SvIOK(sv) &&
> + SvLEN(sv) == SvCUR(sv) + 3*sizeof(IV) + 1 &&
> + ((IV *)SvEND(sv))[1] == SvIVX(sv))
> + {
> + return (const char *)SvIVX(sv);
> + }
> + else {
> + return apr_table_get(t, SvPV_nolen(sv));
> + }
> }
> Index: xs/maps/apr_functions.map
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/xs/maps/apr_functions.map,v
> retrieving revision 1.52
> diff -u -r1.52 apr_functions.map
> --- xs/maps/apr_functions.map 15 Apr 2003 08:39:52 -0000 1.52
> +++ xs/maps/apr_functions.map 6 Jun 2003 04:07:34 -0000
> @@ -246,10 +246,11 @@
> -apr_table_setn
> apr_table_unset
> -apr_table_vdo
> - const char *:DEFINE_FETCH | | apr_table_t *:t, const char *:key
> +#const char *:DEFINE_FETCH | | apr_table_t *:t, const char *:key
> void:DEFINE_STORE | | apr_table_t *:t, const char *:key, const char *:value
> void:DEFINE_DELETE | | apr_table_t *:t, const char *:key
> void:DEFINE_CLEAR | | apr_table_t *:t
> + mpxs_APR__Table_FETCH
> mpxs_APR__Table_FIRSTKEY
> mpxs_APR__Table_NEXTKEY
> mpxs_APR__Table_EXISTS
> Index: xs/tables/current/ModPerl/FunctionTable.pm
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v
> retrieving revision 1.116
> diff -u -r1.116 FunctionTable.pm
> --- xs/tables/current/ModPerl/FunctionTable.pm 4 Jun 2003 16:50:38 -0000 1.116
> +++ xs/tables/current/ModPerl/FunctionTable.pm 6 Jun 2003 04:07:39 -0000
> @@ -4958,7 +4958,7 @@
> ]
> },
> {
> - 'return_type' => 'const char *',
> + 'return_type' => 'SV *',
> 'name' => 'mpxs_APR__Table_FIRSTKEY',
> 'attr' => [
> 'static',
> @@ -4977,6 +4977,28 @@
> },
> {
> 'return_type' => 'const char *',
> + 'name' => 'mpxs_APR__Table_FETCH',
> + 'attr' => [
> + 'static',
> + '__inline__'
> + ],
> + 'args' => [
> + {
> + 'type' => 'PerlInterpreter *',
> + 'name' => 'my_perl'
> + },
> + {
> + 'type' => 'SV *',
> + 'name' => 'tsv'
> + },
> + {
> + 'type' => 'SV *',
> + 'name' => 'key'
> + }
> + ]
> + },
> + {
> + 'return_type' => 'SV *',
> 'name' => 'mpxs_APR__Table_NEXTKEY',
> 'attr' => [
> 'static',
--
-- -----------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
>
>>what happens if the key is used in a numeric context? won't this
>>scheme blow?
>
>
> Sure would, if someone's expecting a key to have
> a normal numeric value. I won't try to pursue
> the point further, I'm just trying to leave you
> with an idea of how crappy a workaround would look.
;)
>>Probably the only safe way to do that is to attach magic :(
>
>
> Doesn't work (the bottom line is that string assignment doesn't
> copy SvMAGIC, but I won't bore you with the sv_setsv details).
>
> Anyways, I still think it's best to just document the
> brokenness of the TIEHASH API for APR::Tables, since
>
> 1) perl's TIEHASH API is stupendously slow anyways,
> 2) it isn't designed to handle multivalued keys.
True.
Though there is the do() "filter" interface, which allows you have a callback
which iterates correctly with multivalued keys, though might be an overkill
for big tables.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
> what happens if the key is used in a numeric context? won't this
> scheme blow?
Sure would, if someone's expecting a key to have
a normal numeric value. I won't try to pursue
the point further, I'm just trying to leave you
with an idea of how crappy a workaround would look.
> Probably the only safe way to do that is to attach magic :(
Doesn't work (the bottom line is that string assignment doesn't
copy SvMAGIC, but I won't bore you with the sv_setsv details).
Anyways, I still think it's best to just document the
brokenness of the TIEHASH API for APR::Tables, since
1) perl's TIEHASH API is stupendously slow anyways,
2) it isn't designed to handle multivalued keys.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
>
>>I'd hate to take the joy of adding these wonderful features away from
>>Philippe, who's now busy wrestling with mod_perl 1.28 release, but
>>once done will certainly love to do those. Right Philippe?
>
>
> Curiosity got the better of me. Here's a patch that seems to
> work (all tests pass), but it sure ain't pretty. Hopefully
> it'll help once Philippe gets some free tuits.
;)
> + my $c = 0;
> + while (my($a, $b) = each %$table) {
> + ok $a eq 'foo';
> + ok $b eq $array[$c];
> + ok not defined $table->{'bar'};
> + $c++;
> + }
that reads much better to my idiomatic brain than the scary:
for (1..X) {
my($a, $b) = each %$table;
...
}
as it was in your original patch ;)
> + /* XXX: really nasty hack: set the numeric value of the key
> + * to represent a pointer to the corresponding val.
> + * We mark SvEND with another copy of the val's address
> + * as a means of confirming SvIVX really repesents a
> + * pointer.
> + */
> + SvUPGRADE(sv, SVt_PVIV);
> + SvGROW(sv, len + 3*sizeof(IV) + 1);
> + memcpy(SvPVX(sv), e->key, len);
> + SvCUR_set(sv, len);
> + SvEND(sv)[0] = 0;
> + SvIVX(sv) = (IV) e->val;
> + ((IV *)SvEND(sv))[1] = SvIVX(sv) = (IV) e->val;
> + SvPOK_on(sv);
> + SvIOK_on(sv);
what happens if the key is used in a numeric context? won't this scheme blow?
Probably the only safe way to do that is to attach magic :(
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
> I'd hate to take the joy of adding these wonderful features away from
> Philippe, who's now busy wrestling with mod_perl 1.28 release, but
> once done will certainly love to do those. Right Philippe?
Curiosity got the better of me. Here's a patch that seems to
work (all tests pass), but it sure ain't pretty. Hopefully
it'll help once Philippe gets some free tuits.
Index: t/response/TestAPR/table.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/t/response/TestAPR/table.pm,v
retrieving revision 1.5
diff -u -r1.5 table.pm
--- t/response/TestAPR/table.pm 11 Apr 2002 11:08:44 -0000 1.5
+++ t/response/TestAPR/table.pm 6 Jun 2003 04:07:33 -0000
@@ -15,7 +15,7 @@
sub handler {
my $r = shift;
- plan $r, tests => 17;
+ plan $r, tests => 26;
my $table = APR::Table::make($r->pool, $TABLE_SIZE);
@@ -34,6 +34,14 @@
$array[0] eq 'bar' &&
$array[1] eq 'tar' &&
$array[2] eq 'kar';
+
+ my $c = 0;
+ while (my($a, $b) = each %$table) {
+ ok $a eq 'foo';
+ ok $b eq $array[$c];
+ ok not defined $table->{'bar'};
+ $c++;
+ }
ok $table->unset('foo') || 1;
Index: xs/APR/Table/APR__Table.h
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/APR/Table/APR__Table.h,v
retrieving revision 1.10
diff -u -r1.10 APR__Table.h
--- xs/APR/Table/APR__Table.h 4 Jun 2003 02:31:56 -0000 1.10
+++ xs/APR/Table/APR__Table.h 6 Jun 2003 04:07:33 -0000
@@ -1,4 +1,4 @@
-#define mpxs_APR__Table_FETCH apr_table_get
+//#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
@@ -105,25 +105,45 @@
((apr_table_entry_t *) \
apr_table_elts(t)->elts)[mpxs_apr_table_iterix(sv)++].key
-static MP_INLINE const char *mpxs_APR__Table_NEXTKEY(pTHX_ SV *tsv, SV *key)
+
+static MP_INLINE SV *mpxs_APR__Table_NEXTKEY(pTHX_ SV *tsv, SV *key)
{
apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
-
if (apr_is_empty_table(t)) {
- return NULL;
+ return Nullsv;
}
if (mpxs_apr_table_iterix(tsv) < apr_table_elts(t)->nelts) {
- return mpxs_apr_table_nextkey(t, tsv);
- }
+ apr_table_entry_t *e = ((apr_table_entry_t *)
+ apr_table_elts(t)->elts) + mpxs_apr_table_iterix(tsv)++;
+ STRLEN len = strlen(e->key);
+ SV *sv = newSV(0);
+
+ /* XXX: really nasty hack: set the numeric value of the key
+ * to represent a pointer to the corresponding val.
+ * We mark SvEND with another copy of the val's address
+ * as a means of confirming SvIVX really repesents a
+ * pointer.
+ */
+ SvUPGRADE(sv, SVt_PVIV);
+ SvGROW(sv, len + 3*sizeof(IV) + 1);
+ memcpy(SvPVX(sv), e->key, len);
+ SvCUR_set(sv, len);
+ SvEND(sv)[0] = 0;
+ SvIVX(sv) = (IV) e->val;
+ ((IV *)SvEND(sv))[1] = SvIVX(sv) = (IV) e->val;
+ SvPOK_on(sv);
+ SvIOK_on(sv);
- return NULL;
+ return sv;
+ }
+ mpxs_apr_table_iterix(tsv) = 0; /* done */
+ return &PL_sv_undef;
}
-static MP_INLINE const char *mpxs_APR__Table_FIRSTKEY(pTHX_ SV *tsv)
+static MP_INLINE SV *mpxs_APR__Table_FIRSTKEY(pTHX_ SV *tsv)
{
mpxs_apr_table_iterix(tsv) = 0; /* reset iterator index */
-
return mpxs_APR__Table_NEXTKEY(aTHX_ tsv, Nullsv);
}
@@ -164,4 +184,30 @@
}
});
+}
+
+static MP_INLINE
+const char *mpxs_APR__Table_FETCH(pTHX_ SV *tsv, SV *sv)
+{
+ apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
+ MAGIC *mg;
+ if (!t) {
+ return NULL;
+ }
+
+ /* XXX: really nasty hack, part 2: check for
+ * an SV coming from mpxs_APR_TABLE_NEXT. If
+ * it is, we take the return value directly
+ * from SvIVX.
+ */
+
+ if (SvPOK(sv) && SvIOK(sv) &&
+ SvLEN(sv) == SvCUR(sv) + 3*sizeof(IV) + 1 &&
+ ((IV *)SvEND(sv))[1] == SvIVX(sv))
+ {
+ return (const char *)SvIVX(sv);
+ }
+ else {
+ return apr_table_get(t, SvPV_nolen(sv));
+ }
}
Index: xs/maps/apr_functions.map
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/maps/apr_functions.map,v
retrieving revision 1.52
diff -u -r1.52 apr_functions.map
--- xs/maps/apr_functions.map 15 Apr 2003 08:39:52 -0000 1.52
+++ xs/maps/apr_functions.map 6 Jun 2003 04:07:34 -0000
@@ -246,10 +246,11 @@
-apr_table_setn
apr_table_unset
-apr_table_vdo
- const char *:DEFINE_FETCH | | apr_table_t *:t, const char *:key
+#const char *:DEFINE_FETCH | | apr_table_t *:t, const char *:key
void:DEFINE_STORE | | apr_table_t *:t, const char *:key, const char *:value
void:DEFINE_DELETE | | apr_table_t *:t, const char *:key
void:DEFINE_CLEAR | | apr_table_t *:t
+ mpxs_APR__Table_FETCH
mpxs_APR__Table_FIRSTKEY
mpxs_APR__Table_NEXTKEY
mpxs_APR__Table_EXISTS
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v
retrieving revision 1.116
diff -u -r1.116 FunctionTable.pm
--- xs/tables/current/ModPerl/FunctionTable.pm 4 Jun 2003 16:50:38 -0000 1.116
+++ xs/tables/current/ModPerl/FunctionTable.pm 6 Jun 2003 04:07:39 -0000
@@ -4958,7 +4958,7 @@
]
},
{
- 'return_type' => 'const char *',
+ 'return_type' => 'SV *',
'name' => 'mpxs_APR__Table_FIRSTKEY',
'attr' => [
'static',
@@ -4977,6 +4977,28 @@
},
{
'return_type' => 'const char *',
+ 'name' => 'mpxs_APR__Table_FETCH',
+ 'attr' => [
+ 'static',
+ '__inline__'
+ ],
+ 'args' => [
+ {
+ 'type' => 'PerlInterpreter *',
+ 'name' => 'my_perl'
+ },
+ {
+ 'type' => 'SV *',
+ 'name' => 'tsv'
+ },
+ {
+ 'type' => 'SV *',
+ 'name' => 'key'
+ }
+ ]
+ },
+ {
+ 'return_type' => 'SV *',
'name' => 'mpxs_APR__Table_NEXTKEY',
'attr' => [
'static',
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
>
>>The solution I see is to store the id in the key, so when FETCH is
>>called in the SCALAR context it checks whether the key has an id and
>>uses it to get the appropriate value. Otherwise it calls
>>apr_table_get(). Does this work?
>
>
> It just might. Try fiddling with NEXTKEY and FETCH,
> maybe changing the "char *" prototypes to "SV *", and
> see if you can get them to communicate with one another.
I'd hate to take the joy of adding these wonderful features away from
Philippe, who's now busy wrestling with mod_perl 1.28 release, but once done
will certainly love to do those. Right Philippe?
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
> The solution I see is to store the id in the key, so when FETCH is
> called in the SCALAR context it checks whether the key has an id and
> uses it to get the appropriate value. Otherwise it calls
> apr_table_get(). Does this work?
It just might. Try fiddling with NEXTKEY and FETCH,
maybe changing the "char *" prototypes to "SV *", and
see if you can get them to communicate with one another.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
>
>>Check this out. It seems to work now. Here is a complete patch.
>>You will have to rebuild the whole thing, because of the API change.
>
>
> [...]
>
>
>>+static MP_INLINE
>>+const char *mpxs_APR__Table_FETCH(pTHX_ SV *tsv, const char *key)
>>+{
>>+ apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
>>+
>>+ if (!t) {
>>+ return "";
>>+ }
>>+
>>+ if (!mpxs_apr_table_iterix(tsv)) {
>>+ return apr_table_get(t, key);
>>+ }
>>+ else {
>>+ const apr_array_header_t *arr = apr_table_elts(t);
>>+ apr_table_entry_t *elts = (apr_table_entry_t *)arr->elts;
>>+ return elts[mpxs_apr_table_iterix(tsv)-1].val;
>>+ }
>> }
>
>
> Although I haven't tested the patch, I still don't think this is a
> good approach. The problem is that this introduces "state" into
> FETCH, and normal perl hashes don't have this. Look at this
> (albeit contrived) example:
>
> # do something to each entry until the "foo" key is encountered
>
> sub foo_do {
> my $t = shift;
> my ($k, $v) = each %$t;
> return if not defined $k or $k eq "foo";
>
> # ...do stuff here...
> }
>
> foo_do($t);
>
> my $bar = $t->{bar}; # do we get foo's value here, or bar's value ?
>
> If the "foo" key is ever present in $t, the iterator state (SvCUR)
> for $t will remain positive, so a subsequent normal FETCH will
> not produce accurate results. $bar will likely wind up holding foo's
> value here.
>
> With a normal hash, there's no problem here because perl calls
> hv_iterval (not hv_fetch) when it wants to do an "iterative" fetch.
>
> It seems to me that the TIEHASH API isn't as flexible as it needs to
> be- IMO it _should_ call FIRST_KEY and NEXT_KEY in list context
> when it needs to produce each(%$t) (or values (%$t)) in a list
> context (of course, it doesn't- all TIE methods are called in
> scalar context by the Perl_magic_*pack functions in mg.c.
That's true. My "fix" takes it a bit further, but as your example shows it
introduces the state problem. The solution I see is to store the id in the
key, so when FETCH is called in the SCALAR context it checks whether the key
has an id and uses it to get the appropriate value. Otherwise it calls
apr_table_get(). Does this work? This is in addition to the existing each/keys
solution.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
> Check this out. It seems to work now. Here is a complete patch.
> You will have to rebuild the whole thing, because of the API change.
[...]
> +static MP_INLINE
> +const char *mpxs_APR__Table_FETCH(pTHX_ SV *tsv, const char *key)
> +{
> + apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
> +
> + if (!t) {
> + return "";
> + }
> +
> + if (!mpxs_apr_table_iterix(tsv)) {
> + return apr_table_get(t, key);
> + }
> + else {
> + const apr_array_header_t *arr = apr_table_elts(t);
> + apr_table_entry_t *elts = (apr_table_entry_t *)arr->elts;
> + return elts[mpxs_apr_table_iterix(tsv)-1].val;
> + }
> }
Although I haven't tested the patch, I still don't think this is a
good approach. The problem is that this introduces "state" into
FETCH, and normal perl hashes don't have this. Look at this
(albeit contrived) example:
# do something to each entry until the "foo" key is encountered
sub foo_do {
my $t = shift;
my ($k, $v) = each %$t;
return if not defined $k or $k eq "foo";
# ...do stuff here...
}
foo_do($t);
my $bar = $t->{bar}; # do we get foo's value here, or bar's value ?
If the "foo" key is ever present in $t, the iterator state (SvCUR)
for $t will remain positive, so a subsequent normal FETCH will
not produce accurate results. $bar will likely wind up holding foo's
value here.
With a normal hash, there's no problem here because perl calls
hv_iterval (not hv_fetch) when it wants to do an "iterative" fetch.
It seems to me that the TIEHASH API isn't as flexible as it needs to
be- IMO it _should_ call FIRST_KEY and NEXT_KEY in list context
when it needs to produce each(%$t) (or values (%$t)) in a list
context (of course, it doesn't- all TIE methods are called in
scalar context by the Perl_magic_*pack functions in mg.c.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
>
>>so we can't simply alias FETCH to apr_table_get, the following sort
>>of works (but breaks some other things):
>>
>>static MP_INLINE
>>const char *mpxs_APR__Table_FETCH(pTHX_ SV *tsv, const char *key)
>>{
>> apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
>>
>> if (!t) {
>> return "";
>> }
>>
>> if (!mpxs_apr_table_iterix(tsv)) {
>> return apr_table_get(t, key);
>> }
>> else {
>> const apr_array_header_t *arr = apr_table_elts(t);
>> apr_table_entry_t *elts = (apr_table_entry_t *)arr->elts;
>> return elts[mpxs_apr_table_iterix(tsv)-1].val;
>> }
>>}
>>
>>I'm not sure if it's a good idea though.
>
>
> Probably not as it is- testing mpxs_apr_table_iterix(tsv)
> isn't what you want. I think you really need to know something
> about the calling context (iterator call or actual lookup?),
> but I'm not sure if perl provides enough info.
>
> It might be easiest to just change the comment at the bottom
> of the docs, and just recommend do() for iterating over
> the table values. Short of that, someone might instead wrap
> apr_table_elts() as an array. For apreq, I'd just be concerned
> about how the value attribute of apr_table_entry_t were
> typemapped.
Check this out. It seems to work now. Here is a complete patch. You will have
to rebuild the whole thing, because of the API change.
Index: t/response/TestAPR/table.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/response/TestAPR/table.pm,v
retrieving revision 1.5
diff -u -r1.5 table.pm
--- t/response/TestAPR/table.pm 11 Apr 2002 11:08:44 -0000 1.5
+++ t/response/TestAPR/table.pm 5 Jun 2003 09:33:04 -0000
@@ -15,7 +15,7 @@
sub handler {
my $r = shift;
- plan $r, tests => 17;
+ plan $r, tests => 23;
my $table = APR::Table::make($r->pool, $TABLE_SIZE);
@@ -35,6 +35,14 @@
$array[1] eq 'tar' &&
$array[2] eq 'kar';
+ my $c = 0;
+ while (my ($a, $b) = each %$table) {
+ warn ("$a $b\n");
+ ok $a eq 'foo';
+ ok $b eq $array[$c];
+ $c++;
+ }
+
ok $table->unset('foo') || 1;
ok not defined $table->get('foo');
@@ -105,7 +113,7 @@
my ($key,$value) = @_;
$filter_count++;
unless ($key eq chr($value+97)) {
- die "arguments I received are bogus($key,$value)";
+ die "arguments I received are bogus($key,$value)".chr($value+97);
}
return 1;
}
Index: xs/APR/Table/APR__Table.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/Table/APR__Table.h,v
retrieving revision 1.10
diff -u -r1.10 APR__Table.h
--- xs/APR/Table/APR__Table.h 4 Jun 2003 02:31:56 -0000 1.10
+++ xs/APR/Table/APR__Table.h 5 Jun 2003 09:33:05 -0000
@@ -1,4 +1,4 @@
-#define mpxs_APR__Table_FETCH apr_table_get
+//#define mpxs_APR__Table_FETCH mpxs_apr_table_FETCH1
#define mpxs_APR__Table_STORE apr_table_set
#define mpxs_APR__Table_DELETE apr_table_unset
#define mpxs_APR__Table_CLEAR apr_table_clear
@@ -117,6 +117,7 @@
return mpxs_apr_table_nextkey(t, tsv);
}
+ mpxs_apr_table_iterix(tsv) = 0; /* done */
return NULL;
}
@@ -145,7 +146,8 @@
if (GIMME_V == G_SCALAR) {
const char *val = apr_table_get(t, key);
-
+ fprintf(stderr, "SCALAR CONTEXT\n");
+
if (val) {
XPUSHs(sv_2mortal(newSVpv((char*)val, 0)));
}
@@ -154,6 +156,7 @@
const apr_array_header_t *arr = apr_table_elts(t);
apr_table_entry_t *elts = (apr_table_entry_t *)arr->elts;
int i;
+ fprintf(stderr, "LIST CONTEXT\n");
for (i = 0; i < arr->nelts; i++) {
if (!elts[i].key || strcasecmp(elts[i].key, key)) {
@@ -164,4 +167,23 @@
}
});
+}
+
+static MP_INLINE
+const char *mpxs_APR__Table_FETCH(pTHX_ SV *tsv, const char *key)
+{
+ apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
+
+ if (!t) {
+ return "";
+ }
+
+ if (!mpxs_apr_table_iterix(tsv)) {
+ return apr_table_get(t, key);
+ }
+ else {
+ const apr_array_header_t *arr = apr_table_elts(t);
+ apr_table_entry_t *elts = (apr_table_entry_t *)arr->elts;
+ return elts[mpxs_apr_table_iterix(tsv)-1].val;
+ }
}
Index: xs/maps/apr_functions.map
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/maps/apr_functions.map,v
retrieving revision 1.52
diff -u -r1.52 apr_functions.map
--- xs/maps/apr_functions.map 15 Apr 2003 08:39:52 -0000 1.52
+++ xs/maps/apr_functions.map 5 Jun 2003 09:33:05 -0000
@@ -246,10 +246,11 @@
-apr_table_setn
apr_table_unset
-apr_table_vdo
- const char *:DEFINE_FETCH | | apr_table_t *:t, const char *:key
+# const char *:DEFINE_FETCH | | apr_table_t *:t, const char *:key
void:DEFINE_STORE | | apr_table_t *:t, const char *:key, const char *:value
void:DEFINE_DELETE | | apr_table_t *:t, const char *:key
void:DEFINE_CLEAR | | apr_table_t *:t
+ mpxs_APR__Table_FETCH
mpxs_APR__Table_FIRSTKEY
mpxs_APR__Table_NEXTKEY
mpxs_APR__Table_EXISTS
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v
retrieving revision 1.116
diff -u -r1.116 FunctionTable.pm
--- xs/tables/current/ModPerl/FunctionTable.pm 4 Jun 2003 16:50:38 -0000 1.116
+++ xs/tables/current/ModPerl/FunctionTable.pm 5 Jun 2003 09:33:05 -0000
@@ -4977,6 +4977,28 @@
},
{
'return_type' => 'const char *',
+ 'name' => 'mpxs_APR__Table_FETCH',
+ 'attr' => [
+ 'static',
+ '__inline__'
+ ],
+ 'args' => [
+ {
+ 'type' => 'PerlInterpreter *',
+ 'name' => 'my_perl'
+ },
+ {
+ 'type' => 'SV *',
+ 'name' => 'tsv'
+ },
+ {
+ 'type' => 'const char *',
+ 'name' => 'key'
+ }
+ ]
+ },
+ {
+ 'return_type' => 'const char *',
'name' => 'mpxs_APR__Table_NEXTKEY',
'attr' => [
'static',
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
> so we can't simply alias FETCH to apr_table_get, the following sort
> of works (but breaks some other things):
>
> static MP_INLINE
> const char *mpxs_APR__Table_FETCH(pTHX_ SV *tsv, const char *key)
> {
> apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
>
> if (!t) {
> return "";
> }
>
> if (!mpxs_apr_table_iterix(tsv)) {
> return apr_table_get(t, key);
> }
> else {
> const apr_array_header_t *arr = apr_table_elts(t);
> apr_table_entry_t *elts = (apr_table_entry_t *)arr->elts;
> return elts[mpxs_apr_table_iterix(tsv)-1].val;
> }
> }
>
> I'm not sure if it's a good idea though.
Probably not as it is- testing mpxs_apr_table_iterix(tsv)
isn't what you want. I think you really need to know something
about the calling context (iterator call or actual lookup?),
but I'm not sure if perl provides enough info.
It might be easiest to just change the comment at the bottom
of the docs, and just recommend do() for iterating over
the table values. Short of that, someone might instead wrap
apr_table_elts() as an array. For apreq, I'd just be concerned
about how the value attribute of apr_table_entry_t were
typemapped.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
>
>>Joe Schaefer wrote:
>>
>>>Stas Bekman <st...@stason.org> writes:
>>>
>>>
>>>>Hmm, what about storing the pointer to apreq_value_t as val
>>>
>>>AFAICT, that helps nada.
>>
>>true, again because of newSVpv
>>
>>
>>>>in the table and subclass get() to do what you want?
>>>
>>>Just subclassing get() might work! At the moment
>>>I'm concerned about how each() is implemented; does
>>>each make a call to FETCH to get at the value? If so, how does it
>>>handle multivalued keys?
>>
>> From the perltie manpage:
>>
>> FIRSTKEY and NEXTKEY implement the keys() and each() functions
>> to iterate over all the keys.
>
>
> Unfortunately that's a borked approach when multivalued
> keys are present: values() and each() (in list context)
> will always grab the first value instead of the
> "current" one. The remark at the bottom of docs/api/APR/Table.pod
> appears to be incorrect:
>
> ... The only exception to this is if you iterate over the list with
> I<each>, then you can access all key-value pairs that share the same key.
>
> See test below (tests 8 & 10 check the above assertion, and fail for me).
>
>
>>looks like mpxs_APR__Table_NEXTKEY is correct, though we don't really
>>have a test in t/response/TestAPR/table.pm for this.
>
>
> It looks ok to me also- too bad the TIEHASH api doesn't have
> EACH & VALUES subs.
>
> Index: t/response/TestAPR/table.pm
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/t/response/TestAPR/table.pm,v
> retrieving revision 1.5
> diff -u -r1.5 table.pm
> --- t/response/TestAPR/table.pm 11 Apr 2002 11:08:44 -0000 1.5
> +++ t/response/TestAPR/table.pm 5 Jun 2003 06:57:28 -0000
> @@ -15,7 +15,7 @@
> sub handler {
> my $r = shift;
>
> - plan $r, tests => 17;
> + plan $r, tests => 23;
>
> my $table = APR::Table::make($r->pool, $TABLE_SIZE);
>
> @@ -34,6 +34,12 @@
> $array[0] eq 'bar' &&
> $array[1] eq 'tar' &&
> $array[2] eq 'kar';
> +
> + for (0..2) {
> + ($a, $b) = each %$table;
> + ok $a eq 'foo';
> + ok $b eq $array[$_];
> + }
>
> ok $table->unset('foo') || 1;
>
correct it's not working.
so we can't simply alias FETCH to apr_table_get, the following sort of works
(but breaks some other things):
static MP_INLINE
const char *mpxs_APR__Table_FETCH(pTHX_ SV *tsv, const char *key)
{
apr_table_t *t = mp_xs_sv2_APR__Table(tsv);
if (!t) {
return "";
}
if (!mpxs_apr_table_iterix(tsv)) {
return apr_table_get(t, key);
}
else {
const apr_array_header_t *arr = apr_table_elts(t);
apr_table_entry_t *elts = (apr_table_entry_t *)arr->elts;
return elts[mpxs_apr_table_iterix(tsv)-1].val;
}
}
I'm not sure if it's a good idea though.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
> Joe Schaefer wrote:
> > Stas Bekman <st...@stason.org> writes:
> >
> >> Hmm, what about storing the pointer to apreq_value_t as val
> > AFAICT, that helps nada.
>
> true, again because of newSVpv
>
> >>in the table and subclass get() to do what you want?
> > Just subclassing get() might work! At the moment
> > I'm concerned about how each() is implemented; does
> > each make a call to FETCH to get at the value? If so, how does it
> > handle multivalued keys?
>
> From the perltie manpage:
>
> FIRSTKEY and NEXTKEY implement the keys() and each() functions
> to iterate over all the keys.
Unfortunately that's a borked approach when multivalued
keys are present: values() and each() (in list context)
will always grab the first value instead of the
"current" one. The remark at the bottom of docs/api/APR/Table.pod
appears to be incorrect:
... The only exception to this is if you iterate over the list with
I<each>, then you can access all key-value pairs that share the same key.
See test below (tests 8 & 10 check the above assertion, and fail for me).
> looks like mpxs_APR__Table_NEXTKEY is correct, though we don't really
> have a test in t/response/TestAPR/table.pm for this.
It looks ok to me also- too bad the TIEHASH api doesn't have
EACH & VALUES subs.
Index: t/response/TestAPR/table.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/t/response/TestAPR/table.pm,v
retrieving revision 1.5
diff -u -r1.5 table.pm
--- t/response/TestAPR/table.pm 11 Apr 2002 11:08:44 -0000 1.5
+++ t/response/TestAPR/table.pm 5 Jun 2003 06:57:28 -0000
@@ -15,7 +15,7 @@
sub handler {
my $r = shift;
- plan $r, tests => 17;
+ plan $r, tests => 23;
my $table = APR::Table::make($r->pool, $TABLE_SIZE);
@@ -34,6 +34,12 @@
$array[0] eq 'bar' &&
$array[1] eq 'tar' &&
$array[2] eq 'kar';
+
+ for (0..2) {
+ ($a, $b) = each %$table;
+ ok $a eq 'foo';
+ ok $b eq $array[$_];
+ }
ok $table->unset('foo') || 1;
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
>
>>Hmm, what about storing the pointer to apreq_value_t as val
>
>
> AFAICT, that helps nada.
true, again because of newSVpv
>>in the table and subclass get() to do what you want?
>
>
> Just subclassing get() might work! At the moment
> I'm concerned about how each() is implemented; does
> each make a call to FETCH to get at the value? If
> so, how does it handle multivalued keys?
From the perltie manpage:
FIRSTKEY and NEXTKEY implement the keys() and each() functions
to iterate over all the keys.
looks like mpxs_APR__Table_NEXTKEY is correct, though we don't really have a
test in t/response/TestAPR/table.pm for this.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
> Hmm, what about storing the pointer to apreq_value_t as val
AFAICT, that helps nada.
> in the table and subclass get() to do what you want?
Just subclassing get() might work! At the moment
I'm concerned about how each() is implemented; does
each make a call to FETCH to get at the value? If
so, how does it handle multivalued keys?
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Hmm, what about storing the pointer to apreq_value_t as val in the table and
subclass get() to do what you want?
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
>
>>>I still don't see how that's useful here. There's nothing wrong with the
>>>data inside apr_table. It's the c2perl
>>>translation (newSVpv) that's causing the lossage.
>>
>>and I don't get where the lossage is :(
>
>
> OK- I see we're having a disconnect. Let me try to draw a picture
> of an apr_table_entry_t that's coming from an apreq-2 parser:
>
> ATTRIBUTE STORAGE (BYTES)
> table_entry: key -------------------+
> |
> v
> | f | o | o | \0
>
> val -------------------+
> |
> |
> v
> | 12345 | 0 | 8 | b | a | r | \0 | n | o | n | e | \0
> ^ ^ ^ ^
> | | | |
> name status size data
>
>
> Notice that val points to the end of an apreq_value_t struct; what's
> important here is that the location of the metadata (ie. the rest of
> the apreq_value_t struct) is directly in front of *val.
Thanks for the drawing. Now I don't understand how do you get to the
apreq_value_t pointer from table_entry->val. Do you just subtract the size of
the apreq_value_t struct and based on the knowledge that the memory allocation
is continues, you magically retrieve the whole struct? I think I understand
the trick now. it wasn't obvious at all ;)
and since newSVpv copies the string, you no longer can get to the beginning of
the struct. Now I understand why you wanted to mark it as SvFAKE (that would
prevent the copy).
Now that I understand this thing I can think intelligently ;) For example how
about making SV into a dual-var, ala $!. it might not work if IV slot is used
to store the original pointer, since using val in the numeric context will
need to make use of this field. Same with NV? I'm just thinking how can we
save the overhead of messing with MAGIC if we can.
So you say that due to the bug we can't sub-class get() to do newSVpv while
using the pointer to apreq_value_t and then setting SvCUR to the real string?
>>That's cool. But Joe, you don't seem to understand what I'm asking you
>>about. I want to see an example of a conversion function where this
>>meta data kicks in.
>
>
> I don't know if I've said enough to answer your question, but
> I hope I've explained the problem with newSVpv well enough.
> Short of coding up an entire APR::Table subclass, can you be
> more specific about what you'd like to see?
>
>
>>I don't seem to get into your head to see things the same way you do.
>
>
> Dreadfully laconic, I know. Sorry about that.
> My thesis advisor used to say the same thing :-/.
I think that I'm game now ;) The drawing made things clear.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
> > I still don't see how that's useful here. There's nothing wrong with the
> > data inside apr_table. It's the c2perl
> > translation (newSVpv) that's causing the lossage.
>
> and I don't get where the lossage is :(
OK- I see we're having a disconnect. Let me try to draw a picture
of an apr_table_entry_t that's coming from an apreq-2 parser:
ATTRIBUTE STORAGE (BYTES)
table_entry: key -------------------+
|
v
| f | o | o | \0
val -------------------+
|
|
v
| 12345 | 0 | 8 | b | a | r | \0 | n | o | n | e | \0
^ ^ ^ ^
| | | |
name status size data
Notice that val points to the end of an apreq_value_t struct; what's
important here is that the location of the metadata (ie. the rest of
the apreq_value_t struct) is directly in front of *val.
So what happens when newSVpv copies *val? It ignores the metadata,
so we wind up with unusable (segfaultable) garbage in front of SvPVX:
SvPVX(sv) ----------------------------+
|
|
v
*** apreq_value_t is lost *** | b | a | r | \0
^^^^^^^^^^^^^^^^^^
rest of string is missing
The original metadata isn't lost, it's just inaccessible through the
APR::Table API. But by adding magic to the SV, so that mg->mg_ptr
points at the original C-string pointed at by the apr_table_entry_t's
val pointer, the full metadata can be recovered (via the conversion
macros in apreq.h, e.g. apreq_strtoval()), including the remainder
of the string that newSVpv lopped off.
[...]
> That's cool. But Joe, you don't seem to understand what I'm asking you
> about. I want to see an example of a conversion function where this
> meta data kicks in.
I don't know if I've said enough to answer your question, but
I hope I've explained the problem with newSVpv well enough.
Short of coding up an entire APR::Table subclass, can you be
more specific about what you'd like to see?
> I don't seem to get into your head to see things the same way you do.
Dreadfully laconic, I know. Sorry about that.
My thesis advisor used to say the same thing :-/.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Fwd: Re: [rfc] APR::Table & polymorphic values]
Posted by Stas Bekman <st...@stason.org>.
>>Joe Schaefer wrote:
>>>>What if you use mpxs_apr_table_do to fixup the elements on the first
>>>>FETCH? Would that be a bad approach?
>>>
>>>Not sure what you have in mind.
>>
>>you can run filter on all pairs in the table, see
>>t/response/TestAPR/table.pm
>
>
> I still don't see how that's useful here. There's nothing
> wrong with the data inside apr_table. It's the c2perl
> translation (newSVpv) that's causing the lossage.
and I don't get where the lossage is :(
>>>>The problem is that I still don't understand what do you need that
>>>>meta data for and how is it used.
>>>
>>> 1) the perl-glue for apreq will finally be able to handle embedded
>>> nul characters,
>>> 2) we can duplicate CGI.pm's overloading of params, since file-uploads
>>> are just params with a non-NULL bucket-brigade attached.
>>> 3) we can do lots of cool stuff with cookie values, too.
>>
>>Any examples of how is it used?
>
>
> Nothing solid API-wise, since the perl API needs to be discussed on
> apreq-dev. But given all the recent confusion on modperl@ over the
> Cookie API, there's good reason to simplify the API. What I'd like to
> do is push the idea of params & cookies of being _values_ with _objects_
> attached to them, instead of the other way around (which is what we
> do in apreq-1). So for example, if you want to get at the 'foo'
> cookie, I'd rather write something like
>
> my $jar = Apacke::Cookie->fetch($r); # ref(jar) = Apache::Cookie::Table
> my $foo = $jar->{'foo'}; # $cookie_obj = tied $foo;
>
> instead of the current monstrosity:
>
> my $jar = Apache::Cookie->fetch;
> my $cookie_obj = $jar->{'foo'};
> my $foo = $cookie && $cookie_obj->value;
>
> The '&&' test is both necessary and really fugly.
>
> [...]
That's cool. But Joe, you don't seem to understand what I'm asking you about.
I want to see an example of a conversion function where this meta data kicks
in. I don't seem to get into your head to see things the same way you do.
[...]
>>I can't see where the meta data is in str. Is it before str?
>
>
> Yes, the metadata is ALWAYS right before str! str is
> pointer to the _end_ of an apreq struct
> (str == param->v.data or str == cookie->v.data).
>
> Patch attached- hopefully this will clarify the issues. Please comment.
I can't intelligently comment on the patch, before I can understand why is it
needed :(
One thing I can tell is that it won't work with ithreads-disabled perl.
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org