How to wrap a subroutine call in a for loop

Tuna has asked for the wisdom of the Perl Monks concerning the following question:

Given the following: (look familiar?)

sub establishSession {
   my  @sshPort;
  if ($sshPort =~ /(\d+)/)
    {@sshPort = ("port", $1)}
else {return &error ("invalid port number");}
 
  &xferOpen ('SESSION', $clusterHost,"compression", "yes", @sshPort)
 
||  return &error("couldn't connect to $clusterHost : $xferError");

  return $success;
}
[download]

I am trying to rewrite this routine so that it will make 3 atempts to succeed before "return &error()". Why doesn't this work?

sub establishSession {
   my  @sshPort;
  if ($sshPort =~ /(\d+)/)
    {@sshPort = ("port", $1)}
else {return &error ("invalid port number");}

for (my $tries = 1; $tries < 4; $tries++) { 

print "Attempt $tries to connect\n";
  
&xferOpen ('SESSION', $clusterHost,"compression", "yes", @sshPort);
} 

return &error("couldn't connect to $clusterHost : $xferError");

  return $success;
}
[download]

I can't seem to get the hang of iteration using a FOR loop. Here's the output:

[ssesar@burl-dhcp152-211 20010808]$ ./pushproc-devel-20010808.pl
Attempt 1 to connect
SAxfer.pl: spawning timeout 3 ssh '-oBatchMode yes' -x -n -p 22 -c 3de
+s '-oCompression yes' localhost /bin/true
Attempt 2 to connect
Attempt 3 to connect 
p
pushproc.pl: ERROR: couldn't connect to localhost : process timed out 
+after 3 seconds
 
 
pushproc.pl: disconnecting from host (localhost)
 
pushproc.pl: reactivation of localhost failed
 
 
pushproc.pl statistics:
    1 hosts attempted
    0 host(s) succeeded
    1 host(s) failed
[ssesar@burl-dhcp152-211 20010808]$
[download]

What am I missing?

Comment on How to wrap a subroutine call in a for loop Select or Download Code

Replies are listed 'Best First'.
Re: How to wrap a subroutine call in a for loop by Zed_Lopez (Chaplain) on Aug 09, 2001 at 02:03 UTC
`&xferOpen ('SESSION', $clusterHost,"compression", "yes", @sshPort) \|\| return &error("couldn't connect to $clusterHost : $xferError");` is a really crucial line. If the xferOpen returns true, the return &error is never run. If it returns false, it is. Your replacement breaks this functionality. You run xferOpen 3 times whether or not any of them was successful. Then you always return an error. Your code as written can never get to the return $success line. Some ways to do what you want: `my $connected; for (my $tries = 1; $tries < 4; $tries++) { print "Attempt $tries to connect\n"; $connected = &xferOpen ('SESSION', $clusterHost,"compression", "yes" +, @sshPort); last if $connected; } return $connected ? $success : &error("couldn't connect to $clusterHos +t : $xferError");` [download] or `my $connected; my $tries = 1; do { print "Attempt $tries to connect\n"; $connected = &xferOpen ('SESSION', $clusterHost,"compression", " +yes", @sshPort); $tries++; } until $connected or $tries > 3;` [download] or `eval (join '\|\|', ('&xferOpen ("SESSION", $clusterHost,"compression", " +yes", @sshPort)') x 3) \|\| return &error("couldn't connect to $cluster +Host : $xferError"); return $success;` [download] Yes, that last was intentionally silly. And, in general, it's good to avoid unlabelled magic numbers like 3. You might like to find a good place to say something like %config = (establish_session_attempts => 3).	[reply] [d/l] [select]

Replies are listed 'Best First'.

Re: How to wrap a subroutine call in a for loop
by Zed_Lopez (Chaplain) on Aug 09, 2001 at 02:03 UTC

&xferOpen ('SESSION', $clusterHost,"compression", "yes", @sshPort) || return &error("couldn't connect to $clusterHost : $xferError");

is a really crucial line. If the xferOpen returns true, the return &error is never run. If it returns false, it is.

Your replacement breaks this functionality. You run xferOpen 3 times whether or not any of them was successful. Then you always return an error. Your code as written can never get to the return $success line.

Some ways to do what you want:

my $connected;
for (my $tries = 1; $tries < 4; $tries++) { 
  print "Attempt $tries to connect\n";    
  $connected = &xferOpen ('SESSION', $clusterHost,"compression", "yes"
+, @sshPort);
  last if $connected;
} 
return $connected ? $success : &error("couldn't connect to $clusterHos
+t : $xferError");
[download]

my $connected;
my $tries = 1;
do {  print "Attempt $tries to connect\n";    
      $connected = &xferOpen ('SESSION', $clusterHost,"compression", "
+yes", @sshPort);  
      $tries++; 
   } until $connected or $tries > 3;
[download]

eval (join '||', ('&xferOpen ("SESSION", $clusterHost,"compression", "
+yes", @sshPort)') x 3) || return &error("couldn't connect to $cluster
+Host : $xferError");
return $success;
[download]

Yes, that last was intentionally silly. And, in general, it's good to avoid unlabelled magic numbers like 3. You might like to find a good place to say something like %config = (establish_session_attempts => 3).

[reply]
[d/l]
[select]