#!/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 '
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 <
Example: perl $0 192.168.0
Sort: perl $0 sort
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=;
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;
}