perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject Re: DECLINE_CMD
Date Fri, 09 May 2003 00:26:26 GMT
Geoffrey Young wrote:
> 
>> I think there is a much cleaner solution. Fix ModPerl::Code to make
>> modperl_constants_lookup_apr and modperl_constants_lookup_apache 
>> return SV. 
> 
> 
> good idea - I thought it would be harder than it was to change the 
> return type of everything. 

Hmm, I haven't realized that it's used in _module.c as well. So my idea 
introduces a bit of an overhead to allocate the SV, when it only wants an int.
But since this happens only during the server startup, I suppose it's not a 
problem.

> here is take 2.  the only thing that I'm not 
> sure about is
> 
> -        newCONSTSUB(stash, (char *)name, newSViv(val));
> +        newCONSTSUB(stash, (char *)name, newSVsv(val));
> 
> and whether we need to copy val with newSVsv or can just pass val along 
> without the copy.

I think it should be:

-        newCONSTSUB(stash, (char *)name, newSViv(val));
+        newCONSTSUB(stash, (char *)name, val);


>> BTW, why do we have the same generated files in two places?
>>
>> grep -Ilr modperl_constants_lookup_apr src xs 
>> src/modules/perl/modperl_const.c | grep '\.c$'
>> src/modules/perl/modperl_const.c
>> src/modules/perl/modperl_constants.c
>> xs/ModPerl/Const/modperl_const.c
>> xs/ModPerl/Const/modperl_constants.c
> 
> 
> I have no idea :)

Hmm...

A few more comments below:

> Index: lib/ModPerl/Code.pm
[...]
>  $proto
>  {
> +    dTHX;

It's going to be much more efficient to pass pTHX_ to the function.

> +          if (strEQ(name, "$name")) {
> +              return newSVpv($alias{$name},0);

Use newSVpvn, you know the length already (faster).

> +          }
> +$ifdef[1]
> +EOF
> +            }
> +            else {
> +                print $c_fh <<EOF;
>  $ifdef[0]
>            if (strEQ(name, "$name")) {
> -              return $alias{$name};
> +              return newSViv($alias{$name});

Use strnEQ (faster), like the orig int functions do. You don't really have to 
make special cases for these two constants. Just use the old generation code 
and put 'return SVpvn($alias{$name});' if perl sees these two constants, 
otherwise put 'return SViv($alias{$name});'

> -typedef int (*constants_lookup)(const char *);
> +typedef SV *(*constants_lookup)(const char *);

So pTHX_ goes here:

-typedef int (*constants_lookup)(const char *);
+typedef SV *(*constants_lookup)(pTHX_ const char *);

and aTHX_ in the caller...

> +                    SvIV(modperl_constants_lookup_apache(SvPV(val, len)));
>              }
>          }
> 
> @@ -695,7 +695,7 @@
>              }
>              else {
>                  cmd->req_override =
> -                    modperl_constants_lookup_apache(SvPV(val, len));
> +                    SvIV(modperl_constants_lookup_apache(SvPV(val, len)));

it's probably safe to use SvIVX here, but since this code is not run-time 
critical SvIV is just fine.

__________________________________________________________________
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


Mime
View raw message