Mark Martinec writes:
> Justin,
> 
> > [EMAIL PROTECTED] writes:
> > > The Buildbot has detected a new failure of mc-fast.
> > > Full details are available at:
> > >  http://bbmass.spamassassin.org:8011/mc-fast/builds/893
> 
> > Undefined subroutine &Mail::SpamAssassin::Conf::untaint_var
> > called at ... Mail/SpamAssassin/Conf.pm line 2843
> 
> Ok, reverted for that module:
> 
> $ svn -m 'Conf.pm: revert importing of untaint_var,
>           anonymous subs may run in another context' ci
> Sending  lib/Mail/SpamAssassin/Conf.pm
> Committed revision 575772.
> 
> I wonder why self-checks and my regular usage did not notice it.
> Sorry.
> 
> > by the way, I'm not sure it's a good idea to use exported functions -- in
> > my opinion it's easier to track down function source code when the full
> > path to their module is specified:
> >     Mail::SpamAssassin::Util::untaint_var(...)
> > vs.
> >     untaint_var(...)
> 
> The Util.pm is already exporting local_tz and base64_decode,
> so I thought exporting another frequently used sub from
> a 'tools' module is not a big deal.

ah, I'd missed those...  if we're already doing it, I'm not quite
so bothered.

> > Also, does this impose a memory overhead by adding "untaint_var" to the
> > package-local namespace of every module that loads it?  if I recall
> > correctly it does, whereas the nonexported version does not.  (I
> > may be wrong ;)
> 
> I'm not aware of it, I lived under impression that just some aliasing
> is done, costing few dozens of bytes, but I may be wrong. Any refs?

actually, I misremembered -- it's a speed thing.
http://www.ccl4.org/~nick/P/Fast_Enough/#needless_importing_is_slow
Probably not quite as relevant for our code as for POSIX.pm!

> If cost of polluting a namespace is of concern, then the crusade
> should probably start with dozens of library modules which
> (through 'use') import mostly whatever is offered by default.

We already deal with those -- by explicitly naming the imported
fns:

    use POSIX qw(foo);

or just using

    use POSIX qw();

and calling POSIX::foo().

Anyway, since we're already exporting from Util (which I'd forgotten), and
since this is probably going to be quite widely used throughout, go ahead
and keep it exported. I take it back ;)

--j.

Reply via email to