On Friday 15 August 2003 14:16, Keith Olmstead wrote:
>
> Hello,
Hello,
> I have this script to gzip logs in different directories. Can some
> one check it for me. It works fine, but I want to make sure I did it
> correctly and the best way.
If it works then it must be correct! :-)
As to "the best way", Perl's motto is "There's more then one way to do
it".
> BTW, this is my frist script that I wrote.
Good to see that you got it working correctly.
> #!/usr/bin/perl
The next two lines should be:
use warnings;
use strict;
> my $startdir = "/opt/log/hosts/";
You usually use double quotes if you require perl to interpolate the
string contents however there is no interpolation required so you
should use single quotes instead. Also the path separator (/) is not
required at the end of the path.
> use File::Find;
> use warnings;
> my @dirlist;
> $date=&getDate;
You shouldn't use an ampersand (&) on a subroutine call. The reasons
are explained in the perlsub.pod document.
perldoc perlsub
my $date = getDate();
> @logfile = ("cron","messages","maillog","ldap");
> foreach $log (@logfile) {
> find (\&eachFile, "$startdir");
You shouldn't quote variables.
perldoc -q quoting
find( \&eachFile, $startdir );
> }
>
> foreach $file(@log){
> system("gzip $file") unless ($file =~ m/$date/);
You should verify that system() did not fail.
unless ( $file =~ /$date/ ) {
system( "gzip $file" ) == 0
or die "system 'gzip $file' failed: $?";
}
> #print "$file done!\n" unless ($file =~ m/$date/);
> } # End for loop
>
> sub eachFile {
> if (-e $_ && $_ =~ /$log$/) { push @log, $File::Find::name;}
File::Find gets the file names of files it finds so they must exist for
File::Find to find them so "-e $_" is redundant.
push @log, $File::Find::name if /$log$/;
> } # End eachFile
>
> sub getDate {
> my ($sec, $min, $hour, $mday, $mon, $year) = localtime(time);
You are not using $sec, $min and $hour so there is no reason to declare
them.
my ( undef, undef, undef, $mday, $mon, $year ) = localtime;
Or:
my ( $mday, $mon, $year ) = ( localtime )[ 3 .. 5 ];
> $year = $year + 1900;
> $mon++;
> if ($mon < 10) {$mon = "0" . $mon;}
> if ($mday < 10) {$mday = "0" . $mday;}
> if ($hour < 10) {$hour = "0" . $mday;}
> if ($min < 10) {$min = "0" . $mday;}
> if ($sec < 10) {$sec = "0" . $mday;}
You would usually use sprintf to zero pad numbers.
> $date = "\/$year\/$mon\/$mday\/";
You don't need to escape slashes (see your $startdir variable.)
> return ($date);
my ( $mday, $mon, $year ) = ( localtime )[ 3 .. 5 ];
return sprintf '%04d/%02d/%02d', $year + 1900, $mon + 1, $mday;
Or you could use POSIX::strftime.
use POSIX 'strftime';
return strftime '%Y/%m/%d', localtime;
> } # End getDate
John
--
use Perl;
program
fulfillment
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
