Re: [Bug-wget] [PATCH] Stylistic and idiomatic cleanups in Perl tests

[Pär Karlsson]

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 <address@hidden>:

> > 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
>
>

0001-Fix-undeclared-loop-variable-in-Perl-test-suite.patch
Description: Text Data