gctaylor1 has asked for the wisdom of the Perl Monks concerning the following question:

I'm having a hard time explaining this problem and having spent the weekend reading about closures and nested subroutines hasn't helped, I'm afraid.

Last week I received great advice here about using dispatch tables so I could add more command line initiated actions to my script.

Currently my script operates in a linearly fashion from top to bottom and has one main thing it does. I have all of my subroutines at the bottom and to put together the script I just call subroutines and walk through the individual tasks I want to complete until done.
I was thinking that in order to use dispatch tables all I should do is put my current script inside a new major subroutine which would continue to call other existing minor subroutines as it does now. Then I would eventually add additional major subroutines and pick and choose from the minor subroutines to complete that action until done.

The problem is that when I put my main script in a subroutine I get all kinds of "Variable $x will not stay shared at script.pl line nnn" errors.

- Is there a rookie friendly way to wrap my current script in a subroutine and not get the "Variable will not be shared" error?

The research I've done leads me to anonymous subroutines, closures, and much confusion. This example below isn't really the best example without including my whole script but it might be a good enough example because of the subroutine reference of File::Find \&wanted and it does give me the same error. There are several issues I'm dealing with in this example.

#!/usr/bin/perl use strict; use warnings; use File::Find; my %actions_hash = define_actions(); if (!defined @ARGV ) { @ARGV = "-h"; } my $action = shift; # First argument my @args = @ARGV; # Remaining arguments if (exists $actions_hash{$action}) { $actions_hash{$action}->(@args); } else { $actions_hash{_default_}->(@args); } sub define_actions { return ( '-f' => \&findfile, ); } sub findfile { my $target = $args[0]; my $location = "/tmp/dir1"; my $filename = find \&wanted, "$location"; sub wanted { (my $foundfile = $File::Find::name) if $File::Find::name =~ $t +arget; return $foundfile; } print $filename; }
  1. How to get rid of the "Varable will not be shared" errors? Either in my main script or maybe here and I can attempt to apply the knowledge globally to my script.
  2. How to get the $foundfile out of the wanted() subroutine?

For the record I've looked at the tutorials here but can't seem to apply the knowledge to my script.
Thank-you for your consideration.

  • Comment on Adding a dispatch table and getting "Variable $x will not stay shared" errors.
  • Download Code

Replies are listed 'Best First'.
Re: Adding a dispatch table and getting "Variable $x will not stay shared" errors.
by GrandFather (Saint) on Mar 03, 2009 at 01:15 UTC

    The problem is your 'nested' sub wanted. The lexical variable $target used by the named sub 'wanted' is not what you think it is. Named subs are compiled before the script is run and at that time $target is a different animal than when findfile is called during the execution of the script.

    Your best solution is to unnest wanted and pass $target in as an argument.

    For much more discussion of this issue see Perl scoping not logical...?.


    True laziness is hard work
Re: Adding a dispatch table and getting "Variable $x will not stay shared" errors.
by tilly (Archbishop) on Mar 03, 2009 at 06:13 UTC
    Expanding on GrandFather's answer, the problem here is that wanted is a globally accessible name, which is defined in the lexical scope of a single call to findfile. There will therefore be only ever be one wanted function that will only see one lexically scoped $target. So the second, third, and later times you call findfile, wanted will not see the private values of $target set on those calls.

    If this confuses you, see the code example at Re (tilly) 9: Why are closures cool? to see the issue at work.

    On an unrelated note, once you resolve that bug you will have an even more mysterious one that Perl is not giving you a warning about. The problem is in the line

    (my $foundfile = $File::Find::name) if $File::Find::name =~ $target;
    It looks innocuous, but the problem is that my has both a compile time and a run time effect. What run time effect, you say? Well as an optimization, Perl does not actually clear lexical variables when you leave a function. Instead if possible it will reinitialize them when it encounters them again. (This is generally faster.) The problem is that in the pattern:
    my $foo = $bar if condition();
    if condition() is false, then my $foo is not executed, and Perl does not reinitialize your data.

    The result? If your condition ever matches, you will continue to return that file name until the next time the condition matches at which point you will return the new file name until another one matches, etc.

    The fix is to never, ever, ever put a my where it can be hidden by a conditional. In this case you would avoid it with:

    my $foundfile; $foundfile = $File::Find::name) if $File::Find::name =~ $target;
    See Unusual Closure Behaviour for further discussion.
Re: Adding a dispatch table and getting "Variable $x will not stay shared" errors.
by runrig (Abbot) on Mar 03, 2009 at 01:12 UTC
    I second using a Getopt::* module, but to comment on your code, you are passing @args to findfile(), but not using it as proper function argument (you probably mean $_[0] instead of $args[0] in the findfile() function).

    Also, there's no point in having named nested subroutines. Instead of \&wanted, just use an anonymous subroutine...but File::Find doesn't work like what you are trying to do anyway. you shouldn't try to return things from your wanted function (find() doesn't return anything useful), you might set a variable in an outer scope though.


    So here's my untested rewrite:
    sub findfile { my $target = $_[0]; my $location = "/tmp/dir1"; my $foundfile; $target = qr/$target/; my $filename = find(sub { ($foundfile = $File::Find::name) if $File::Find::name =~ $targ +et; }, $location); print $filename; }
    But even with this, do you really want to search all subdirectories of /tmp/dir1 with find()? Or is -f "$location/target" what you want? Is "$target" supposed to be a regular expression? Or just the file basename (in which case you would want "if $_ eq $target" in the function)?

    Update: Oops. I removed the accidentally left in "return $foundfile".

      • "But even with this, do you really want to search all subdirectories of /tmp/dir1 with find()? "
        - Well, (he says hesitatingly) I think I do. What I do in my real script is have a hash tied to a part of the regex ($target in this script) that gets me closer to where I want to look($location) for the regex filename. Even then it has to search through several hundreds of files before finding the right one. When it finds the right file it captures the name and path for later use. Are you thinking I'm not using find() correctly?
      • "Or is -f "$location/target" what you want?"
        - I know for sure I don't want to include the $location in the $target on my command line. Part of my reason for using find() is to be able to find the path for me by descending in to directories. So my command line would be something like
        $ script.pl -f test.txt
        and the end result is where in the directory structure test.txt is located.
      • "Is "$target" supposed to be a regular expression?"
        - It's a regex.
      • "Or just the file basename (in which case you would want "if $_ eq $target" in the function)?"
        Ok, so you do want to descend into sub-directories. But File::Find doesn't easily let you stop searching when you've found your file, so you could either use something like File::Next, or wrap the call to find() in an eval {} block, and die when you've found the file. Something like:
        eval { find(sub { ($foundfile = $File::Find::name) if $File::Find::name =~ $target; die "FoundFile" if $foundfile; }, $location) }; die $@ if $@ and $@ !~ /FoundFile/;
Re: Adding a dispatch table and getting "Variable $x will not stay shared" errors.
by Lawliet (Curate) on Mar 03, 2009 at 01:02 UTC

    Not entirely on topic, but have you considered using one of the Getopt modules? It is usually the best way to go when dealing with command line flags. Personally, I recommend Getopt::Long.

    There are sundry examples of Getopt::Long examples in The Monastery's archives. Supersearch if you need help. :)

    And you didn't even know bears could type.