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 2001/11/19 02:55:34 UTC
Re: cvs commit: modperl-2.0/src/modules/perl modperl_config.c
dougm@apache.org wrote:
> Index: modperl_config.c
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_config.c,v
> - else if ((val = (char *)apr_table_get(base, entries[i].key))){
> - apr_table_set(merge, entries[i].key, val);
> + else {
> + /*XXX: should setn() be addn()for PerlSetVar?
> + * since we have PerlAddVar, there may be multiple values.
> + */
> + apr_table_setn(merge, entries[i].key, entries[i].val);
yes, that's a bug, must be addn()
--
_____________________________________________________________________
Stas Bekman JAm_pH -- Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide http://perl.apache.org/guide
mailto:stas@stason.org http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/src/modules/perl modperl_config.c
Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:
> ok, its the mixing of Add and Set directives that i was missing. i agree
> something needs to be fixed, but i don't think 2.0 is fixing it the
> right way. the main problem i have is that current 2.0 breaks this:
>
> Add A 1
>
> <Location /foo>
> Add A 2
> </Location>
>
> inside foo, 1.x would get both 1 and 2, 2.0 only gets 2. that does not
> seem like a proper merge to me.
Yes, I was just thinking about this too.
> and the reason i was looking closely again is that apr_table_overlay is
> much better performance wise than the current modperl_table_overlap.
Absolutely.
> i have an idea that would 'fix' the Add + Set problem by modifying the
> parsed config syntax tree before it is processed, then we can go back to
> using apr_table_overlay().
looking forward for the details/commit ;)
_____________________________________________________________________
Stas Bekman JAm_pH -- Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide http://perl.apache.org/guide
mailto:stas@stason.org http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/src/modules/perl modperl_config.c
Posted by Doug MacEachern <do...@covalent.net>.
ok, its the mixing of Add and Set directives that i was missing. i agree
something needs to be fixed, but i don't think 2.0 is fixing it the
right way. the main problem i have is that current 2.0 breaks this:
Add A 1
<Location /foo>
Add A 2
</Location>
inside foo, 1.x would get both 1 and 2, 2.0 only gets 2. that does not
seem like a proper merge to me.
and the reason i was looking closely again is that apr_table_overlay is
much better performance wise than the current modperl_table_overlap.
i have an idea that would 'fix' the Add + Set problem by modifying the
parsed config syntax tree before it is processed, then we can go back to
using apr_table_overlay().
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/src/modules/perl modperl_config.c
Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:
> On Tue, 20 Nov 2001, Stas Bekman wrote:
>
>
>>which as we have discussed is not what we want, since we don't want all
>>the data from the two. The overlay structure should override the base
>>structure if the same key exists in both. I guess I should put a better
>>comment in place.
>>
>
> ok, remind me why we don't want all the data from the two then.
> or let me ask differently: why do we want to be different from 1.x?
> and most important: how do you know this will not break 1.x code?
>
> seems to me PerlAddVars should be merged the same as they are in 1.x.
> if you're only using PerlSetVar, add will still override base, since the
> add entry is there first, which is what scalar $r->dir_config->get('key')
> will return.
I said:
http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=100169675916552&w=2
You replied:
http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=100169883124652&w=2
The whole thread is here:
http://marc.theaimsgroup.com/?t=100167436200003&w=2&r=1
The gist of things is that 1.x has a broken Perl(Set|Add)Var
merging/overriding logic and we better fix it, rather than keep it.
_____________________________________________________________________
Stas Bekman JAm_pH -- Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide http://perl.apache.org/guide
mailto:stas@stason.org http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/src/modules/perl modperl_config.c
Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 20 Nov 2001, Stas Bekman wrote:
> which as we have discussed is not what we want, since we don't want all
> the data from the two. The overlay structure should override the base
> structure if the same key exists in both. I guess I should put a better
> comment in place.
ok, remind me why we don't want all the data from the two then.
or let me ask differently: why do we want to be different from 1.x?
and most important: how do you know this will not break 1.x code?
seems to me PerlAddVars should be merged the same as they are in 1.x.
if you're only using PerlSetVar, add will still override base, since the
add entry is there first, which is what scalar $r->dir_config->get('key')
will return.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/src/modules/perl modperl_config.c
Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:
> On Tue, 20 Nov 2001, Stas Bekman wrote:
>
>
>>exactly for the same reason that addn() should be used.
>>apr_table_overlay uses setn() which will loose all pairs with the same
>>key but the last one added.
>>
>>apr_table_overlay:
>> * 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);
>> ^^^^^^^^^^^^^^^
>> * }
>> * }
>>
>
> that is not apr_table_overlay(), that is apr_table_overlap().
> and the current version of overlap uses addn().
>
> i think we should be using apr_table_overlay() just as 1.x does, no?
oops, still waking up. The reason not to use apr_table_overlay() is:
/**
* Merge two tables into one new table
* @param p The pool to use for the new table
* @param overlay The first table to put in the new table
* @param base The table to add at the end of the new table
* @return A new table containing all of the data from the two passed in
^^^^^^^^^^^^^^^^^^^^^^^^^^
*/
APR_DECLARE(apr_table_t *) apr_table_overlay(apr_pool_t *p,
const apr_table_t *overlay,
const apr_table_t *base);
which as we have discussed is not what we want, since we don't want all
the data from the two. The overlay structure should override the base
structure if the same key exists in both. I guess I should put a better
comment in place.
_____________________________________________________________________
Stas Bekman JAm_pH -- Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide http://perl.apache.org/guide
mailto:stas@stason.org http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/src/modules/perl modperl_config.c
Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 20 Nov 2001, Stas Bekman wrote:
> exactly for the same reason that addn() should be used.
> apr_table_overlay uses setn() which will loose all pairs with the same
> key but the last one added.
>
> apr_table_overlay:
> * 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);
> ^^^^^^^^^^^^^^^
> * }
> * }
that is not apr_table_overlay(), that is apr_table_overlap().
and the current version of overlap uses addn().
i think we should be using apr_table_overlay() just as 1.x does, no?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/src/modules/perl modperl_config.c
Posted by Stas Bekman <st...@stason.org>.
Doug MacEachern wrote:
> On Mon, 19 Nov 2001, Stas Bekman wrote:
>
>
>>yes, that's a bug, must be addn()
>>
>
> remind me why we don't just use apr_table_overlay like 1.x does for SetVar?
exactly for the same reason that addn() should be used.
apr_table_overlay uses setn() which will loose all pairs with the same
key but the last one added.
apr_table_overlay:
* 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);
^^^^^^^^^^^^^^^
* }
* }
of course this function should be better moved to APR.
_____________________________________________________________________
Stas Bekman JAm_pH -- Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide http://perl.apache.org/guide
mailto:stas@stason.org http://ticketmaster.com http://apacheweek.com
http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/src/modules/perl modperl_config.c
Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 19 Nov 2001, Stas Bekman wrote:
> yes, that's a bug, must be addn()
remind me why we don't just use apr_table_overlay like 1.x does for SetVar?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org