git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Subject: [PATCH 4/5] gitweb: Localize magic variable $/
Date: Sun, 10 May 2009 02:39:39 +0200	[thread overview]
Message-ID: <200905100239.40092.jnareb@gmail.com> (raw)
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

  parent reply	other threads:[~2009-05-10  0:42 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-10  0:03 [PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern) Jakub Narebski
2009-05-10  0:05 ` [PATCHv2 1/5] gitweb: Remove function prototypes Jakub Narebski
2009-05-10  9:05   ` Jakub Narebski
2009-05-10  0:36 ` [PATCH 2/5] gitweb: Do not use bareword filehandles Jakub Narebski
2009-05-10  7:50   ` Petr Baudis
2009-05-10  9:27     ` Jakub Narebski
2009-05-11  1:21   ` [PATCH v2 " Jakub Narebski
2009-05-10  0:38 ` [PATCH 3/5] gitweb: Always use three argument form of open Jakub Narebski
2009-05-11  1:29   ` [PATCH v2 " Jakub Narebski
2009-05-10  0:39 ` Jakub Narebski [this message]
2009-05-10  0:40 ` [PATCH 5/5] gitweb: Use block form of map/grep in a few cases more Jakub Narebski
2009-05-11  0:47 ` [PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern) Junio C Hamano
2009-05-11  1:33   ` Jakub Narebski
2009-05-11  4:21     ` Junio C Hamano
2009-05-11  5:13       ` Daniel Pittman
2009-05-11  7:19       ` Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200905100239.40092.jnareb@gmail.com \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).