Yet another problem with recursive code

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

Can anyone tell me why I ran out of memory when I ran this code. It is a simple directory search that starts off where you are and searched to the end of that directory. However, when I ran it, it gave me an illegal operation error and said that I was out of memory. I believe the problem is in the @ARGV statement in the first sub function. Any clues?

#!/usr/bin/perl -ws

use strict;
use Cwd;

my $mode;

&op;
&scandir("C:/mydocu~1");

sub op
{
    if (defined (@ARGV) $ARGV[0]=~/-all/)
    {
        $mode="all";
    }else{
        $mode="";
     }
}

sub scandir
{

my ($workdir) = &scandir;
my ($startdir) = &cwd;

chdir($workdir) || die "Unable to change to $workdir: $!\n";
opendir(DIR, ".") || die "Unable to open $workdir: $!\n";
my @files = readdir(DIR) || die "Unable to read $workdir: $!\n";
closedir(DIR) || die "Unable to close $workdir: $!\n";

foreach my $file (@files)
{
    next if ($file eq ".");
    next if ($file eq "..");

    if ($mode eq "all")
    {
        if (-d $file)
        {
            &scandir;
            next;
        }

        if (-f $file)
        {
            if($file =~ m/\.\d+\w+\-\d+\wm$/i || 
                $file =~ m/\.log$/i)
            {
                unlink($file) || warn "Unable to delete: $file: $!\n";
            }else{
                print "No files to delete!\n";
                 }
        }
    }
    elsif ($mode eq "")
    {
        if(-d $file)
        {
            &scandir;
            next;
        }

        if (-f $file)
        {
            if($file =~ m/\.\d+\w+\-\d+\wm$/i)
            {
                unlink($file) || warn "Unable to delete: $workdir\/$fi
+le: $!\n";
            }else{
                print "No files to delete!\n";
                 }
            }
        }
      chdir($startdir) || die "Unable to change to $startdir: $!\n";
    }
}

&scandir(".");
[download]

Thanks in advance curtisb -- "Be careful what you wish for, it may come back and bit you in the ass!"

Comment on Yet another problem with recursive code Download Code

Replies are listed 'Best First'.
Re: Yet another problem with recursive code by Fastolfe (Vicar) on Nov 10, 2000 at 02:22 UTC
Two words: deep recursion. Check this out: `sub scandir { my ($workdir) = &scandir; ...` [download] The very first thing you do inside your 'scandir' sub is call it again. This is probably not what you want.	[reply] [d/l]
Re: Yet another problem with recursive code by btrott (Parson) on Nov 10, 2000 at 02:26 UTC
To elaborate a bit on what Fastolfe wrote: when you write recursive functions you need to have stop conditions (which may have, um, a more technical name :). But the point is, you need to stop at some point! Think about it from a logical perspective: do you want your program to run forever? If you were recursing through a dir. structure by hand, where would you stop? When you could no longer find any sub-directories, right? Sorry if that seems incredibly basic etc.; in your case you could've just forgotten to write a stop condition. But I can't know that, can I. That said, use File::Find. :)	[reply]
Re: Yet another problem with recursive code by Fastolfe (Vicar) on Nov 10, 2000 at 02:27 UTC
In addition, this code is weird: `if (defined (@ARGV) $ARGV[0]=~/-all/) { $mode="all"; } else { $mode=""; }` [download] What is that `$ARGV[0]=~/-all/` doing out there? Do you want an `&&` or `and` in there? Also: `if (-d $file) { &scandir;` [download] You're not chdiring into `$file` first, nor are you passing that to `&scandir`, so it looks to me like it's just going to repeat itself in the current directory/context again. You're also calling `&scandir` twice in your code.. once towards the top and once at the bottom (for "."). Not sure if you intended to do that or not. It looks to me like you need to figure out how you're wanting your functions to be called, and perhaps re-work the way you're passing arguments around. Take a look at File::Find and File::Recurse also.	[reply] [d/l] [select]
(tye)Re: Yet another problem with recursive code by tye (Sage) on Nov 10, 2000 at 04:51 UTC
Never use `defined()` on arrays nor on hashes because it doesn't mean what you think it means. You want one of these: `if( @ARGV && $ARGV[0] eq "-all" ) { if( 0 < @ARGV && "-all" eq $ARGV[0] ) {` [download] An empty array can still be "defined" (or un"defined"), depending on obscure details about its history. Also, `$ARGV[0] =~ /-all/` will be true if your first argument is "tell-all books", for example, probably not what you wanted. - tye (but my friends call me "Tye")	[reply] [d/l] [select]
Re: Yet another problem with recursive code by blogan (Monk) on Nov 10, 2000 at 06:53 UTC
I wrote a subdirectory scanning program. I ran into trouble because I forgot to limit scope on a variable. The way I found it was by printing out my array everytime I went into the subfunction and everytime I came out of it. The hints above will help you with your problem. Hopefully this will help you in the future.	[reply]
Re: Yet another problem with recursive code by curtisb (Monk) on Nov 10, 2000 at 02:29 UTC
Thanks for the tips. I will take a look and use them. Thanks again. curtisb	[reply]