You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by "Philippe M. Chiasson" <go...@cpan.org> on 2004/02/13 00:15:38 UTC
Re: cvs commit: modperl-2.0/xs/Apache/RequestRec
Apache__RequestRec.h
On Thu, 2004-02-12 at 23:06 +0000, geoff@apache.org wrote:
> geoff 2004/02/12 15:06:24
>
> Modified: . Changes
> lib/ModPerl Code.pm
> src/modules/perl mod_perl.c modperl_callback.c modperl_env.c
> modperl_env.h
> t/hooks/TestHooks headerparser.pm
> t/modperl cookie.t
> t/response/TestDirective env.pm
> t/response/TestModperl cookie.pm
> xs/Apache/RequestRec Apache__RequestRec.h
> Added: t/modperl setupenv.t
> t/response/TestModperl setupenv.pm
> Log:
> standard %ENV population with CGI variables and contents of the
> subprocess_env table (such as SetEnv and PassEnv) has been delayed
> until the last possible moment before content-generation runs.
> PerlSetEnv and PerlPassEnv are each an exception to this and are
> placed in both %ENV and the subprocess_env table immediately,
> regardless of the current [+-]SetupEnv setting.
>
> [...]
A few compilation problems were introduced by this one, see below.
> Index: mod_perl.c
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.c,v
> retrieving revision 1.206
> retrieving revision 1.207
> diff -u -r1.206 -r1.207
> --- mod_perl.c 10 Jan 2004 05:01:04 -0000 1.206
> +++ mod_perl.c 12 Feb 2004 23:06:23 -0000 1.207
> @@ -837,11 +837,40 @@
>
> int modperl_response_handler(request_rec *r)
> {
> + MP_dDCFG;
> + apr_status_t retval;
> +
> +#ifdef USE_ITHREADS
> + pTHX;
> + modperl_interp_t *interp;
> +#endif
> +
> if (!strEQ(r->handler, "modperl")) {
> return DECLINED;
> }
>
> - return modperl_response_handler_run(r, TRUE);
> + /* default is -SetupEnv, add if PerlOption +SetupEnv */
> + if (MpDirSETUP_ENV(dcfg)) {
> +#ifdef USE_ITHREADS
> + interp = modperl_interp_select(r, r->connection, r->server);
> + aTHX = interp->perl;
> +#endif
> +
> + modperl_env_request_populate(aTHX_ r);
> + }
> +
> + retval = modperl_response_handler_run(r, TRUE);
> +
> + if (MpDirSETUP_ENV(dcfg)) {
> +#ifdef USE_ITHREADS
> + if (MpInterpPUTBACK(interp)) {
> + /* PerlInterpScope handler */
> + modperl_interp_unselect(interp);
> + }
> +#endif
> + }
> +
> + return retval;
> }
>
> int modperl_response_handler_cgi(request_rec *r)
>
mod_perl.c: In function `modperl_response_handler':
mod_perl.c:845: warning: `interp' might be used uninitialized in this
function
>
> 1.69 +14 -4 modperl-2.0/src/modules/perl/modperl_callback.c
>
> Index: modperl_callback.c
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_callback.c,v
> retrieving revision 1.68
> retrieving revision 1.69
> diff -u -r1.68 -r1.69
> --- modperl_callback.c 9 Feb 2004 19:32:42 -0000 1.68
> +++ modperl_callback.c 12 Feb 2004 23:06:24 -0000 1.69
> @@ -198,15 +198,25 @@
> modperl_config_req_cleanup_register(r, rcfg);
>
> switch (type) {
> - case MP_HANDLER_TYPE_PER_DIR:
> case MP_HANDLER_TYPE_PER_SRV:
> modperl_handler_make_args(aTHX_ &av_args,
> "Apache::RequestRec", r, NULL);
>
> - /* only happens once per-request */
> - if (MpDirSETUP_ENV(dcfg)) {
> - modperl_env_request_populate(aTHX_ r);
> + /* per-server PerlSetEnv and PerlPassEnv - only once per-request */
> + if (! MpReqPERL_SET_ENV_SRV(rcfg)) {
> + modperl_env_configure_request_srv(aTHX_ r);
> + }
> +
> + break;
> + case MP_HANDLER_TYPE_PER_DIR:
> + modperl_handler_make_args(aTHX_ &av_args,
> + "Apache::RequestRec", r, NULL);
> +
> + /* per-directory PerlSetEnv - only once per-request */
> + if (! MpReqPERL_SET_ENV_DIR(rcfg)) {
> + modperl_env_configure_request_dir(aTHX_ r);
> }
> +
> break;
> case MP_HANDLER_TYPE_PRE_CONNECTION:
> case MP_HANDLER_TYPE_CONNECTION:
modperl_callback.c: In function `modperl_callback_run_handlers':
modperl_callback.c:206: warning: implicit declaration of function `MpReqPERL_SET_ENV_SRV'
modperl_callback.c:216: warning: implicit declaration of function `MpReqPERL_SET_ENV_DIR'
>
> 1.30 +98 -16 modperl-2.0/src/modules/perl/modperl_env.c
>
> Index: modperl_env.c
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_env.c,v
> retrieving revision 1.29
> retrieving revision 1.30
> diff -u -r1.29 -r1.30
> --- modperl_env.c 22 Sep 2003 23:29:52 -0000 1.29
> +++ modperl_env.c 12 Feb 2004 23:06:24 -0000 1.30
> @@ -143,10 +143,13 @@
>
> void modperl_env_configure_server(pTHX_ apr_pool_t *p, server_rec *s)
> {
> - /* XXX: propagate scfg->SetEnv to environ */
> MP_dSCFG(s);
> int i = 0;
>
> + /* make per-server PerlSetEnv and PerlPassEnv entries visible
> + * to %ENV at config time
> + */
> +
> for (i=0; MP_env_pass_defaults[i]; i++) {
> const char *key = MP_env_pass_defaults[i];
> char *val;
> @@ -180,18 +183,79 @@
> r->subprocess_env, \
> tab)
>
> -void modperl_env_configure_request(request_rec *r)
> +void modperl_env_configure_request_dir(pTHX_ request_rec *r)
> {
> + MP_dRCFG;
> MP_dDCFG;
> - MP_dSCFG(r->server);
> +
> + /* populate %ENV and r->subprocess_env with per-directory
> + * PerlSetEnv entries.
> + *
> + * note that per-server PerlSetEnv entries, as well as
> + * PerlPassEnv entries (which are only per-server), are added
> + * to %ENV and r->subprocess_env via modperl_env_configure_request_srv
> + */
>
> if (!apr_is_empty_table(dcfg->SetEnv)) {
> - overlay_subprocess_env(r, dcfg->SetEnv);
> + apr_table_t *setenv_copy;
> +
> + /* add per-directory PerlSetEnv entries to %ENV
> + * collisions with per-server PerlSetEnv entries are
> + * resolved via the nature of a Perl hash
> + */
> + MP_TRACE_e(MP_FUNC, "\n\t[%s/0x%lx/%s]"
> + "\n\t@ENV{keys dcfg->SetEnv} = values dcfg->SetEnv;",
> + modperl_pid_tid(r->pool), modperl_interp_address(aTHX),
> + modperl_server_desc(r->server, r->pool));
> + modperl_env_table_populate(aTHX_ dcfg->SetEnv);
> +
> + /* make sure the entries are in the subprocess_env table as well.
> + * we need to use apr_table_overlap (not apr_table_overlay) because
> + * r->subprocess_env might have per-server PerlSetEnv entries in it
> + * and using apr_table_overlay would generate duplicate entries.
> + * in order to use apr_table_overlap, though, we need to copy the
> + * the dcfg table so that pool requirements are satisfied */
> +
> + setenv_copy = apr_table_copy(r->pool, dcfg->SetEnv);
> + apr_table_overlap(r->subprocess_env, setenv_copy, APR_OVERLAP_TABLES_SET);
> + }
> +
> + MpReqPERL_SET_ENV_DIR_On(rcfg);
> +}
> +
> +void modperl_env_configure_request_srv(pTHX_ request_rec *r)
> +{
> + MP_dRCFG;
> + MP_dSCFG(r->server);
> +
> + /* populate %ENV and r->subprocess_env with per-server PerlSetEnv
> + * and PerlPassEnv entries.
> + *
> + * although both are setup in %ENV in modperl_request_configure_server
> + * %ENV will be reset via modperl_env_request_unpopulate.
> + */
> +
> + if (!apr_is_empty_table(scfg->SetEnv)) {
> + MP_TRACE_e(MP_FUNC, "\n\t[%s/0x%lx/%s]"
> + "\n\t@ENV{keys scfg->SetEnv} = values scfg->SetEnv;",
> + modperl_pid_tid(r->pool), modperl_interp_address(aTHX),
> + modperl_server_desc(r->server, r->pool));
> + modperl_env_table_populate(aTHX_ scfg->SetEnv);
> +
> + overlay_subprocess_env(r, scfg->SetEnv);
> }
>
> if (!apr_is_empty_table(scfg->PassEnv)) {
> + MP_TRACE_e(MP_FUNC, "\n\t[%s/0x%lx/%s]"
> + "\n\t@ENV{keys scfg->PassEnv} = values scfg->PassEnv;",
> + modperl_pid_tid(r->pool), modperl_interp_address(aTHX),
> + modperl_server_desc(r->server, r->pool));
> + modperl_env_table_populate(aTHX_ scfg->PassEnv);
> +
> overlay_subprocess_env(r, scfg->PassEnv);
> }
> +
> + MpReqPERL_SET_ENV_SRV_On(rcfg);
> }
>
> void modperl_env_default_populate(pTHX)
> @@ -217,27 +281,45 @@
> void modperl_env_request_populate(pTHX_ request_rec *r)
> {
> MP_dRCFG;
> -
> - if (MpReqSETUP_ENV(rcfg)) {
> - return;
> - }
> -
> - /* XXX: might want to always do this regardless of PerlOptions -SetupEnv */
> - modperl_env_configure_request(r);
> -
> - ap_add_common_vars(r);
> - ap_add_cgi_vars(r);
> +
> + /* this is called under the following conditions
> + * - if PerlOptions +SetupEnv
> + * - if $r->subprocess_env() is called in a void context with no args
> + *
> + * normally, %ENV is only populated once per request (if at all) -
> + * just prior to content generation if +SetupEnv.
> + *
> + * however, in the $r->subprocess_env() case it will be called
> + * more than once - once for each void call, and once again just
> + * prior to content generation. while costly, the multiple
> + * passes are required, otherwise void calls would prohibit later
> + * phases from populating %ENV with new subprocess_env table entries
> + */
>
> MP_TRACE_e(MP_FUNC, "\n\t[%s/0x%lx/%s%s]"
> "\n\t@ENV{keys r->subprocess_env} = values r->subprocess_env;",
> modperl_pid_tid(r->pool), modperl_interp_address(aTHX),
> modperl_server_desc(r->server, r->pool), r->uri);
> - modperl_env_table_populate(aTHX_ r->subprocess_env);
> +
> + /* we can eliminate some of the cost by only doing CGI variables once
> + * per-request no matter how many times $r->subprocess_env() is called
> + */
> + if (! MpReqSETUP_ENV(rcfg)) {
> +
> + ap_add_common_vars(r);
> + ap_add_cgi_vars(r);
>
> #ifdef MP_COMPAT_1X
> - modperl_env_default_populate(aTHX); /* reset GATEWAY_INTERFACE */
> + modperl_env_default_populate(aTHX); /* reset GATEWAY_INTERFACE */
> #endif
> + }
> +
> + modperl_env_table_populate(aTHX_ r->subprocess_env);
>
> + /* don't set up CGI variables again this request.
> + * this also triggers modperl_env_request_unpopulate, which
> + * resets %ENV between requests - see modperl_config_request_cleanup
> + */
> MpReqSETUP_ENV_On(rcfg);
> }
>
modperl_env.c: In function `modperl_env_configure_request_dir':
modperl_env.c:223: warning: implicit declaration of function `MpReqPERL_SET_ENV_DIR_On'
modperl_env.c: In function `modperl_env_configure_request_srv':
modperl_env.c:258: warning: implicit declaration of function `MpReqPERL_SET_ENV_SRV_On'
> [...]
Re: cvs commit: modperl-2.0/xs/Apache/RequestRec Apache__RequestRec.h
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>A few compilation problems were introduced by this one, see below.
>
>
> bah, I just don't see those with my compiler.
>
> I guess we need to decide what to do about warnings like these that aren't
> caught by older versions of gcc - either the remaining two of us need to
> upgrade or, well, I don't know what.
These are real problems, that we need to fix.
As philippe suggested, he can report them (and try patches that we post for
review ;) till we get our gcc updated. I sync mine all the time with mandrake
cooker, which I guess is a bit conservative.
As I've mentioned gcc 3.4 is out, but I'll wait a bit before upgrading to it.
__________________________________________________________________
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: cvs commit: modperl-2.0/xs/Apache/RequestRec Apache__RequestRec.h
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> On Thu, 2004-02-12 at 18:46 -0500, Geoffrey Young wrote:
>
>>>Oh, well, that's the price to pay when you are not riding the "bleeding"
>>>edge ;-)
>>
>>:)
>>
>>
>>>
>>>>I guess we need to decide what to do about warnings like these that aren't
>>>>caught by older versions of gcc - either the remaining two of us need to
>>>>upgrade or, well, I don't know what.
>>>
>>>
>>>Well, for now, I don't mind being the gcc -Wall police and expose & fix
>>>problems as I experience them. No urgent need to force a forward upgrade
>>>for anybody.
>>
>>ok, cool. the first error looks pretty bogus to me, though - I guess it's
>>the split if-blocks that are giving it trouble (or it's too late in the
>>evening for me :)
>
>
> Yes, seems the compiler is tripping there (probably the #ifdefs) and
> shouldn't be a problem, really.
>
> Just doing modperl_interp_t *interp = NULL; should shut it up (read,
> 'please geoff')
The compiler is correct about this warning. Since MpDirSETUP_ENV(dcfg) may
return false on the first call, but true on the second.
I think another solution, is what I've proposed before - drop MpDirSETUP_ENV
conditional from select/unselect code and leave it only around
modperl_env_request_populate(aTHX_ r);
It shouldn't have a big overhead, because select/unselect happens anyway later
on in the callback, which is does two if calls and returns:
...
if (rcfg && rcfg->interp) {
/* if scope is per-handler and something selected an interpreter
* before modperl_callback_run_handlers() and is still holding it,
* e.g. modperl_response_handler_cgi(), that interpreter will
* be here
*/
MP_TRACE_i(MP_FUNC,
"found interp 0x%lx in request config\n",
(unsigned long)rcfg->interp);
return rcfg->interp;
}
Philippe's solution is fine too.
__________________________________________________________________
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: cvs
commit: modperl-2.0/xs/Apache/RequestRec Apache__RequestRec.h
Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Thu, 2004-02-12 at 18:46 -0500, Geoffrey Young wrote:
> > Oh, well, that's the price to pay when you are not riding the "bleeding"
> > edge ;-)
>
> :)
>
> >
> >
> >>I guess we need to decide what to do about warnings like these that aren't
> >>caught by older versions of gcc - either the remaining two of us need to
> >>upgrade or, well, I don't know what.
> >
> >
> > Well, for now, I don't mind being the gcc -Wall police and expose & fix
> > problems as I experience them. No urgent need to force a forward upgrade
> > for anybody.
>
> ok, cool. the first error looks pretty bogus to me, though - I guess it's
> the split if-blocks that are giving it trouble (or it's too late in the
> evening for me :)
Yes, seems the compiler is tripping there (probably the #ifdefs) and
shouldn't be a problem, really.
Just doing modperl_interp_t *interp = NULL; should shut it up (read,
'please geoff')
> >
> > I do suspect, though, that this error is genuinly a problem :
> >
> > modperl_callback.c: In function `modperl_callback_run_handlers':
> > modperl_callback.c:206: warning: implicit declaration of function `MpReqPERL_SET_ENV_SRV'
> > modperl_callback.c:216: warning: implicit declaration of function `MpReqPERL_SET_ENV_DIR'
> >
> > Looks like you created those 2 new functions and did not check in their
> > implementation.
>
> those are autogenerated in modperl_flags.h via ModPerl::Code. did you start
> from 'perl Makefile.PL...' or simply 'make'? or maybe I need to add an
> #include?
Oy! I got bitten by the autogeneration... Just ignore my negative
comments, a full rebuild fixed my problem and all is good again ;)
Problem solved!
> --Geoff
>
Re: cvs commit: modperl-2.0/xs/Apache/RequestRec Apache__RequestRec.h
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> Oh, well, that's the price to pay when you are not riding the "bleeding"
> edge ;-)
:)
>
>
>>I guess we need to decide what to do about warnings like these that aren't
>>caught by older versions of gcc - either the remaining two of us need to
>>upgrade or, well, I don't know what.
>
>
> Well, for now, I don't mind being the gcc -Wall police and expose & fix
> problems as I experience them. No urgent need to force a forward upgrade
> for anybody.
ok, cool. the first error looks pretty bogus to me, though - I guess it's
the split if-blocks that are giving it trouble (or it's too late in the
evening for me :)
>
> I do suspect, though, that this error is genuinly a problem :
>
> modperl_callback.c: In function `modperl_callback_run_handlers':
> modperl_callback.c:206: warning: implicit declaration of function `MpReqPERL_SET_ENV_SRV'
> modperl_callback.c:216: warning: implicit declaration of function `MpReqPERL_SET_ENV_DIR'
>
> Looks like you created those 2 new functions and did not check in their
> implementation.
those are autogenerated in modperl_flags.h via ModPerl::Code. did you start
from 'perl Makefile.PL...' or simply 'make'? or maybe I need to add an
#include?
--Geoff
---------------------------------------------------------------------
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/xs/Apache/RequestRec Apache__RequestRec.h
Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Thu, 2004-02-12 at 18:18 -0500, Geoffrey Young wrote:
> > A few compilation problems were introduced by this one, see below.
>
> bah, I just don't see those with my compiler.
Oh, well, that's the price to pay when you are not riding the "bleeding"
edge ;-)
> I guess we need to decide what to do about warnings like these that aren't
> caught by older versions of gcc - either the remaining two of us need to
> upgrade or, well, I don't know what.
Well, for now, I don't mind being the gcc -Wall police and expose & fix
problems as I experience them. No urgent need to force a forward upgrade
for anybody.
I do suspect, though, that this error is genuinly a problem :
modperl_callback.c: In function `modperl_callback_run_handlers':
modperl_callback.c:206: warning: implicit declaration of function `MpReqPERL_SET_ENV_SRV'
modperl_callback.c:216: warning: implicit declaration of function `MpReqPERL_SET_ENV_DIR'
Looks like you created those 2 new functions and did not check in their
implementation.
> --Geoff
>
Re: cvs commit: modperl-2.0/xs/Apache/RequestRec Apache__RequestRec.h
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> A few compilation problems were introduced by this one, see below.
bah, I just don't see those with my compiler.
I guess we need to decide what to do about warnings like these that aren't
caught by older versions of gcc - either the remaining two of us need to
upgrade or, well, I don't know what.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org