Some random comments on your code follow. Please keep in mind that I only casually perused (parts of) your script, so don't consider this a proper code review by any means. I also don't really understand much of the underlying functionality, so this is restricted to Perl-specific aspects of the code.
storing config options in a list of scalars vs. a hash
In process_command_line_args you initialize a long list of variables, and pass them back as an array reference:
sub process_command_line_args {
#set default values for dynamic config settings
my ($layout,$wait_for_child,$wait_for_exec,$prompt,$configf,$quiet
+,$outf,$debug,$batchsize,$proto,$tacacs,$verbose,$header,@maildst) =
+(0,120,3,1,"$name.cfg",1,"$name.csv",0,0,"none",0,0);
GetOptions ( ...)
...
my @args = ($layout,$wait_for_child,$wait_for_exec,$name,$prompt,$
+configf,$quiet,$outf,$debug,$batchsize,$proto,$tacacs,$verbose,$heade
+r,$mailref,$path);
return \@args;
}
Then in various subs (Main, validate_command_line_args, populate_config_hash) you access its elements via array subscripts and by repeatedly unpacking it into a long list of scalars again.
This seems like a really cumbersome way to deal with those values (making your script more difficult to maintain).
A hash would serve them much better I think, especially since in populate_config_hash you end up creating the %CONFIG hash from them anyways.
One possibility would be to use %CONFIG right from the start:
sub Main {
my %CONFIG;
initialize_config_hash(\%CONFIG);
get_command_line_args(\%CONFIG);
validate_command_line_args(\%CONFIG);
get_username_and_password_from_stdin(\%CONFIG) if $CONFIG{prompt};
...
}
sub initialize_config_hash {
my $CONFIG = shift;
%$CONFIG = (
quiet => 0 , cfg => "$name.cfg" ,
verbose => 0 , outcsv => "$name.csv" ,
debug => 0 , proto => "none" ,
prompt => 1 , header => undef ,
layout => 0 , maildst => [] ,
wait_for_child => 120 , mail_to => "" ,
wait_for_exec => 0 , mail_from => "NMS-script@me.net" ,
batchsize => 0 , mail_sub => "" ,
tacacs => 0 , mail_cc => "" ,
tacuser => "user" , mail_data => "" ,
tacpass => "pass" ,
);
}
sub get_command_line_args {
my $CONFIG = shift;
GetOptions (
'layout|l' => \$CONFIG->{layout},
'file-based-auth|f' => sub { \$CONFIG->{prompt} = 0; },
'prompt-based-auth|p' => sub { \$CONFIG->{prompt} = 1; },
...
)
...
}
calling 'next'/'last' to break out of a callback sub...
...is not a good idea.
In process_device you do:
$WARN = 0; #supress flow control warning in following block due to exi
+ting anon sub
LOOPCTRL: while ($lb < 10){
$exp->expect(...
[ ..., sub {...
$lb++;
next LOOPCTRL; }],
...
);
$exp->expect(...
[ ..., sub {...
last LOOPCTRL; }]
);
}
$WARN = 1; # reset to default
The warning is there for a reason: The callback now never returns to the code that called it (in this case, Expect.pm), so if that code needs to do any cleanup etc. before passing control back to you, that will be skipped which might lead to hard-to-track bugs.
I don't have hands-on experience with the Expect.pm module, but the documentation suggests that expect() returns a positive number on success and undef otherwise, so it seems to me that this loop could be re-written like so:
while (...) {
next if $exp->expect(...
...
);
last if $exp->expect(...
...
);
}
Suppressing the warning becomes unnecessary, and so is the "LOOPCTRL" label btw.
(Also make extra sure that those expect calls + loop control really result in the intended behavior... I can't help you there, because I have no understading of the external program you're controlling.)
suppressing warnings
In my experience, suppressing core Perl warnings is rarely the best solution - usually they are an indicator that what you're trying to do can be done in a better/safer way.
But even when you do want to temporarily suppress Perl warnings, using a global variable together with a global signal handler is not the proper way to do it. Instead, simply disable the "warnings" pragma within an as-small-as-possible lexical scope:
use strict;
use warnings;
# warnings coming from here will be printed
{
no warnings;
# warnings coming from here will NOT be printed
}
# warnings coming from here will be printed
Or, even better, only turn off specific warning categories instead of all at once - for example:
# Don't warn about exiting subs via loop control:
no warnings 'exiting';
# Don't warn about using undef in numeric or string operations:
no warnings 'uninitialized';
To suppress warnings emitted by modules (rather than Perl itself), first look if the module provides an option to turn them off. If not, overriding the warn signal handler is the way to go, but this too is better done locally instead of for the whole program:
{
local $SIG{__WARN__} = sub {};
# call the noisy module function here
}
# warnings coming from here will not be affected
referencing + dereferencing
I see some contructs like this in your code:
\%{ $CONFIG }
\%$CONFIG
You can just write $CONFIG instead. The % dereferences the hash ref, but then the \ takes the reference again - i.e. they cancel each other out.
Of course if you're doing it on purpose, to remind yourself that what is being passed along is a hash ref - then don't let me keep you from doing so... :)
creating temporary files
In populate_config_hash you manually generate a hopefully-unique temporary file name like this:
my $range = '1000000';
my $rand = int(rand($range)); ##generate random temp-db filename to al
+low for simultanous script execution
...
tempdb => "$path/$rand.db",
There is a module for safely creating temporary files: File::Temp (which is part of Perl since v5.6.1)
unpacking a subroutine argument
At the beginning of subroutines that only take one argument, you tend to write:
my $foo = $_[0];
There's nothing wrong with this technically, but I thought I'd point out that the following idioms are much more commonly used for that:
my $foo = shift;
my ($foo) = @_;
Using shift for this may seem particularly strange to those new to Perl, but it has a lot of tradition and is also very fast.
Edit: Fixed typo ("shift" not "unshift")
-
Are you posting in the right place? Check out Where do I post X? to know for sure.
-
Posts may use any of the Perl Monks Approved HTML tags. Currently these include the following:
<code> <a> <b> <big>
<blockquote> <br /> <dd>
<dl> <dt> <em> <font>
<h1> <h2> <h3> <h4>
<h5> <h6> <hr /> <i>
<li> <nbsp> <ol> <p>
<small> <strike> <strong>
<sub> <sup> <table>
<td> <th> <tr> <tt>
<u> <ul>
-
Snippets of code should be wrapped in
<code> tags not
<pre> tags. In fact, <pre>
tags should generally be avoided. If they must
be used, extreme care should be
taken to ensure that their contents do not
have long lines (<70 chars), in order to prevent
horizontal scrolling (and possible janitor
intervention).
-
Want more info? How to link
or How to display code and escape characters
are good places to start.