Find the bug!

Ovid on 2008-02-25T15:23:27

I encountered a delightful bug today. I'll make it easy for you and not try to obfuscate it. It's a potentially fatal error in this code, not the calling code.

sub import {
    my $class = shift;
    while ( @_ ) {
        $_ = shift;
        # do stuff
    }
}

That's bad code, but it took me a while to see it. If you've read some of my past complaints about the type of code causing problems in our BBC code base, that's a hint.


Re: Find the bug! (localize)

Corion on 2008-02-25T16:06:56

As a lesson, you should always localize $_ and the other stuff when assigning to it...

perl -le "sub _import{$_=shift};for('foo'){_import($class,$arg)}"

Re: Find the bug! (localize)

jplindstrom on 2008-02-25T17:58:09

Another lesson is to always. name. variables.

Re: Find the bug! (localize)

Aristotle on 2008-02-26T01:35:29

Sorry, not gonna. Perl has $_ for a reason.

Re: Find the bug! (localize)

slanning on 2008-02-29T11:18:50

The ultimate conquest of humanity by a species of intelligent rats?

Re: Find the bug! (localize)

Ovid on 2008-02-29T13:12:22

At this point, I can't say we'd be that much worse off, except for the horrible programming language they would force us to standardize on.

Modifying @_?

Smylers on 2008-02-25T19:08:46

If # do stuff modifies $_ then foreach aliasing will try to modify the relevant entry in @_.

That's unlikely to be desired, and I think will cause a crash if the caller passed a constant in.

Re: Modifying @_?

jarich on 2008-02-26T00:21:59

Since the code is using while rather than foreach and more importantly shift rather than aliasing, this isn't going to be the bug in this code. When we shift, we copy, so changes (in this case) to $_ won't change @_.

Having said that, I'd much prefer the original code to say:

sub import {
    my $class = shift;
    foreach my $arg (@_) {
        # do stuff
    }
}

But then changing $arg would hit the problems you're thinking of.

Re: Modifying @_?

Aristotle on 2008-02-26T01:38:00

for my $arg (my @copy = @_) { ... }

Re: Modifying @_?

Smylers on 2008-02-26T05:12:38

the code is using while rather than foreach

Hmmmm, clearly I can't read! Ooops.