perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <>
Date Thu, 08 May 2003 05:17:57 GMT
Geoffrey Young wrote:
> hi all
>   I've been playing around with directive handlers and have found a 
> problem with DECLINE_CMD, namely that it doesn't exist within 
> Apache::Const.
>   unfortunately, implementing DECLINE_CMD is not as easy as just adding 
> it to the constants map - DECLINE_CMD is the only constant in Apache/APR 
> that isn't an integer, and all the autogeneration/newCONSTSUB stuff is 
> geared toward creating integer constants.
>   the below patch works and seems a decent way to do it (now we have a 
> routine for character-based constants in case there end up being 
> others), but somebody else might have a better idea.

I think I do have such idea ;)

I think your implementation is fine, however this:

  > +    else if (strnEQ(name, "DECLINE_CMD", 11)) {

adds an unnecessary string comparison in more than 50% of times.

I think there is a much cleaner solution. Fix ModPerl::Code to make
modperl_constants_lookup_apr and modperl_constants_lookup_apache return SV. 
Voila, now replace all current returns:

   return Foo;


   return newSViv(Foo);

and add a special case for "DECLINE_CMD", which will do:

   return newSVpvn(DECLINE_CMD, 11);

Does it sound good?

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$'

Also I'd suggest to put this sub-test in t/apache/constants.t? since you just 
compare the string, without testing the functionality. Let's keep this 
particular test focused on perlmodule testing.

However a new test verifying the actual use of Apache::DECLINE_CMD would be 


> Index: t/response/TestDirective/
> ===================================================================
> RCS file: 
> /home/cvspublic/modperl-2.0/t/response/TestDirective/,v


> +
> +    ok t_cmp("\x07\x08", Apache::DECLINE_CMD);

Finally check: lib/Apache/ (uncomment DECLINE_CMD there?)

And here is another missing constant, which is a string as well, (which will 
make your code even less efficient)

#define DIR_MAGIC_TYPE "httpd/unix-directory"

it's now commented out in lib/Apache/

Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker     mod_perl Guide --->

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message