[PATCH 4/5] gitweb: Localize magic variable $/

From: Jakub Narebski <jnareb@gmail.com>
Date: May 10, 2009, 00:39 UTC
Message-ID: <200905100239.40092.jnareb@gmail.com>
In-Reply-To: <200905100203.51744.jnareb@gmail.com>

Instead of undefining and then restoring magic variable $/ (input record separator) for 'slurp mode', localize it.

While at it, state explicitly that "local $/;" makes it undefined, by using explicit "local $/ = undef;".

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Perl::Critic::Policy::Variables::RequireLocalizedPunctuationVars

  Magic variables should be assigned as "local".

  Punctuation variables (and their English.pm equivalents) are global
  variables. Messing with globals is dangerous in a complex program as it
  can lead to very subtle and hard to fix bugs. If you must change a magic
  variable in a non-trivial program, do it in a local scope.

  For example, to slurp a filehandle into a scalar, it's common to set the
  record separator to undef instead of a newline. If you choose to do this
  (instead of using File::Slurp!) then be sure to localize the global and
  change it for as short a time as possible.

See also Damian Conway's book "Perl Best Practices", 5.6. Localizing Punctuation Variables (If you're forced to modify a punctuation variable, localize it.)

Perl::Critic::Policy::Variables::RequireInitializationForLocalVars

  Write  'local $foo = $bar;'  instead of just  'local $foo;'.

  Most people don't realize that a localized copy of a variable does
  not retain its original value. Unless you initialize the variable
  when you local-ize it, it defaults to undef. If you want the
  variable to retain its original value, just initialize it to
  itself. If you really do want the localized copy to be undef, then
  make it explicit.

 gitweb/gitweb.perl |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 852beb6..1cb3a4f 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3326,7 +3326,7 @@ sub git_get_link_target {
 	open my $fd, "-|", git_cmd(), "cat-file", "blob", $hash
 		or return;
 	{
-		local $/;
+		local $/ = undef;
 		$link_target = <$fd>;
 	}
 	close $fd
@@ -4801,11 +4801,10 @@ sub git_blob_plain {
 		-content_disposition =>
 			($sandbox ? 'attachment' : 'inline')
 			. '; filename="' . $save_as . '"');
-	undef $/;
+	local $/ = undef;
 	binmode STDOUT, ':raw';
 	print <$fd>;
 	binmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi
-	$/ = "\n";
 	close $fd;
 }
 
@@ -4907,12 +4906,15 @@ sub git_tree {
 		}
 	}
 	die_error(404, "No such tree") unless defined($hash);
-	$/ = "\0";
-	open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash
-		or die_error(500, "Open git-ls-tree failed");
-	my @entries = map { chomp; $_ } <$fd>;
-	close $fd or die_error(404, "Reading tree failed");
-	$/ = "\n";
+
+	{
+		local $/ = "\0";
+		open my $fd, "-|", git_cmd(), "ls-tree", '-z', $hash
+			or die_error(500, "Open git-ls-tree failed");
+		my @entries = map { chomp; $_ } <$fd>;
+		close $fd
+			or die_error(404, "Reading tree failed");
+	}
 
 	my $refs = git_get_references();
 	my $ref = format_ref_marker($refs, $hash_base);
@@ -5807,7 +5809,7 @@ sub git_search {
 
 		print "<table class=\"pickaxe search\">\n";
 		my $alternate = 1;
-		$/ = "\n";
+		local $/ = "\n";
 		open my $fd, '-|', git_cmd(), '--no-pager', 'log', @diff_opts,
 			'--pretty=format:%H', '--no-abbrev', '--raw', "-S$searchtext",
 			($search_use_regexp ? '--pickaxe-regex' : ());
@@ -5877,7 +5879,7 @@ sub git_search {
 		print "<table class=\"grep_search\">\n";
 		my $alternate = 1;
 		my $matches = 0;
-		$/ = "\n";
+		local $/ = "\n";
 		open my $fd, "-|", git_cmd(), 'grep', '-n',
 			$search_use_regexp ? ('-E', '-i') : '-F',
 			$searchtext, $co{'tree'};

-- 
1.6.3