On Mon, May 11, 2009 at 7:42 PM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> This issue was caught by perlcritic in harsh severity level noticing
> that catch variable was used outside conditional thanks to the
> Perl::Critic::Policy::RegularExpressions::ProhibitCaptureWithoutTest
> policy. See "Perl Best Practices", chapter 12. Regular Expressions,
> section 12.15. Captured Values:
>
> Pattern matches that fail never assign anything to $1, $2, etc.,
> nor do they leave those variables undefined. After an unsuccessful
> pattern match, the numeric capture variables remain exactly as they
> were before the match was attempted.
>
> New version is in my opinion much easier to understand; previous
> version worked correctly due to the fact that we returned from loop
> on first found match.
>
> Signed-off-by: Jakub Narebski <jnareb@xxxxxxxxx>
> ---
> This is not the only place caught by this policy, but this is the one
> where fix was obviously needed to improve readibility of code.
>
> In _most_ (but not all) of other places we assume that output we parse
> is in given format, and that regexp would always match...
>
> gitweb/gitweb.perl | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 097bd18..c72ae10 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -690,9 +690,10 @@ sub evaluate_path_info {
> # format key itself, with a prepended dot
> while (my ($fmt, $opt) = each %known_snapshot_formats) {
> my $hash = $refname;
> - my $sfx;
> - $hash =~ s/(\Q$opt->{'suffix'}\E|\Q.$fmt\E)$//;
> - next unless $sfx = $1;
> + unless ($hash =~ s/(\Q$opt->{'suffix'}\E|\Q.$fmt\E)$//) {
> + next;
> + }
> + my $sfx = $1;
> # a valid suffix was found, so set the snapshot format
> # and reset the hash parameter
> $input_params{'snapshot_format'} = $fmt;
Totally mea culpa on this, I was optimizing it the wrong way.
Acked-by: Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx>
--
Giuseppe "Oblomov" Bilotta
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html