Re: Is this script safe?
- From: "Paul Lalli" <mritty@xxxxxxxxx>
- Date: 27 Oct 2005 08:09:26 -0700
Dermot Paikkos wrote:
> I wanted a script that would rename files from lower to upper case. I
> have something but I am a bit worried about going live with it as I
> can imagine there is plenty of room for error and I really don't want
> to knacker my filesystem.
>
> Could I get any comments of suggestion on what I have. I want
> something safe/secure.
>
> I am not sure about what would happen if it was passed more than one
> argument.
Since you're only shifting off the first argument from @ARGV, any other
arguments would be completely ignored. Only the directory representing
the first argument would be processed.
> Should I allow more than one argument?
That is entirely up to you. It would be pretty simple to use the `my
$dir = shift` expression in the body of a while loop. Or you could
simply put a check for the size of @ARGV, and exit the program if the
user enters more than one argument.
> Are the any gotcha's to watch out for?
>
> =============== upper.pl===============
>
> #!/usr/bin/perl -Tw
> # upper.pl
>
> # Upper case all lowercase file in a given directory.
> use File::Copy;
> use strict;
>
> my $dir = shift;
> my $found = 0;
>
> opendir(DIR,$dir) or die "Can't open $dir: $!\n";
> # Credit Randal L. Schwartz
> foreach my $name (sort grep !/^\./, readdir DIR) {
Why do you care if the files are processed in a sorted order?
> # Look for lc file. Wot about files with numbers??
What about them? What is your concern?
> if ($name =~ /[a-z]/) {
You understand that this matches every file that contains at least one
lowercase letter, right? I don't know if that's what you want or not.
More to the point, why are you bothering testing at all. If the
filename doesn't contain any lowercase letters, nothing will happen to
the filename.
> ++$found;
> if ($dir !~ /\/$/) { # Add a slash if there is'nt one
> $dir = "$dir"."/";
You are performing this test for every single file found. That seems
needless. Just do it once, at the beginning of the script, before
opening the directory. Also, you're using a variable in double quotes
where none is needed. This creates more work for the interpreter (see:
perldoc -q quoting)
$dir .= '/';
or
$dir = $dir . '/';
or
$dir = "$dir/";
> }
> (my $new = $name) =~ tr/[a-z]/[A-Z]/; # trans the name
That is not proper use of the tr/// operator. It *works*, but only by
happenstance. [ ] are used in a character class in a regular
expression. tr/// does not use regular expressions. The above is
actually saying: "Replace all [ with [, all a with A, all b with B,
...., all z with Z, and all ] with ]"
(my $new = $name) =~ tr/a-z/A-Z/;
Or, perhaps easier to read: my $new = uc($name);
> $name = "$dir"."$name";
> $new = "$dir"."$new";
More senseless doublequoting
$name = "$dir$name";
$new = "$dir$new";
or
$name = $dir . $name;
$new = $dir . $new;
or, my preference: reverse that test for the slash above, and *remove*
the slash if it exists. Then your concatenation will *look* like what
you're actually trying to do:
$name = "$dir/$name";
> #mv("$name","$new") or die "Can't upper $name: $!\n";
And again with the double quotes.
Also, IIRC, mv() is not exported by default by File::Copy. You have to
specifically request the shortened versions (cp and mv), or use the
longer versions (copy and move)
> print "$name -> $new\n";
> }
>
> }
> print "Found $found lowercase files\n"
Here's how I would probably write this (untested):
#!/usr/bin/perl
use strict;
use warnings;
use File::Copy;
for my $dir (@ARGV){
opendir my $dh, $dir or die "Couldn't open directory '$dir': $!\n";
chdir $dir or die "Could not change to directory '$dir': $!\n";
while (my $file = readdir($dh)){
next if $file =~ /^\./; #skip 'hidden' files
move $file, uc($file) or die "Could not rename $file to uppercase:
$!\n";
}
}
__END__
Hope this helps,
Paul Lalli
.
- References:
- Is this script safe?
- From: Dermot Paikkos
- Is this script safe?
- Prev by Date: Re: Is this script safe?
- Next by Date: input validation and persistency module for (mod_perl) web apps?
- Previous by thread: Re: Is this script safe?
- Next by thread: Re: Is this script safe?
- Index(es):
Relevant Pages
|
