On Mar 4, 4:46 am, que...@gmail.com (Jerald Sheets) wrote: > > I really think you're doing yourself a disservice by just throwing > your program commands on lines, not indenting according to best > practices. It makes your code unreadable, and can make it very hard > to debug the more involved your programs get. > [snip]
> Consider picking up Damian's book: > > http://oreilly.com/catalog/9780596001735/index.html > > It'll serve you well and is a must have for the serious perl programmer. > I agree, however your code as shown below doesn't follow "Perl Best Practices". You use improper indentation, and failed to use vertical whitespace making it harder to read/follow. > #!/usr/bin/perl -w > It's better to use the warnings pragma, instead of the -w switch > use Net::SSH2; > use strict; > > # Set up variables to use > my $hostlist = "/path/to/hostlist.txt"; > my @hosts = (); > > # open the hostlist filehandle, and read in values > open (hostlist_fh, "<", $hostlist) better written as: open my $hostlist_fh, '<', $hostlist > # fail gracefully if something happens > or die "Couldn't open $hostlist for reading: $!"; > # while reading the list > while (<hostlist_fh>) { Why the added indentation? The initialization of the while loop should be at the same level as open. > # clean up the values > chomp; > # stuff them into an array > push(@hosts, $_); > } Same indentation issue here, push should be at the same level as chomp. Why loop over that data twice, once when building the array and again in the below foreach loop? > > # for each element of the array > foreach my $host (@hosts) { > # clean it up again Why clean it up again? Was it not done properly the first time? > chomp($host); > # do the SSH stuff > my $ssh2.... > > { all your code here. host variable is $host } > > } > The code below is the direction of style that I'd use, most of which is borrowed from Rob's and Ruud's recommendations. #!/usr/bin/perl use strict; use warnings; use Net::SSH2; use Readonly; Readonly::Scalar my $buflen = 10_000; my $input = 'input.txt'; # I'm creating the ssh object here because # I don't see the need to recreate it at each iteration of the while loop my $ssh2 = Net::SSH2->new(); open my $hosts, '<', $input or die "Failed to open '$hosts' $!"; while ( my $host = <$hosts> ) { chomp $host; eval { $ssh2->connect($host) }; 1; # success } or do { my $err = $@ || "[unknown error]"; warn "Unable to connect host $host: $err" and next; }; $ssh2->auth_password('<username>','<password>'); my $chan = $ssh2->channel; $chan->exec('ls -al'); my $buf; my $read = $chan->read($buf, $buflen); die "More than $buflen characters in listing" if $read >= $buflen; print "BUF:$buf\n"; $chan->exec('exit'); $ssh2->disconnect; } -- To unsubscribe, e-mail: beginners-unsubscr...@perl.org For additional commands, e-mail: beginners-h...@perl.org http://learn.perl.org/
