git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/4] gitweb: Simplify snapshot format detection logic in evaluate_path_info

From
Jakub Narebski <jnareb@gmail.com>
Date
May 11, 2009, 17:42 UTC
Message-ID
<20090511173950.15152.61267.stgit@localhost.localdomain>
In-Reply-To
<20090511173025.15152.94215.stgit@localhost.localdomain>

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@gmail.com>
---
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;
Previous: Jakub NarebskiNext: Giuseppe Bilotta
Message 4 of 6 in “gitweb: Some code cleanups, part 2 (low hanging fruit)”
  1. 0/4 gitweb: Some code cleanups, part 2 (low hanging fruit)Jakub Narebski, May 11, 2009
  2. 1/4 gitweb: Replace wrongly added tabs with spacesJakub Narebski, May 11, 2009
  3. 2/4 gitweb: Use capturing parentheses only when you intend to captureJakub Narebski, May 11, 2009
  4. 3/4 gitweb: Simplify snapshot format detection logic in evaluate_path_infoJakub Narebski, May 11, 2009
  5. Giuseppe BilottaMay 11, 2009
  6. 4/4 gitweb: Remove unused $hash_base parameter from normalize_link_targetJakub Narebski, May 11, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.