On Thu, 2003-12-11 at 15:49, Stas Bekman wrote:
> 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.

After some thinking, I've come to agree with your opinion on this one.
It's easy to expose too many things, and you end up with a big
commitment to a big API. And in this case, there is so little to gain
from it.

> > 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).

I'll work up a patch that will warn if Apache::ReadConfig:: is used
directly. How does that sound?

"[warn] Direct usage of %Apache::ReadConfig:: is deprecated"

But for this to be any use, I'd also implement the alternatives to
add_config() we discussed last week (add_hash, add_namespace, add_glob,
etc)

> > 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?

noted!

> > +#define MP_VALID_PATH_DELIM(c) ((c) == '/' || (c) =='\\')
> 
> is it always '\\'? couldn't it be "\\\\"?

Just stripping any occurences of those, so \\\\ becomes __, that's all.

> > +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/;

Because perl rocks!

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

Wish we could, but I don't think I'd feel good about calling back into
perl-land from perldo just for this.

> [...]
> 
> kudos on docs and tests!

Working up on the next patch, and already checked in the small doc patch
to ModPerl::Util without filename2package

> __________________________________________________________________
> 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 /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to