Beefy Boxes and Bandwidth Generously Provided by pair Networks
Don't ask to ask, just ask
 
PerlMonks  

"require Carp" may be hazardous to your code

by Steve_p (Priest)
on Dec 05, 2006 at 15:51 UTC ( [id://587888]=perlmeditation: print w/replies, xml ) Need Help??

I've been testing a lot of modules with bleadperl and have run into a few issues with some modules, but for the most part modules have installed just fine. There have been one rather common issue with some CPAN authors' use of Carp. In normal code, it will probably look like this.

use warnings; use strict; sub complain { require Carp; Carp::carp "This should not work!"; }

Here's a one-liner with Perl 5.8.8 and bleadperl that demonstrates the problem.

[steve@kirk ~]$ perl -Mstrict -Mwarnings \ -le'sub complain{ require Carp; Carp::carp "This should not work!"; } +\ complain' This should not work! at -e line 1 main::complain() called at -e line 1 [steve@kirk ~]$ /tmp/bleadperl/bin/perl5.9.5 -Mstrict \ > -Mwarnings -le'sub complain{ require Carp;\ > Carp::carp "This should not work!"; } complain' String found where operator expected at -e line 2, near "Carp::carp "T +his shoulld not work!"" (Do you need to predeclare Carp::carp?) syntax error at -e line 2, near "Carp::carp "This should not work!"" Execution of -e aborted due to compilation errors.

What's the difference? The code above is relying on some undefined behavior of warnings.pm. warnings used Carp which would invade the namespaces of other modules that used warnings. With change #23768, warnings was changed to only require Carp when it is needed. Without some module using Carp and importing it into the global namespace, it turns into another bareword, unless you put parentheses around your arguments.

[steve@kirk ~]$ /tmp/bleadperl/bin/perl5.9.5 -Mstrict \ -Mwarnings -le'sub complain{ require Carp; \ Carp::carp("This will work!"); } complain' This will work! at -e line 1 main::complain() called at -e line 2

How common is this problem? Common enough, I guess. I’ve so far tested around 800 CPAN distributions with bleadperl, and have seen around ten modules fail with this problem. That’s just over 1% of the modules. This figure may go up or down, since it seems to correlate to the CPAN author, rather than something happening at random. If an author uses the require Carp idiom incorrectly once, it seems pretty likely that they’ll do it again. This may hit non-CPAN code a bit harder since error handling routines are typically copy-and-pasted around or placed in a common module. If done incorrectly, it could bring down an application, or a whole suite of applications.

So, please, please, please, test your modules with bleadperl! Its better to do it now rather than wait for Perl 5.10 to be released. See my signature for details on how to get a bleadperl and test with it.


Steve_p

Test your modules with bleadperl!

  rsync -avz rsync://public.activestate.com/perl-current/ .
  ./Configure -des -Dusedevel -Dprefix=/path/to/test/perl
  make test
  make install

Now, please test you modules! If you have test failures that don't happen with Perl 5.8.8, send a simplified test case to

perlbug at perl.org

Replies are listed 'Best First'.
Re: "require Carp" may be hazardous to your code
by grinder (Bishop) on Dec 05, 2006 at 17:40 UTC

    And remember, one doesn't even have to worry about fiddling with the prefix. If you don't do anything else, the perl binary itself will be installed as (for instance) perl5.9.5, so it is not going to overwrite your existing 5.8 (or earlier) perl.

    Then, one just has to change to the development directory of a module, and run

    perl5.9.5 Makefile.PL && make all test

    Note that Module::Build is bundled with bleadperl, so you can run perl5.9.5 Build.PL and it should Just Work.

    • another intruder with the mooring in the heart of the Perl

Re: "require Carp" may be hazardous to your code
by webfiend (Vicar) on Dec 05, 2006 at 18:41 UTC

    I have a dumb question. I'm not familiar with the idiom you described above. Admittedly, this may be due to the fact that I haven't delved too deeply into the innards of CPAN modules or prepared any Perl modules for distribution beyond work.

    Why would somebody do this in their modules:

    use warnings; use strict; sub complain { require Carp; Carp::carp "This should not work!"; # Or whatever }
    ... rather than doing this?
    use warnings; use strict; use Carp; ...

      chdir to your Perl's lib directory and search for *heavy*. The idea is to not immediately load things which take (small, but sometimes not insignificant) time to load if you are likely not going to use them. "require Exporter" now delays loading most of the code that used to be in Exporter.pm until such time as someone uses an "advanced" feature of Exporter.pm. At that time it loads some "Exporter*heavy*" thingy (there is some variability in the details of how each of these is done, but "heavy" seems a common component). Other heavily used modules do similar tricks.

      At some point, at least one module author decided to delay loading Carp.pm until such time as a need for it was demonstrated. I'm not sure how good the evidence was that motivated this decision, but the idea was followed by other module authors under the assumption that there was good enough reason for one to go through this small bit of extra work.

      In any case, I do it in some of my modules out of what some would call "cargo cult" practice and some would call "best practices", depending on whether they want to make such look bad or good.

      But I'm well aware of the pitfalls of conditional compile-time magic and so don't leave off my parens. Actually, my soon-to-be-released module does:

      sub _croak { require Carp; return \&Carp::croak; } sub Whatever { _croak->( "Usage: Whatever()" ) if 0 < @ARGV;

      (because I don't like depending on the sometimes-inappropriate "intelligence" Carp tries to apply to skipping stack frames to be reported)

      - tye        

      In a typical run, carp is not needed, so Carp is only loaded when needed in an effort to speed up the program.

      However, the savings are negligible unless the script is called frequently. And if the script is called frequently, the savings are eclipsed by those that would be obtained by using a persistant process.

      There's really no reason to do this. Carp is a lightweight module.

        There's really no reason to do this. Carp is a lightweight module.

        Although the proponent(s) of this practice have yet to post data to back it up where I recall having seen it, I don't recall you having posted data to support your claim either. Note that this practice predates the following in Carp.pm:

        require Carp::Heavy unless $INC{"Carp/Heavy.pm"};

        Note that I get quite a kick out of the last half of that statement. I'm sure the very first thing that require does is exactly that anyway, so I'd never add such trivial code in an obvious attempt at micro-optimization. But perhaps this whole hubbabaloo is just micro-optimization. (Update: Which brings up my long-standing catch phrase, "Nothing is obvious unless you are overlooking something" -- see ikegami's reply below.)

        It is my belief that more than just unfounded attempts are micro-optimization were required to motivate so much reworking of these modules. I could certainly be wrong in that belief, but your say-so isn't enough to convince me otherwise.

        Also note that I delay loading Carp.pm for my own reasons that I find few people share. I don't load modules that I likely don't need because I don't like my modules failing in those rare environments when some module that most people presume that everyone has is actually missing. I've certainly run into such environments many times. In fact, checking my soon-to-be-released module, the code I wrote is actually:

        BEGIN { my $croak; sub _croak { if( ! $croak ) { if( eval { require Carp; 1 } ) { $croak= \&Carp::croak; } else { $croak= sub { die @_; }; } } return $croak; } }

        Which means that my module works fine even if Carp.pm is nowhere to be found.

        Update: I also get a kick out of the fact that most of the code has been moved out of Carp.pm and yet "require Carp" has to read over a ton of POD in order to get to that small bit of code. I'd micro-optimize that as well and move the POD after __END__. (:

        - tye        

      Lazy loading. If you don't need the module, don't load it.

      --MidLifeXis

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlmeditation [id://587888]
Approved by Sidhekin
Front-paged by demerphq
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others perusing the Monastery: (7)
As of 2024-04-18 10:25 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found