Yes, you are right. The declaration of $i inside the for expression shadows
the scope of the outer $i inside the for loop (which indeed makes the outer
$i uninitialized after the loop).

The for loop itself is not stylistically Perlish with the C-style loop, but
that's a minor gripe. I made an error when using the Perl-style range
operator ( N1 .. N2 ) loop and forgot to remove the commented out line too.
And I mistakenly left the extraneous 'my' there, which declares $i in a new
scope. After some unintentional off-list conversation with Hubert - which
was lucky since he discovered another embarrasing mistake - my suggestion
would be to change it to a while loop instead (see attached patch).

Best regards,

/Pär

2015-05-19 20:04 GMT+02:00 Hubert Tarasiuk <[email protected]>:

> > From: Pär Karlsson <address@hidden>
> >
> > A number of Perl-specific cleanups in the test suite perl modules.
> >
> > Most of these changes have no immediate functional difference but puts
> code
> > in line with the same formatting (perltidy GNU style) for readability.
> >
> > A warning regarding invalid operators on incompatible values (numeric vs
> > strings) have been fixed.
> >
> > Some common but discouraged Perl idioms have been updated to a somewhat
> > clearer style, especially regarding evals and map() vs. for loops.
> >
> > I did not touch the FTP and HTTP server modules functionally, but there
> seem
> > to be a lot of strange code there in, and would warrant further cleanups
> > and investigations if these tests would remain in Perl, instead of moving
> > over to the Python based tests.
> >
> > All tests work nominally on my machine (with perl-5.20.1) but should be
> > compatible with versions down to 5.6.
>
> I believe I have found an issue with WgetTests module:
> >     my $i;
> >
> >     # for ($i=0; $i != $min; ++$i) {
> >     for my $i (0 .. $min - 1)
> >     {
> >         last if substr($expected, $i, 1) ne substr $actual, $i, 1;
> >         if (substr($expected, $i, 1) eq q{\n})
> >         {
> >             $line++;
> >             $col = 0;
> >         }
> >         else
> >         {
> >             $col++;
> >         }
> >     }
> >     my $snip_start = $i - ($SNIPPET_SIZE / 2);
> >     if ($snip_start < 0)
> >     {
> >         $SNIPPET_SIZE += $snip_start;    # Take it from the end.
> >         $snip_start = 0;
> >     }
> I am no Perl expert but I tested it and current for loop will not set
> the $i variable to any value (outside the loop). (Actually it gives me
> warning about using uninitialized variable when calculating $snip_start
> and the diff is not printed correctly.)
> Reverting the commented version seems to fix this problem. Removing the
> "my" from current for loop does not seem to fix the problem.
> --
> Hubert Tarasiuk
>
>
From 30ff711d6568a9269390ea7a8847357ba021fad9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A4r=20Karlsson?= <[email protected]>
Date: Tue, 19 May 2015 22:20:17 +0200
Subject: [PATCH] Fix undeclared loop variable in Perl test suite

---
 tests/WgetTests.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/WgetTests.pm b/tests/WgetTests.pm
index 501aeba..34253b1 100644
--- a/tests/WgetTests.pm
+++ b/tests/WgetTests.pm
@@ -265,13 +265,12 @@ sub _show_diff
     my $min  = $explen <= $actlen ? $explen : $actlen;
     my $line = 1;
     my $col  = 1;
-    my $i;
+    my $i    = 0;
 
-    # for ($i=0; $i != $min; ++$i) {
-    for my $i (0 .. $min - 1)
+    while ( $i < $min )
     {
         last if substr($expected, $i, 1) ne substr $actual, $i, 1;
-        if (substr($expected, $i, 1) eq q{\n})
+        if (substr($expected, $i, 1) eq "\n")
         {
             $line++;
             $col = 0;
@@ -280,6 +279,7 @@ sub _show_diff
         {
             $col++;
         }
+        $i++;
     }
     my $snip_start = $i - ($SNIPPET_SIZE / 2);
     if ($snip_start < 0)
-- 
2.3.6

Reply via email to