Philippe M. Chiasson wrote:
Following this discussion: http://marc.theaimsgroup.com/?t=107100040400003&r=1&w=2

I've made a few adjustements and cleanups.

The following patch adds ModPerl::Util::file2package() to build a safe
package from a pathname or filename.

Do you think we should really expose it in the public API? package2filename is clear and generic, but file2package does a few assumptions that might not be suitable to users. Do you think it'll really speed up registry? If not I'd keep it as an internal util function.


This is in turn used by <Perl> sections to put each block in it's own
namespace.


Configuration data placed in Apache::ReadConfig:: directly is processed
after the end of each <Perl> blocks to preserve current behaviour.
Should be marked as deprecated as soon as users can feed their own
configuration to Apache::PerlSections (not possible quite yet)

I'd deprecate it for a few releases to let people time to make the transition and then completely drop the support (at the latest by 2.0 release).


How about this?

haven't tested, but conceptually looks good.


Index: src/modules/perl/modperl_cmd.c

+ namespace = modperl_file2package(p, parms->directive->filename);
+
+ package_name = apr_psprintf(p, "%s::%s::line_%d", + package_name,
^^^^^^^^^^^^^
+ namespace, + parms->directive->line_num);

can this be a different name? It doesn't feel very good when you use 'package_name' to create 'package_name', e.g. call the component as package_base_name?


+#define MP_VALID_PATH_DELIM(c) ((c) == '/' || (c) =='\\')

is it always '\\'? couldn't it be "\\\\"?


+char *modperl_file2package(apr_pool_t *p, const char *file)
+{
+ char *package;
+ char *c;
+ const char *f;
+ int len = strlen(file)+1;
+
+ /* First, skip invalid prefix characters */
+ while (!MP_VALID_PKG_CHAR(*file)) {
+ file++;
+ len--;
+ }
+
+ /* Then figure out how big the package name will be like */
+ for(f = file; *f; f++) {
+ if (MP_VALID_PATH_DELIM(*f)) {
+ len++;
+ }
+ }
+
+ package = apr_pcalloc(p, len);
+
+ /* Then, replace bad characters with '_' */
+ for (c = package; *file; c++, file++) {
+ if (MP_VALID_PKG_CHAR(*file)) {
+ *c = *file;
+ }
+ else if (MP_VALID_PATH_DELIM(*file)) {
+
+ /* Eliminate subsequent duplicate path delim */
+ while (*(file+1) && MP_VALID_PATH_DELIM(*(file+1))) {
+ file++;
+ }
+ + /* path delim not until end of line */
+ if (*(file+1)) {
+ *c = *(c+1) = ':';
+ c++;
+ }
+ }
+ else {
+ *c = '_';
+ }
+ }

ayi, why does it look so much simpler in perl:


    # Escape everything into valid perl identifiers
    $package =~ s/([^A-Za-z0-9_])/sprintf("_%2x", unpack("C", $1))/eg;

    # make sure that the sub-package doesn't start with a digit
    $package =~ s/^(\d)/_$1/;

can't we mirror this 1:1 with much simpler code?

[...]

kudos on docs and tests!

__________________________________________________________________
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]



Reply via email to