Linted old scripts

gcoxmoz · Oct 5, 2017 · f48a40a · f48a40a
1 parent 0f3dea2
commit f48a40a
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 48 deletions.
diff --git a/scripts/check_junction_mounts.pl b/scripts/check_junction_mounts.pl
@@ -3,8 +3,9 @@
 # gcox@mozilla
 #
 use strict;
+use warnings;
 use Getopt::Long;
-$ENV{'PATH'} = '';  # Taint-check safety
+local $ENV{'PATH'} = '';  # Taint-check safety
 
 #
 # Designed to report issues with NetApp junction mounts
@@ -57,38 +58,45 @@
 }
 
 my $unchecked_mountpath = shift @ARGV;
-   $unchecked_mountpath =~ m#^(/[-/_a-zA-Z0-9]+)#;  #Could be overrestrictive, but, avoiding dangerous input
-my $mountpath = $1;
-if (!$mountpath) {
+my $mountpath = undef;
+if ($unchecked_mountpath =~ m#^(/[-/_a-zA-Z0-9]+)#) {
+    # Could be overrestrictive, but, avoiding dangerous input
+    $mountpath = $1;
+} else {
     print "Didn't safely regex-match a mount path.  Funky character in the name?\n";
     exit 3;
-} elsif (!(-d $mountpath)) {
+}
+
+if (!(-d $mountpath)) {
     print "$mountpath is not a directory.\n";
     exit 3;
 }
 
 my $overall_exit_code = 0;
-sub set_exit_code_warning () {
+sub set_exit_code_warning {
     $overall_exit_code = 1 if ($overall_exit_code < 1);
+    return 1;
 }
-sub set_exit_code_critical () {
+sub set_exit_code_critical {
     $overall_exit_code = 2 if ($overall_exit_code < 2);
+    return 1;
 }
 
 my $insane_percentage = 1001+int(rand(100));
 my @mounts = ();
-open  PROCMOUNTS, '</proc/mounts' or die "Can't open /proc/mounts: $!\n";
-foreach my $line (<PROCMOUNTS>) {
-    next unless ($line =~ m#^\S+\s+($mountpath\S*)\s#);
-    push @mounts, $1;
+open  my $procmounts_fh, '<', '/proc/mounts' or die "Can't open /proc/mounts: $!\n";
+while (my $line = <$procmounts_fh>) {
+    if ($line =~ m#^\S+\s+($mountpath\S*)\s#) {
+        push @mounts, $1;
+    }
 }
-close PROCMOUNTS or die "Can't close /proc/mounts: $!\n";
+close $procmounts_fh or die "Can't close /proc/mounts: $!\n";
 
 
 # Sample:
 # Filesystem         1024-blocks      Used Available Capacity Mounted on
 # server:/some/dir 689944080 387792528 246956016      62% /mnt/point
-sub df_mount($$) {
+sub df_mount {
     my ($fs, $space_or_inodes, ) = @_;
     my %df_commands = (
         'space'  => ['/bin/df', '-kP',],
@@ -103,18 +111,18 @@ ($$)
 
     eval {
 
-      sub pipe_from_fork ($) {
+      sub pipe_from_fork {
           my $parent = shift;
           no strict 'refs';
-              pipe $parent, my $child or die;
+              pipe $parent, my $child or die "Could not open pipe: $!\n";
           use strict 'refs';
           my $pid = fork();
-          die "fork() failed: $!" unless defined $pid;
+          die "fork() failed: $!\n" unless defined $pid;
           if ($pid) {
               close $child;
           } else {
               close $parent;
-              open(STDOUT, '>&=' . fileno($child)) or die;
+              open(STDOUT, '>&=', fileno($child)) or die "Could not open against $child: $!\n";
           }
           $pid;
       }

diff --git a/scripts/oversubscription-check.pl b/scripts/oversubscription-check.pl
@@ -3,6 +3,7 @@
 # gcox@mozilla
 #
 use strict;
+use warnings;
 
 #
 # Against a cDOT filer, jump in (assuming you have keys) as the cluster admin
@@ -12,41 +13,43 @@
 my $filer = shift or die "Which filer?\n";
 chomp $filer;
 
-open (AGS, 'ssh admin@'.$filer.' aggr show -fields aggregate -has-mroot false |');
-<AGS>;
-<AGS>;
+open (my $ags_fh, '-|', 'ssh admin@'.$filer.' aggr show -fields aggregate -has-mroot false') or die "Can't open a command pipe for aggr show: $!\n";
+<$ags_fh>;
+<$ags_fh>;
 my @aggrs;
-while (<AGS>) {
-  last if (m#were displayed#);
-  s#\s*\r?\n$##;
-  push @aggrs, $_;
+while (<$ags_fh>) {
+   last if (m#were displayed#);
+   s#\s*\r?\n$##;
+   push @aggrs, $_;
 }
-close AGS;
+close $ags_fh;
 
 foreach my $aggr (@aggrs) {
-   my %data = ();
-   my $cmd = '';
+    my %data = ();
+    my $cmd = '';
 
-   $cmd = 'ssh admin@'.$filer.' "set -units gb ; aggr show -aggregate '.$aggr.' -fields size"';
-   open (CMD1, $cmd.'|');
-   foreach my $line (<CMD1>) {
-     next unless ( $line =~ m#$aggr\s+([0-9]+)GB\b# );
-     $data{$aggr}{'agsize'} = $1;
-     last;
-   }
-   close CMD1;
+    $cmd = 'ssh admin@'.$filer.' "set -units gb ; aggr show -aggregate '.$aggr.' -fields size"';
+    open (my $cmd1_fh, '-|', $cmd) or die "Can't open a command pipe for '$cmd': $!\n";
+    while (my $line = <$cmd1_fh>) {
+        if ( $line =~ m#$aggr\s+([0-9]+)GB\b# ) {
+            $data{$aggr}{'agsize'} = $1;
+            last;
+        }
+    }
+    close $cmd1_fh;
 
-   $data{$aggr}{'volsize'} = 0;
-   $cmd = 'ssh admin@'.$filer.' "set -units gb ; volume show -aggregate '.$aggr.' -fields size"';
-   open (CMD2, $cmd.'|');
-   foreach my $line (<CMD2>) {
-     next unless ( $line =~ m#.*\s+([0-9]+)GB\b# );
-     $data{$aggr}{'volsize'} += $1;
-   }
-   close CMD2;
-   my $overage = $data{$aggr}{'volsize'} - $data{$aggr}{'agsize'};
-   my $amt = abs($overage);
-   my $warning = (($overage > 0) ? 'OVER ' : 'under'). ':';
-   my $ratio = int($data{$aggr}{'volsize'} * 10000 / $data{$aggr}{'agsize'}) / 100 ;
-   printf "%-30s \tAg: %7iGB\tVols: %7iGB\t%-6s %7iGB (%6.2f%%)\n", $aggr, $data{$aggr}{'agsize'}, $data{$aggr}{'volsize'}, $warning, $amt, $ratio ;
+    $data{$aggr}{'volsize'} = 0;
+    $cmd = 'ssh admin@'.$filer.' "set -units gb ; volume show -aggregate '.$aggr.' -fields size"';
+    open (my $cmd2_fh, '-|', $cmd) or die "Can't open a command pipe for '$cmd': $!\n";
+    while (my $line = <$cmd2_fh>) {
+        if ( $line =~ m#.*\s+([0-9]+)GB\b# ) {
+            $data{$aggr}{'volsize'} += $1;
+        }
+    }
+    close $cmd2_fh;
+    my $overage = $data{$aggr}{'volsize'} - $data{$aggr}{'agsize'};
+    my $amt = abs($overage);
+    my $warning = (($overage > 0) ? 'OVER ' : 'under'). ':';
+    my $ratio = int($data{$aggr}{'volsize'} * 10000 / $data{$aggr}{'agsize'}) / 100 ;
+    printf "%-30s \tAg: %7iGB\tVols: %7iGB\t%-6s %7iGB (%6.2f%%)\n", $aggr, $data{$aggr}{'agsize'}, $data{$aggr}{'volsize'}, $warning, $amt, $ratio ;
 }