Beefy Boxes and Bandwidth Generously Provided by pair Networks
Pathologically Eclectic Rubbish Lister
 
PerlMonks  

while(<>) { ... } considered harmful

by IlyaM (Parson)
on Sep 07, 2002 at 23:22 UTC ( #195926=perlmeditation: print w/ replies, xml ) Need Help??

I've just spent more than one hour debugging very wierd problem. Test script for one of my projects I'm currently working on died with error like:
Modification of a read-only value attempted at /usr/share/perl5/Net/DN +S/Resolver.pm line 235. Compilation failed in require at /usr/share/perl5/Net/DNS.pm line 23. BEGIN failed--compilation aborted at /usr/share/perl5/Net/DNS.pm line +23. Compilation failed in require at /usr/share/perl5/Mail/CheckUser.pm li +ne 28. BEGIN failed--compilation aborted at /usr/share/perl5/Mail/CheckUser.p +m line 30. .... ....
Let's look in Net/DNS/Resolver.pm:
.... sub res_init { if ($os eq "unix") { res_init_unix(); } .... } .... sub res_init_unix { read_config($resolv_conf) if (-f $resolv_conf) and (-r $resolv +_conf); .... } .... sub read_config { .... while (<FILE>) { <------ This is line 235 where it fails .... } .... res_init()
As you can see on load of Net::DNS::Resolver module it tries to read DNS resolver configuration file in subroutine read_config() if it runs on Unix and there exist this file. Nothing wrong with it at first look. Yet it fails with 'Modification of a read-only value attempted' error on while (<FILE>) { line.

Turned out that I had some code which could load Net::DNS module on demand and this code was called inside of map. My next step was finding that following snipplet dies with same error. map { require Net::DNS } 1;
Than I removed all non-essential details and I got final test case: map { while(<>) { } } 1;
Suddendly this error started to make sense to me. map associates $_ to elements of the passed list. In this case it is a list with a single element - constant 1. As it is a constant $_ becomes read-only but while(<>) { } still tries to assign data obtained from file handle to $_ and it results in this error.

So what is the conclusion? Do not use while(<>) { } unless you localize $_ or you are completly sure that nobody will call you code from map (or grep, or for/foreach - they are subject to the same problem). Personally I prefer always using while(my $line = <>) construct.

Update: These nodes demonstrate same problem with while(<>) { ... }: Method call to tied hash leads to file read error and opening a file destroys nulling entries in a list?!?!.

--
Ilya Martynov (http://martynov.org/)

Comment on while(<>) { ... } considered harmful
Select or Download Code
Re: while(<>) { ... } considered harmful
by shotgunefx (Parson) on Sep 07, 2002 at 23:30 UTC
    Any module that uses $_ should localize it or any other globals they futz around with IMHO.

    -Lee

    "To be civilized is to deny one's nature."
      I guess it is a time to send patches for 5.8.0 core Perl modules :)
      • Locale::Country
      • Locale::Currency
      • Locale::Language
      • Locale::Script
      It seems all of them touch $_ but do not localize $_.

      --
      Ilya Martynov (http://martynov.org/)

Re: while(<>) { ... } considered harmful
by dws (Chancellor) on Sep 07, 2002 at 23:37 UTC
    Since   map { substr($_, 1, 1) = "foo" } qw(bar baz);
    and   grep { substr($_, 1, 1) = "foo" } qw(bar baz);
    both exhibit the problem you've observed, perhaps the advice might better be phrased

    Don't write to $_ from within map { } or grep { }

    local $_; is one way to avoid writing to it, since the 'it' is now different.


    Update: Hm.. It's a bit worse than that.   foreach ( qw(foo) ) { $_ = 1 }
    also exhibits the problem.

Re: while(<>) { ... } considered harmful
by Aristotle (Chancellor) on Sep 07, 2002 at 23:42 UTC
    I concur with shotgunefx: I go by the rule that the main program is free to do with $_ whatever it wants and everyone else has to make sure they're not getting in the way.

    Makeshifts last the longest.

Re: while(<>) { ... } considered harmful
by BrowserUk (Pope) on Sep 08, 2002 at 00:36 UTC

    'scuse my ignorance of nether regions below Perl's skirts, but would it impose a huge performance penalty for map and grep to localise $_ on entry?

    I visualise them as subs with their own local scope. Would a local $_; at the top cost so very much?


    Well It's better than the Abottoire, but Yorkshire!

      Unless I totally misunderstood the above comments -- which is possible as I just briefly skimmed them -- the problem here is not that map and grep don't localize $_, but that they may execute code that may alter the value of $_, which in this case is aliased to a constant. Consider this code:

      $_ = 'Ovid'; %hash = map { $_ => 1 } qw/ foo bar baz /; print;

      That code will print 'Ovid' because $_ has been localized within the map statement. Now, if you try to alter the $_, your code fails because you're trying to alter a constant:

      $_ = 'Ovid'; %hash = map { $_ .= 'asdf'; $_ => 1 } qw/ foo bar baz /; print;

      To get around that, you can do this:

      $_ = 'Ovid'; %hash = map { local $_ = $_; $_ .= 'asdf'; $_ => 1 } qw/ foo bar baz /; print;

      That allows you to assign the aliased constant to a real variable, thus avoiding the problems outlined in this thread.

      Cheers,
      Ovid

      Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

Re: while(<>) { ... } considered harmful
by Juerd (Abbot) on Sep 08, 2002 at 00:55 UTC

    map { require Net::DNS } 1;

    Is that a map in void context? Nasty and evil. (I don't recall require ever returning anything useful.)

    Personally I prefer always using while(my $line = <>) construct.

    I don't always. Setting $_ lets one use regexes and simple commands easier. I do use a lexical for more complicated blocks, but I often find myself just slurping the file into an array first.

    - Yes, I reinvent wheels.
    - Spam: Visit eurotraQ.
    

          map { require Net::DNS } 1;
        Is that a map in void context? Nasty and evil.

      I believe that's a minimal demonstration of a bug, rather than "real" code.

      On the subject of while(<>) vs. while(my $line = <>), I tend to prefer the former:

      • If I don't have to do much processing for each line, it's usually much nicer to work on $_, as Juerd points out.
      • If I do have a lot of work to do, it's usually in a subroutine call, where this sort of thing isn't an issue (since the scope has changed).

      I haven't really thought about it, but if a loop's so complex that you have to assign to an explicit iterator variable just to figure out what's going on (as opposed to a loop where defaulting to $_ would be inappropriate for other reasons), it's probably an indication that the loop needs simplifying.

      --
      F o x t r o t U n i f o r m
      Found a typo in this node? /msg me
      The hell with paco, vote for Erudil!

        I believe that's a minimal demonstration of a bug, rather than "real" code.

        You are right. Real code looks like:

        my @other_connectors = map $ad_obj->connector(field => $_), qw(header description region country state);
        No require on the surface. It was deeply hidden in one of method calls. I though it was just a Perl bug when I got that error for first time.

        --
        Ilya Martynov (http://martynov.org/)

      require returns the last evaluated expression in the file. Normally this is 1, but I've seen people do wacky things like have "return %hash" as the last expression and do "%hash = require Foo::Bar";

        Since my post, I have used eval { require ... } ? ...->import : *symbol = sub { dummy here } a lot.

        Juerd # { site => 'juerd.nl', plp_site => 'plp.juerd.nl', do_not_use => 'spamtrap' }

        require returns the last evaluated expression in the file

        Only the first time:

        > perl -de 0 ... DB<1> x require 'con' 'this is true'; ^Z 0 'this is true' DB<2> x require 'con' 0 1 DB<3> x require 'con' 0 1 DB<4>
        I've seen people do wacky things like have "return %hash" as the last expression and do "%hash = require Foo::Bar";

        So such code will break as soon as more than one use of the module is attempted from a single run. That's a bit of a shame too, since Perl could really use something along those lines.

                        - tye
Re: while(<>) { ... } considered harmful
by Anonymous Monk on Sep 08, 2002 at 04:14 UTC
    I probably wrong but I'd say map is considered harmful.
    I use while(<>){...} all the time but never use map.
    Would I run into this problem if I avoid using
    map? I guess I just don't understand map.
    Why use map?
      Because something like my @idx = map /(id\d+)/, @items;
      is a lot more natural than
      my @idx; /(id\d+)/ && push @idx, $1 for @items;
      (which is already pushing legibility). Basically when you build one list out of another, non-destructively, map is the ticket. It can be in other cases as well, but those can often go either way.

      Makeshifts last the longest.

      I disagree with saying that map is considered harmful. As I noted in other reply in this thread its behaviour is consistent with other Perl operators: for/foreach and grep. In my opinion it is while(<>)'s magic is wrong. It should localize $_ so it doesn't affect outer scope.

      --
      Ilya Martynov (http://martynov.org/)

Re: while(<>) { ... } considered harmful
by converter (Priest) on Sep 08, 2002 at 17:00 UTC

    The perlsub man page mentions localization of $_ in the "When to Still Use local()" section, and says:

    In particular, it's important to "local"ize $_ in any routine that assigns to it. Look out for implicit assignments in "while" conditionals.

    With all due respect, both you and the authors of the modules you mention are ignoring the sound advice offered in the Perl documentation and should consider making the appropriate changes to make your code "play well with others".

      IMHO it just too easy to do such mistake. Most of Perl operators that automagically assign to $_ (for/foreach, map, grep) do localize it so it doesn't affect outer scope. On the other hand magic in while(<>) construct doesn't work same way while newbie to Perl can expect Perl to do it. Frankly I've never though about such side effect of this construct before and I think I can say that I'm seasoned Perl programmer.

      Anyway what I did wrong? I just have simple map construct (I've posted snipplet of real code in other reply to this thread) and that error was very confusing. Most of the time Perl does right thing with scopes and badly written external to my code module cannot affect my code so destructively. Usually I can treat them as black boxes.

      Now instead of just writting use Net::DNS in my modules I will have to patch them to have something like { local $_; require Net::DNS } (until Net::DNS is fixed).

      --
      Ilya Martynov (http://martynov.org/)

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others pondering the Monastery: (10)
As of 2014-12-21 17:36 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    Is guessing a good strategy for surviving in the IT business?





    Results (106 votes), past polls