Re: logrotate

[CharlesClarkson]

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