in reply to WhichHost AKA pingtest.pl
You are not testing the success of your open calls, and as they are in fact pipes, they are, in my experience, more prone to failure than regular files. To make matters worse, you are relying (hoping) that a program named 'mail' in somewhere on your PATH when the script is run. (But which one)? Not to mention trickier things like Net::Telnet objects.
Your firewalls could roll belly up, and you wouldn't even know about it... Always always always check the results of system calls.
Other minor issue: you should initialise an array with my @errors = (), not with my @errors = "". That said, it is a laudable practice to explicitly initialise variables. I find things are a lot clearer when spelt out:
my $widget = undef; while( <> ) { if( useable($_) ) { $widget = $_; last; } } if( defined $widget ) { ... }
The undef and subsequent defined form a pair. It makes it easy to see that the purpose of the loop is to get $widget out of the undefined state. Hmm, but I digress :)
Don't do things like $availablefirewall = "$backupfirewall". Omit needless string interpolation.
It's poor practice to munge arrays passed to subroutines. You call Notify() so:
And retrieve the parameters in Notify() thusly:Notify ($key, $script, @errors, @message);
my $key = shift @_; my $script = shift @_; my $errors = "@_";
Someone else reading this code may puzzle for a while wondering where the fourth parameter disappeared to. Merge the arrays explicitly before passing them, or pass them by reference. Also, when you my $errors = "@_"; it's good practice to precede it with a local $" = ' '; It's a habit that will prevent you from shooting yourself in the foot in the future.
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re: Re: WhichHost AKA pingtest.pl
by cybear (Monk) on Jun 03, 2002 at 23:55 UTC |