3
\$\begingroup\$

I'm looking for input on the code itself and formatting/style. This script parses the output of df -h on a Linux server and emails the user if the usage percentage is above the threshold (90% in this case).

#!/usr/bin/perl

use warnings;
use strict;
use Mail::Sendmail;

my $svr = `hostname`;
my @output = `df -h`;
my $msg;
my $mailto = 'someone@foo.com';
my $threshold = 90;

sub main
{
  foreach (@output) {
    if (($_ =~ m/(\d+)% (.*)/) && ( $1 > $threshold )) {
      chomp($svr); $msg = "$svr: $2 volume  is $1 percent full\n" }
    }

    my %mail = ( To      => $mailto,
                 From    => 'Server@foo.com',
                 Subject => "$svr has a full file system",
                 Message => $msg
               );

  if ( defined $msg ) {
    sendmail(%mail) or die $Mail::Sendmail::error;
  }

}

main
\$\endgroup\$
3
\$\begingroup\$

There is a bug here:

foreach (@output) {
    if (($_ =~ m/(\d+)% (.*)/) && ( $1 > $threshold )) {
      chomp($svr); $msg = "$svr: $2 volume  is $1 percent full\n" }
    }

If there are more than one partitions full, $msg will contain only the last one. I think you want to append to $msg instead of overwriting. I suggest to rewrite like this:

chomp(my $svr = `hostname`);

foreach (@output) {
    if (m/(\d+)% (.*)/ && $1 > $threshold) {
        $msg .= "$svr: $2 volume is $1 percent full\n";
    }
}

Explanation of other improvements:

  • Removed unnecessary brackets from the if condition
  • Simplified $_ =~ m// to just m// which does the same
  • Instead of chomp($svr) for each line, do it only once, right after initialization
  • Reindented, so it's easier to see the block of code that belongs to the loop

Lastly, you can drop "defined" in if ( defined $msg ) {

\$\endgroup\$
1
  • \$\begingroup\$ All excellent improvements, thanks! I will make these changes to the code. \$\endgroup\$ – Gregg Leventhal Jul 19 '14 at 18:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.