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

Hello, I am working on a script that will enable a user to remove files from a fixed directory. I have been able to get the script to list all of the files in a certain $location and allow the the client to check the files that need to be removed, but for some reason it only removes the last "checked" file. Here's what I'm working with:
read(STDIN, $buffer, $ENV{'CONTENT_LENGTH'}); @pairs=split(/&/,$buffer); foreach $pair (@pairs) { ($name, $value)=split(/=/,$pair); $value =~ tr/+/ /; $value=~s/%(..)/pack("c",hex($1))/ge; $f{$name}=$value; chomp($f{$name}); } print "<tr><td bgcolor=#CCCCCC><b>&nbsp;File Removal</b></td></tr></ta +ble>"; if(($f{'action'} eq "delete")&&(open(FILE,$f{'file'}))){ unlink($f{'file'})||print "<h1>Could Not Delete: ".$f{'file'}."</h +1>"; } $dir=$f{'dir'} if $f{'password'} eq $password; opendir(DIR,$dir); @files=readdir(DIR); closedir(DIR); @files=sort(@files); print "Directory Content:"; $i=0; foreach $file(@files){ if((!opendir(TEST,$dir."/".$file))&&($f{'password'} eq $password)& +&($file ne ".")&&($file ne "..")){ $i++; print "<input type=checkbox name=file value=\"".$dir."/".$file +; if($i==1){ print "\" checked>"; }else{ print "\">"; } print $file."<BR>\n"; } } print "<input type=hidden name=dir value=\"".$location."\">\n";
Any suggestions on how to remove multiples files instead of just one at a time? Thanks! Lisaw

Replies are listed 'Best First'.
Re: Deleting Multiple Files
by shemp (Deacon) on Jun 04, 2003 at 21:19 UTC
    First off, you should use CGI.pm, then you dont have to manually parse the form input.

    You may want to look at the contents of $f{'file'}, in relation to how unlink expects its parameter(s). Here's valid and invalid uses of unlink:
    unlink "a", "b", "c"; # valid unlink "a b c"; # invalid
    I think that your code generates the invalid version. Heres (stripped down) code using CGI.pm
    ... use CGI; my $form = CGI->new(); if ( $form->param('action'} eq "delete" ) { my @files_to_delete = $form->param('chosen_files'); unlink join ",", @files_to_delete; } ...
    hope this helps.
    shemp

    if not, please include output from:
    print $f{'file'};
      unlink join ",", @files_to_delete;

      I don't think that is going to do what you expect it to do. If you have three files named a, b and c, it is literally going to try to delete a file named a,b,c. What you need is simply:

      my $nr = unlink @files_to_delete; if( $nr != scalar @files_to_delete ) { print "uh-oh: helpful error message here"; }

      I would, however, make the script run with taint checks enabled, and carefully untaint the filenames before passing them to unlink. I would also deal with each file one by one anyway:

      for( @files_to_delete ) { # untaint $_ here unlink or print "failed to remove $_: $!\n"; }

      update: looking at the OP more carefully, there are two other points to be made: firstly, you don't need to open a file in order to remove it. If you want to test that it's there you can use the -f operator. But I would just try and delete it and see what happens. Some things you can't test for, you just have to try it and see whether it works.

      Secondly, unlink works in terms of the process' current directory. This might not be where you think it is. To remove the possibility of unexpected surprises, you should really be doing something like unlink "$path_to/$_" in my snippet above, where $path_to contains the name of the the directory /all/the/way/from/the/top.

      _____________________________________________
      Come to YAPC::Europe 2003 in Paris, 23-25 July 2003.

        Ah shoot, you're correct, it should just be
        unlink @files_to_delete;
        brainfart
        shemp
        I seem to be running into a wall converting this over to use CGI. I keep getting an error 500