The error message isn't coming from within your subroutine; it's coming from within your loop through the images, at line 15. It's telling you that the $dir variable is undefined on each trip through the loop, although you're correct that you only assigned to it once, before the loop.

I only have time for a quick glance right now, but I suspect that your "last if not $made" logic has gotten things a little confused. To me, that says to break out of the loop if the directory was NOT made. If it was NOT made (which would be the case if your first attempt at picking a name picked one that already existed), then $made will be false, so your first return will not trigger, and your second return will return undef, which is assigned to $die. You probably want "last if $made", because you want to break out of the loop once the directory is created, right?

Thx aaron and AM, I've got versions working now that are pretty close to what you prescribed:

$ perl tg4.pl
site_1 
site_2 
site_3 
site_4 
site_5 
site_6 
site_7 
site_8 
site_9 
site_10 
site_11 
site_12 
site_13 
site_14 
site_14 
$ cd site_14
$ ls
image_1   image_2   image_3   image_4   image_5   image_6   image_7   
+image_8
...
$ cd ..
$ cat tg4.pl
#!/usr/bin/perl -w
use strict;
use WWW::Mechanize;
use LWP::Simple;
use feature ':5.10';

# get information about images
my $domain = 'http://www.yahoo.com';
my $m = WWW::Mechanize->new(); 
$m->get( $domain);
my @list = $m->images();

# create new folder and download images to it.
my $counter = 0;
my $dir = &DirPP;
for my $img (@list) {
    my $url = $img->url_abs();
    $counter++;
  
    my $filename = "$dir". "/image_". "$counter";  #line 15
    getstore($url,$filename) or die "Can't download '$url': $@\n";
}
sub DirPP {
    use Errno qw/ EACCES /; # permission denied
    my $word    = "site";
    my $counter2 = 1;
    my $name;
    my $made = 0;
    while(1){
        $name = "$word" . "_" . "$counter2";
        $counter2++;
        print "$name \n";
        last if $made = mkdir $name, 0755 ;
        die $! if $! == EACCES;
    }
    print "$name \n";
    return $name if $made;
    return;
}
[download]

Again, I'm looking for tips on style or cautions for robustness. Since one can now demonstrate two ways to do this task, reasonable people could disagree on the final touches, and the discussion might help me to understand these routines better. Here's the one that uses a state variable. Do I use it correctly?

 $ perl tg5.pl
site_1 
site_2 
site_3 
site_4 
site_5 
site_6 
site_7 
site_8 
site_9 
site_10 
site_11 
site_12 
site_13 
site_14 
site_15 
site_16 
site_16 
$ cat tg5.pl
#!/usr/bin/perl -w
use strict;
use WWW::Mechanize;
use LWP::Simple;
use feature ':5.10';
use feature qw/ state /;
state $counter3 = 1;

# get information about images
my $domain = 'http://www.yahoo.com';
my $m = WWW::Mechanize->new(); 
$m->get( $domain);
my @list = $m->images();

# create new folder and download images to it.
my $counter = 0;
my $dir = &DirPP;
for my $img (@list) {
    my $url = $img->url_abs();
    $counter++;
  
    my $filename = "$dir". "/image_". "$counter";  #line 15
    getstore($url,$filename) or die "Can't download '$url': $@\n";
}
sub DirPP {
    use Errno qw/ EACCES /; # permission denied
    my $word    = "site";
    my $name;
    my $made = 0;
    while(1){
        $name = "$word" . "_" . "$counter3";
        $counter3++;
        print "$name \n";
        last if $made = mkdir $name, 0755 ;
        die $! if $! == EACCES;
    }
    print "$name \n";
    return $name if $made;
    return;
}
[download]

Well, I have yet to use the state feature myself, so I may not be the best judge of that, but in this case, it doesn't gain you anything. If you just instantiate $counter3 normally with my, it will be available to subroutines within the file already, without needing state. It won't hurt anything; it just has no particular effect here.

With regard to your logic for finding a new directory name and creating it, I would be concerned about a different kind of race condition. If the mkdir fails because of permissions, your next line deals with that with die. Otherwise it keeps incrementing the counter until it finds a name that can be created. But what if the mkdir fails for some other reason, like EROFS or ENOSPC? Your loop will continue forever, incrementing and trying to mkdir until you kill the program. That's why my version did the mkdir after my do/while loop, and only used the loop to find an available name. As others pointed out, you still need an error check on the mkdir, just in case some other process creates the directory in that split second between your loop selecting the name and mkdir making it, but that's simpler than trying to specifically catch all the possible errors that mkdir might throw.

The double return is awkward, partly because it should never be possible to trigger the second one. Your while loop can only be exited if $made is true, in which case the first return will be executed, or by die, in which case neither return will be reached. The second return is thus a superfluous line that cannot be reached and looks like a possible typo or something.

One last stylistic note: You don't need double quotes to interpolate a single variable, so these are equivalent:

    my $filename = "$dir". "/image_". "$counter";
    my $filename = $dir. "/image_". $counter;
    my $filename = "$dir/image_$counter";
[download]

Aaron B.
My Woefully Neglected Blog, where I occasionally mention Perl.

Yeah, autocheck should have been really called autodie instead, my bad