pinger made in perlmonks

pinger.pl

#!/usr/bin/perl -w
## Always use warnings
#
# Run with no arguments for help
#
# This program scans a C-class subnet and prints the results to corresponding text file.
# If a computer has once answered the ping, it will not be added to the list again.
# To sort the output type 'perl scansub.pl sort <textfile>'

use strict;
use Net::Ping;

## Unbuffer your putput so we see it in real time
$|++;

## Let's be nice and be case-insensitive..
sort_file($ARGV[1]) if lc $ARGV[0] eq 'sort';

my $subnet = shift @ARGV;
## Usage always exits, so we can be succint here..
usage() unless $subnet;

## Periods don't have to be escaped..
# open OF,">>$subnet.txt" or die "Can't create $subnet.txt: $!";
## ..and ALWAYS check the return value of open!
## And it turns out that we'll end up doing this shortly anyways, so we can take it out here.

## This look isn't useful, really.  Hash values start out being undefined.
# for (1 .. 255){
## You could also save yourself a variable AND roll it into one line.
#   $state{"$subnet.$_"}=''
# }

## I pulled this object out of the loop -- we can use the same Net::Ping object over and over.
my $p = Net::Ping->new('icmp');

my %state;
while () {
  ## Always check the return value of open..
  open OF,">>$subnet.txt" or die "Can't create $subnet.txt: $!";

  ## Unbuffer the OF filehandle -- that way, you can tail the file, and killing the process won't lose any data
  select OF;
  $|++;
  select STDOUT;

  ## Scope stuff as tightly as possible -- hence the my $h right here, not up top.
  for my $h (1 .. 254){
    #sleep 2;
    ## Ditto with scoping $host to this block.
    ## Again, period arn't special or anything in double quotes.
    my $host = "$subnet.$h";
    print "\nPinging host $host";
    if ($p->ping($host,1)) {
      ## If we didn't know it was up, spew that out now..
      print OF "$host\twas up at ", scalar localtime, "\n" unless $state{$host};
      ## Update the number of times we've seen it up
      $state{$host}++;
      ## And let STDOUT know about it, too.
      print " alive ($state{$host} times)";
    } else {
      print " down";
    }
  }
  close OF;
  sleep 600;
}

sub usage {
  ## HERE documents are your friend for this kind of thing:
  print <<"EO_USAGE";
This program scans a C-class subnet and lists
all machines, that have answered ping in a text
file.

You can also use this program to sort the output.

Usage:	        perl $0 <subnet>
Example:        perl $0 192.168.0

Sort:	        perl $0 sort <file>
Example:        perl $0 sort 192.168.0.txt
EO_USAGE
  exit;
}

## Naming subroutines the same name as builtins is vaguely sketchy,
## and probably good practice to avoid doing.
sub sort_file {
  ## Find out the filename by what we were passed.
  my $file = shift;

  usage() unless $file;

  ## Good error checking here.  ++
  open IF,"$file" or die "Cannot open file $file for read: $!";
  ## I moved the read and close all together up here 
  ## It's a little more obvious what you're doing this way
  my @data=<IF>;
  close IF;

  ## Not overly useful variable $of cleaned out -- it was only used once
  ## ..and always check the return value of open.  Is there a pattern here? ;>
  open OF,">s.$file" or die "Cannot open file s.$file for write: $!";

  ## Any particular reason you were using grep here instead of map?
  my @sorted = map {s/(^|\D)0+(\d)(?=\t)/$1$2/g; $_}
               sort
               map {s/(\d+)(?=\t)/sprintf"%03.3d",$1/ge; $_}
               @data;

  ## You can print it all in one go -- no need for a foreach.  Just print the array
  print OF @sorted;
  close OF;

  exit;
}