On Wed, 2004-02-11 at 17:02 -0800, Stas Bekman wrote:
> Doesn't save_item completely copy the original sv away and then restore it on
> the scope exit? For some reason it doesn't work (fails with 'insecure
> dependency on eval' error).
>
> Index: src/modules/perl/modperl_cmd.c
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_cmd.c,v
> retrieving revision 1.54
> diff -u -r1.54 modperl_cmd.c
> --- src/modules/perl/modperl_cmd.c 9 Feb 2004 18:18:16 -0000 1.54
> +++ src/modules/perl/modperl_cmd.c 11 Feb 2004 23:43:49 -0000
> @@ -497,19 +497,12 @@
> dollar_zero = get_sv("0", TRUE);
> dollar_zero_tainted = SvTAINTED(dollar_zero);
>
> - if (dollar_zero_tainted) {
> - SvTAINTED_off(dollar_zero);
> - }
> -
> ENTER;
> save_item(dollar_zero);
> + SvTAINTED_off(dollar_zero);
> sv_setpv(dollar_zero, directive->filename);
> eval_pv(arg, FALSE);
> LEAVE;
> -
> - if (dollar_zero_tainted) {
> - SvTAINTED_on(dollar_zero);
> - }
>
> Any idea, why?
No, not really. When I originally wrote that piece of code, I was trying
to get it to work using save_item() only, and had to packpedal and
revert to this nastiness about dollar_zero_tainted..
As far as I can tell, save_item should work in that case.
> The following patch refactors perldo, mainly moving the short-scoped vars like
> handler_name to be declared in the scope they are used. In which case you
> don't need to init most of them, and it makes the code more readable, IMHO.
Yes, I also think it makes this fairly large piece of code more
readable.
Visually inspected and looks good to me (+1)
Gozer out.
> Index: src/modules/perl/modperl_cmd.c
> ===================================================================
> RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_cmd.c,v
> retrieving revision 1.54
> diff -u -r1.54 modperl_cmd.c
> --- src/modules/perl/modperl_cmd.c 9 Feb 2004 18:18:16 -0000 1.54
> +++ src/modules/perl/modperl_cmd.c 12 Feb 2004 00:58:56 -0000
> @@ -432,18 +432,10 @@
> {
> apr_pool_t *p = parms->pool;
> server_rec *s = parms->server;
> - apr_table_t *options = NULL;
> - const char *handler_name = NULL;
> + apr_table_t *options;
> modperl_handler_t *handler = NULL;
> - const char *pkg_base = NULL;
> - const char *pkg_namespace = NULL;
> const char *pkg_name = NULL;
> - const char *line_header = NULL;
> ap_directive_t *directive = parms->directive;
> - int status = OK;
> - AV *args = Nullav;
> - SV *dollar_zero = Nullsv;
> - int dollar_zero_tainted;
> #ifdef USE_ITHREADS
> MP_dSCFG(s);
> MP_PERL_DECLARE_CONTEXT;
> @@ -463,7 +455,12 @@
> MP_PERL_OVERRIDE_CONTEXT;
>
> /* data will be set by a <Perl> section */
> - if ((options = parms->directive->data)) {
> + if ((options = directive->data)) {
> + const char *pkg_namespace;
> + const char *pkg_base;
> + const char *handler_name;
> + const char *line_header;
> +
> if (!(handler_name = apr_table_get(options, "handler"))) {
> handler_name = apr_pstrdup(p, MP_DEFAULT_PERLSECTION_HANDLER);
> apr_table_set(options, "handler", handler_name);
> @@ -492,28 +489,30 @@
> arg = apr_pstrcat(p, "package ", pkg_name, ";", line_header,
> arg, NULL);
> }
> +
> + {
> + /* Set $0 to the current configuration file */
> + SV *dollar_zero = get_sv("0", TRUE);
> + int dollar_zero_tainted = SvTAINTED(dollar_zero);
>
> - /* Set $0 to the current configuration file */
> - dollar_zero = get_sv("0", TRUE);
> - dollar_zero_tainted = SvTAINTED(dollar_zero);
> -
> - if (dollar_zero_tainted) {
> - SvTAINTED_off(dollar_zero);
> - }
> + if (dollar_zero_tainted) {
> + SvTAINTED_off(dollar_zero);
> + }
>
> - ENTER;
> - save_item(dollar_zero);
> - sv_setpv(dollar_zero, directive->filename);
> - eval_pv(arg, FALSE);
> - LEAVE;
> + ENTER;
> + save_item(dollar_zero);
> + sv_setpv(dollar_zero, directive->filename);
> + eval_pv(arg, FALSE);
> + LEAVE;
>
> - if (dollar_zero_tainted) {
> - SvTAINTED_on(dollar_zero);
> + if (dollar_zero_tainted) {
> + SvTAINTED_on(dollar_zero);
> + }
> }
> -
> +
> if (SvTRUE(ERRSV)) {
> - SV *strict;
> - if ((strict = MP_STRICT_PERLSECTIONS_SV) && SvTRUE(strict)) {
> + SV *strict = MP_STRICT_PERLSECTIONS_SV;
> + if (strict && SvTRUE(strict)) {
> char *error = SvPVX(ERRSV);
> MP_PERL_RESTORE_CONTEXT;
> return error;
> @@ -523,12 +522,14 @@
> directive->filename,
> directive->line_num,
> SvPVX(ERRSV)));
> -
> }
> }
>
> if (handler) {
> - SV *saveconfig;
> + int status;
> + SV *saveconfig = MP_PERLSECTIONS_SAVECONFIG_SV;
> + AV *args = Nullav;
> +
> modperl_handler_make_args(aTHX_ &args,
> "Apache::CmdParms", parms,
> "APR::Table", options,
> @@ -538,7 +539,7 @@
>
> SvREFCNT_dec((SV*)args);
>
> - if (!(saveconfig = MP_PERLSECTIONS_SAVECONFIG_SV) ||
> !SvTRUE(saveconfig)) {
> + if (!(saveconfig && SvTRUE(saveconfig))) {
> HV *symtab = (HV*)gv_stashpv(pkg_name, FALSE);
> if (symtab) {
> modperl_clear_symtab(aTHX_ symtab);
>
> __________________________________________________________________
> Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
> http://stason.org/ mod_perl Guide ---> http://perl.apache.org
> mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
> http://modperlbook.org http://apache.org http://ticketmaster.com
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
--
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
signature.asc
Description: This is a digitally signed message part
