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

I received advice on CB today and while I am trying to implement the advice into my sub I find it deleting all files not just files with .bak extensions, even trying to delete the directories. The example that was given to me was for a print, I tried to translate the example into a system call and a print but I am having trouble with it.

Here is the example given to me on cb

@matches; find(sub { push @matches, $File::Find::name if /\.bak\z/ }, $search_dir);

And here is my attempt of implementing it into my script

use strict; use warnings; use File::Find; deleteBak(); sub deleteBak { my $searchdir = '/emc/cccadm/scripts/perl/etc-test'; my ($answer, $file, @bak); print "Are you sure you would like to delete all *.bak files that +exist in: $searchdir [yes/no] "; chomp($answer = <STDIN>); if ($answer eq 'yes'){ my @bak; find(sub { @bak = ($File::Find::name) if /\.bak\z/ or print ("No File +s\n"); foreach $file (@bak) { system("rm -f $file"); print "Deleting $file\n"; } }, $searchdir); } else { print "You did not enter [yes]. Exiting program.\n"; exit; } }

Replies are listed 'Best First'.
Re: Need help deleting *.bak files
by ikegami (Patriarch) on Jan 18, 2010 at 21:59 UTC
    • Poor division of work. Input, processing and output are all intermixed.

    • You have two vars named @bak. One's never used, the other is declared before it should be.

    • You only ever put one element in the array @bak. Maybe you shouldn't be using an array.

    • The whole point of using File::Find was to avoid using the shell. But then you go and pass filenames to the shell without converting them to shell literals first. That's buggy. The most common failure is that it won't delete the files you intend to delete. (Consider a file with a space in its name.) It could also delete files you did not intend to delete, and it can even execute arbitrary commands. Worst case, imagine if someone created a file named "; rm -rf ~ ; .bak". You can use the multi-argument form of system to avoid the shell, or you could use unlink.

    use strict; use warnings; use File::Find qw( find ); my $search_dir = '/emc/cccadm/scripts/perl/etc-test'; { print "Are you sure you would like to delete all *.bak ". "files that exist in: $search_dir [yes/no] "; chomp( my $answer = lc(<STDIN>) ); if ($answer ne 'y' && $answer ne 'yes') { die("You did not enter [yes]. Exiting program.\n"); } my @bak = find_bak_files($search_dir); if (!@bak) { # Why is this worthy of being said? warn("No files were found\n"); } for (@bak) { unlink($_) or warn("Can't delete $_: $!\n"); } } sub find_bak_files { my ($dir) = @_; my @bak; find( sub { push @bak, $File::Find::name if /\.bak\z/ && -f; }, $dir); return @bak; }

    If it wasn't for the useless warning, you could shorten the above to

    use strict; use warnings; use File::Find qw( find ); my $search_dir = '/emc/cccadm/scripts/perl/etc-test'; { print "Are you sure you would like to delete all *.bak ". "files that exist in: $search_dir [yes/no] "; chomp( my $answer = lc(<STDIN>) ); if ($answer ne 'y' && $answer ne 'yes') { die("You did not enter [yes]. Exiting program.\n"); } exit(1) if !delete_bak_files($search_dir); } sub delete_bak_files { my ($dir) = @_; my $success = 1; find( sub { if (/\.bak\z/ && -f) { if (!unlink($_)) { warn("Can't delete $File::Find::name: $!\n"); $success = 0; } } }, $dir); return $success; }
Re: Need help deleting *.bak files
by johngg (Canon) on Jan 18, 2010 at 22:02 UTC

    The first thing that leaps out from your code is the example you were give uses push to add each item to the array whereas you just assign a different single element list to the array each time around. Also, consider using unlink to remove your files and check for success.

    Cheers,

    JohnGG

Re: Need help deleting *.bak files
by Anonymous Monk on Jan 19, 2010 at 00:26 UTC
    A simpler use of find is in order:
    find(sub{ print("$File::Find::name: ", unlink() ? "success" : "failure ($!)" ) if /\.bak\z/; }, $searchdir);
      It's traditional to only be silent on success, and display errors on STDERR. It would also be useful to say what failed.
      find(sub{ if (/\.bak\z/ && -f) { unlink() or warn("Can't delete $File::Find::name: $!\n"); } }, $dir);
        Personally, I'd print everything, save to a file, and grep for errors; assuming this is a "manual" deletion, not part of a bigger project.
Re: Need help deleting *.bak files
by ahmad (Hermit) on Jan 19, 2010 at 15:13 UTC

    You could do it this way, it won't be harmful as unlink won't delete directories so it will always be a file that ends with .bak extension.

    unlink glob '/emc/cccadm/scripts/perl/etc-test/*.bak';
      That's not recursive like the OP's code. It also limits the ability to report errors when you pass more than one file name to unlink.