http://www.perlmonks.org?node_id=124855


in reply to logrotate

You don't need $confignr. Changing the for a bit will load $_ with each array element. (Note: for and foreach are the same.)

foreach (@config) { next unless /^(\b)\s+(\b)\s+(\b)/; my $file = $1; my $versions = $2; my $pidfile = $3;

the next unless /^(\b)\s+(\b)\s+(\b)/; line adds some error checking.

BTW, does that regex work?

The subroutine could be indented to highlight it better

Gzip => 'lib', Post => sub{ open(IN, $pidfile); $appid = <IN>; chomp $appid; kill("HUP", $appid); }, Dir => '/var/log/old', Flock => 'yes',
might look like:
File => $file, Count => $versions, Gzip => 'lib', Post => sub{ open(IN, $pidfile) or die "$pidfile: $!"; $appid = <IN>; chomp $appid; kill("HUP", $appid); }, Dir => '/var/log/old', Flock => 'yes',

You really should check the open for success. I'm uncertain why $appid is defined with our. Can you explain? Try testing $appid before killing it.

There should be no need to undef $log; as $log is lexically scoped to the foreach block.

instead of:
if (defined $log) { print "Looks ok..\n"; } else { die "An error accured processing config data\n"; } $log->rotate(); undef $log; }
how about:
defined $log or die "An error accured processing config data\n"; print "Looks ok..\n"; $log->rotate(); }



HTH,
Charles K. Clarkson



The vast majority of the Earth is underground. - R. Zubrin