Perl Riddle

[lesage]

Our holyness divined a riddle to me, which I am unworthy to understand. Perhaps the higher clergy can guide my way.

package Data::CryptRecord;
BEGIN {
    use strict;
    [ ... ]
    our %ENGINES;
}

sub register { ... } # adds to the %ENGINES hash

# List what's in the hash
sub list_engines {
    my $self = shift;

    my @eng;
    while( my ($k, $v) = each(%ENGINES)){
        push @eng, $v->{'name'};
    }

    return @eng;
}

# Chose a distinct crypt method for this instance
# Lot's of list_engines included for debugging
sub set_engine {
    my ($self, $name) = @_;
    
    $name = uc($name);
    print "Enter set_engine (" . scalar $self->list_engines . ")\n";
    while( my ($k, $v) = each(%ENGINES)){
        next unless(uc($v->{'name'}) eq $name);
        $self->{'enc_type'} = $k;
        print "Exit OK set_engine (" . scalar $self->list_engines . ")
+\n";
        return 0;
    }
    print "Exit fail set_engine (" . scalar $self->list_engines . ")\n
+";
    return -1;
}

And this is what happens, when the program calls set_engine

(not shown, at the end of the CTOR)
Blessed List(1)
Enter set_engine(1)
Exit OK set_engine (0)
after set_list (1)
(This is another list_engines called later in the program)

So sometimes %ENGINES appears to be empty, but it recovers magically. Does anyone have an idea, where to start the search?

BTW: The program has a ncurses UI, therefore perl -d is tedious. Is there a way to redirect the debugging console to a different VT or XTerm?

Humbly praying for revelations ...
- lesage

Thanks a lot for your help. The crucial issue was resetting the each() iterator. So special thanks to ProfVince and shmem.

Still, use warnings; and clarification that BEGIN{} is in no way a special scope, were also very helpful.

I meanwhile figured out that

export PERLDB_OPTS='TTY=/dev/pts/2'
perl -d ./myprg.pl

can be used to redirect the perl debugger console. Tying e.g. a picocom on its own pty does almost what I'd like to have -- apart from local echo and CR/LF conversion. I'd bet there is a Perl program around doing such things. Does anybody have a pointer?

- lesage

[shmem]

Some Problems. Consider:

BEGIN {
    our %ENGINES;
}
use strict;
each %ENGINES;
__END__
Global symbol "%ENGINES" requires explicit package name at - line 5.
Execution of - aborted due to compilation errors.

our creates a lexical scoped alias of a package variable. Using it a BEGIN block does create the package global at compile time, and not much more. Your use of strict in the BEGIN block enforces strictness just there, not outside that block.

You are calling list_engines(), which uses each on %ENGINES from set_engine while iterating over %ENGINES with each. Very bad idea, because the hash iterator is bound to each hash, not to the scope each occurs in.

my %hash;

@hash{a..f} = 10..15;

sub a {
    my $a_count;
    while( my ($k,$v) = each %hash) {
        print "$k => $v\n";
        $a_count++;
    }
    $a_count++;
}
sub b {
    my $b_count;
    while( my ($k,$v) = each %hash) {
        $b_count++;
        print "$k => $v ($b_count out of ", a(), ")\n";
    }
}
b()
__END__
c => 12
a => 10
b => 11
d => 13
f => 15
e => 14 (1 out of 5)
c => 12
a => 10
b => 11
d => 13
f => 15
e => 14 (2 out of 5)
c => 12
a => 10
b => 11
d => 13
f => 15
e => 14 (3 out of 5)
...

This code runs until it hits an integer overflow on $b_count in sub b { }, because the each in sub a { } goes on iterating the same hash from where the pointer was set in sub b(), and thus $a_count is set to 5, not to 6. The hash iteration is through at the end of that while loop, so the iterator is reset. Then the print in sub b() happens, and the iteration starts over after the iterator has been reset.

You get a similar effect resetting the hash iterator with keys or values

my %hash;

@hash{a..f} = 10..15;
my $count;
while(my ($k,$v) = each %hash) {
    $count++;
    print "$k => $v ($count of ", scalar(keys %hash),")\n";
}
__END__
e => 14 (1 of 6)
e => 14 (2 of 6)
e => 14 (3 of 6)
e => 14 (4 of 6)
e => 14 (5 of 6)
e => 14 (6 of 6)
e => 14 (7 of 6)
...
(runs forever)

so don't do that.

[Zaxo]

I can't trace your problem in detail right now, but I'd try saying

use vars '%ENGINES';

before the BEGIN block. Omit the use statement in the block.

I remain uncertain about what our accomplishes, but I'm pretty sure that putting it in a BEGIN block restricts its scope.

Global variables ain't fashionable, but they have their uses in perl for class data and such.

[Somni]

It's really hard to pin down what your problem might be because you're not actually showing any of the code that calls these functions. I'd guess that the problem is you just aren't initializing %ENGINES before you start calling subroutines that use it.

Your use of strict and our is broken, however. Because they are both in a BEGIN block the scope of both is confined to just that block, making them essentially useless.

It's best to have use strict (and probably a use warnings) at the top of your code, outside of any block. The declaration for %ENGINES could be at the top, if you like, but I prefer to have the 'our' declaration near where I actually use the variable. If you intend to initialize %ENGINES at module startup then putting the declaration in a block can be a good idea.

package Data::CryptRecord;

use warnings;
use strict;

{
    our %ENGINES = (...);
}


sub set_engine {
    my($self, $name) = @_;
    our %ENGINES;
    ...
}

This provides some self-documentation; it's clear that %ENGINES is global. You can also initialize %ENGINES outside a block, and not have to declare it in your subroutines.

[Prof Vince]

%ENGINES seems to be empty because the each iterator isn't reset when you call list_engines, causing it to iterate only on the remaining items that weren't yet treated by set_engines' each. You can reset the each iterator by putting scalar keys %ENGINES at the beginning of list_engines.

[webfiend]

It might just be a personal problem, but most times I use each it ends up biting me. I've just settled into using a for loop, and it never caused me any problems related to resetting.

So instead of

while( my ($k, $v) = each(%ENGINES)){
    push @eng, $v->{'name'};
}

... I would probably end up doing this:

for my $k (keys %ENGINES) {
    my $v = $ENGINES{$k};
    push @eng, $v->{'name'};
}

This is a personal preference, and I'll admit that it does result in a little more code. I'm okay with that. I'd probably use map if I was concerned about brevity. The important thing is that I don't have to worry about resetting anything.