[PATCH 0/4] get_maintainer script cleanups

[PATCH 0/4] get_maintainer script cleanups

[Stephen Hemminger]

Some simple perl hacking changes.

[PATCH 1/4] get_maintainer: use three argument open

[Stephen Hemminger]

[-- Attachment #1: get_maintainer-perlcritic.patch --]
[-- Type: text/plain, Size: 528 bytes --]

Use 3 argument open to avoid any issues with parsing
and silence perlcritic.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

--- a/scripts/get_maintainer.pl	2014-05-26 12:47:35.039720690 -0700
+++ b/scripts/get_maintainer.pl	2014-05-26 13:27:52.000000000 -0700
@@ -426,7 +426,7 @@ foreach my $file (@ARGV) {
 	my $file_cnt = @files;
 	my $lastfile;
 
-	open(my $patch, "< $file")
+	open(my $patch, '<', $file)
 	    or die "$P: Can't open $file: $!\n";
 
 	# We can check arbitrary information before the patch

[PATCH 2/4] get_maintainer: remove quoting on hash label

[Stephen Hemminger]

[-- Attachment #1: get_maintainer-label.patch --]
[-- Type: text/plain, Size: 4874 bytes --]

Don't need to quote hash labels, especially when initializing.
Code is cleaner if unquoted.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

--- a/scripts/get_maintainer.pl	2014-05-26 13:09:47.000000000 -0700
+++ b/scripts/get_maintainer.pl	2014-05-26 13:27:49.000000000 -0700
@@ -94,69 +94,63 @@ my $rfc822_char = '[\\000-\\377]';
 my %VCS_cmds;
 
 my %VCS_cmds_git = (
-    "execute_cmd" => \&git_execute_cmd,
-    "available" => '(which("git") ne "") && (-e ".git")',
-    "find_signers_cmd" =>
-	"git log --no-color --follow --since=\$email_git_since " .
-	    '--numstat --no-merges ' .
-	    '--format="GitCommit: %H%n' .
-		      'GitAuthor: %an <%ae>%n' .
-		      'GitDate: %aD%n' .
-		      'GitSubject: %s%n' .
-		      '%b%n"' .
-	    " -- \$file",
-    "find_commit_signers_cmd" =>
-	"git log --no-color " .
-	    '--numstat ' .
-	    '--format="GitCommit: %H%n' .
-		      'GitAuthor: %an <%ae>%n' .
-		      'GitDate: %aD%n' .
-		      'GitSubject: %s%n' .
-		      '%b%n"' .
-	    " -1 \$commit",
-    "find_commit_author_cmd" =>
-	"git log --no-color " .
-	    '--numstat ' .
-	    '--format="GitCommit: %H%n' .
-		      'GitAuthor: %an <%ae>%n' .
-		      'GitDate: %aD%n' .
-		      'GitSubject: %s%n"' .
-	    " -1 \$commit",
-    "blame_range_cmd" => "git blame -l -L \$diff_start,+\$diff_length \$file",
-    "blame_file_cmd" => "git blame -l \$file",
-    "commit_pattern" => "^GitCommit: ([0-9a-f]{40,40})",
-    "blame_commit_pattern" => "^([0-9a-f]+) ",
-    "author_pattern" => "^GitAuthor: (.*)",
-    "subject_pattern" => "^GitSubject: (.*)",
-    "stat_pattern" => "^(\\d+)\\t(\\d+)\\t\$file\$",
+    execute_cmd => \&git_execute_cmd,
+    available   => '(which("git") ne "") && (-e ".git")',
+
+    find_signers_cmd =>
+      "git log --no-color --follow --since=\$email_git_since "
+      . '--numstat --no-merges '
+      . '--format="GitCommit: %H%n'
+      . 'GitAuthor: %an <%ae>%n'
+      . 'GitDate: %aD%n'
+      . 'GitSubject: %s%n' . '%b%n"'
+      . " -- \$file",
+    find_commit_signers_cmd => "git log --no-color "
+      . '--numstat '
+      . '--format="GitCommit: %H%n'
+      . 'GitAuthor: %an <%ae>%n'
+      . 'GitDate: %aD%n'
+      . 'GitSubject: %s%n' . '%b%n"'
+      . " -1 \$commit",
+    find_commit_author_cmd => "git log --no-color "
+      . '--numstat '
+      . '--format="GitCommit: %H%n'
+      . 'GitAuthor: %an <%ae>%n'
+      . 'GitDate: %aD%n'
+      . 'GitSubject: %s%n"'
+      . " -1 \$commit",
+    blame_range_cmd => "git blame -l -L \$diff_start,+\$diff_length \$file",
+    blame_file_cmd  => "git blame -l \$file",
+    commit_pattern  => "^GitCommit: ([0-9a-f]{40,40})",
+    blame_commit_pattern => "^([0-9a-f]+) ",
+    author_pattern       => "^GitAuthor: (.*)",
+    subject_pattern      => "^GitSubject: (.*)",
+    stat_pattern         => "^(\\d+)\\t(\\d+)\\t\$file\$",
 );
 
 my %VCS_cmds_hg = (
-    "execute_cmd" => \&hg_execute_cmd,
-    "available" => '(which("hg") ne "") && (-d ".hg")',
-    "find_signers_cmd" =>
-	"hg log --date=\$email_hg_since " .
-	    "--template='HgCommit: {node}\\n" .
-	                "HgAuthor: {author}\\n" .
-			"HgSubject: {desc}\\n'" .
-	    " -- \$file",
-    "find_commit_signers_cmd" =>
-	"hg log " .
-	    "--template='HgSubject: {desc}\\n'" .
-	    " -r \$commit",
-    "find_commit_author_cmd" =>
-	"hg log " .
-	    "--template='HgCommit: {node}\\n" .
-		        "HgAuthor: {author}\\n" .
-			"HgSubject: {desc|firstline}\\n'" .
-	    " -r \$commit",
-    "blame_range_cmd" => "",		# not supported
-    "blame_file_cmd" => "hg blame -n \$file",
-    "commit_pattern" => "^HgCommit: ([0-9a-f]{40,40})",
-    "blame_commit_pattern" => "^([ 0-9a-f]+):",
-    "author_pattern" => "^HgAuthor: (.*)",
-    "subject_pattern" => "^HgSubject: (.*)",
-    "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
+    execute_cmd => \&hg_execute_cmd,
+    available   => '(which("hg") ne "" ) && ( -d ".hg" )',
+    find_signers_cmd => "hg log --date=\$email_hg_since "
+      . "--template='HgCommit: {node}\\n"
+      . "HgAuthor: {author}\\n"
+      . "HgSubject: {desc}\\n'"
+      . " -- \$file",
+    find_commit_signers_cmd => "hg log "
+      . "--template='HgSubject: {desc}\\n'"
+      . " -r \$commit",
+    find_commit_author_cmd => "hg log "
+      . "--template='HgCommit: {node}\\n"
+      . "HgAuthor: {author}\\n"
+      . "HgSubject: {desc|firstline}\\n'"
+      . " -r \$commit",
+    blame_range_cmd      => "",                               # not supported
+    blame_file_cmd       => "hg blame -n \$file",
+    commit_pattern       => "^HgCommit: ([0-9a-f]{40,40})",
+    blame_commit_pattern => "^([ 0-9a-f]+):",
+    author_pattern       => "^HgAuthor: (.*)",
+    subject_pattern      => "^HgSubject: (.*)",
+    stat_pattern         => "^(\\d+)\t(\\d+)\t\$file\$",
 );
 
 my $conf = which_conf(".get_maintainer.conf");

[PATCH 3/4] get_maintainer: use array for defining available version control

[Stephen Hemminger]

[-- Attachment #1: get-maintainer-array.patch --]
[-- Type: text/plain, Size: 8669 bytes --]

Changes which cleanup how multiple version control systems are
handled:
 * Rather than hardcoding the test for version control, use an array
   of available version control systems and label each one.
 * Instead of copying the hash of current version control, use a reference to
   get the current version control.
 * Remove unnecessary quoting on the label of the hash

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>


--- a/scripts/get_maintainer.pl	2014-05-26 13:14:14.264500168 -0700
+++ b/scripts/get_maintainer.pl	2014-05-26 13:27:47.000000000 -0700
@@ -53,7 +53,7 @@ my $pattern_depth = 0;
 my $version = 0;
 my $help = 0;
 
-my $vcs_used = 0;
+my $vcs_used;	# undef
 
 my $exit = 0;
 
@@ -90,10 +90,8 @@ my $rfc822_lwsp = "(?:(?:\\r\\n)?[ \\t])
 my $rfc822_char = '[\\000-\\377]';
 
 # VCS command support: class-like functions and strings
-
-my %VCS_cmds;
-
 my %VCS_cmds_git = (
+    name	=> 'git',
     execute_cmd => \&git_execute_cmd,
     available   => '(which("git") ne "") && (-e ".git")',
 
@@ -129,6 +127,7 @@ my %VCS_cmds_git = (
 );
 
 my %VCS_cmds_hg = (
+    name	=> 'hg',
     execute_cmd => \&hg_execute_cmd,
     available   => '(which("hg") ne "" ) && ( -d ".hg" )',
     find_signers_cmd => "hg log --date=\$email_hg_since "
@@ -152,6 +151,7 @@ my %VCS_cmds_hg = (
     subject_pattern      => "^HgSubject: (.*)",
     stat_pattern         => "^(\\d+)\t(\\d+)\t\$file\$",
 );
+my @VCS_avail = ( \%VCS_cmds_git, \%VCS_cmds_hg );
 
 my $conf = which_conf(".get_maintainer.conf");
 if (-f $conf) {
@@ -1275,11 +1275,11 @@ sub vcs_find_signers {
     my @authors = ();
     my @stats = ();
 
-    @lines = &{$VCS_cmds{"execute_cmd"}}($cmd);
+    @lines = &{$vcs_used->{execute_cmd}}($cmd);
 
-    my $pattern = $VCS_cmds{"commit_pattern"};
-    my $author_pattern = $VCS_cmds{"author_pattern"};
-    my $stat_pattern = $VCS_cmds{"stat_pattern"};
+    my $pattern = $vcs_used->{commit_pattern};
+    my $author_pattern = $vcs_used->{author_pattern};
+    my $stat_pattern = $vcs_used->{stat_pattern};
 
     $stat_pattern =~ s/(\$\w+)/$1/eeg;		#interpolate $stat_pattern
 
@@ -1310,7 +1310,7 @@ sub vcs_find_author {
     my ($cmd) = @_;
     my @lines = ();
 
-    @lines = &{$VCS_cmds{"execute_cmd"}}($cmd);
+    @lines = &{$vcs_used->{execute_cmd}}($cmd);
 
     if (!$email_git_penguin_chiefs) {
 	@lines = grep(!/${penguin_chiefs}/i, @lines);
@@ -1320,7 +1320,7 @@ sub vcs_find_author {
 
     my @authors = ();
     foreach my $line (@lines) {
-	if ($line =~ m/$VCS_cmds{"author_pattern"}/) {
+	if ($line =~ m/$vcs_used->{author_pattern}/) {
 	    my $author = $1;
 	    my ($name, $address) = parse_email($author);
 	    $author = format_email($name, $address, 1);
@@ -1339,10 +1339,10 @@ sub vcs_save_commits {
     my @lines = ();
     my @commits = ();
 
-    @lines = &{$VCS_cmds{"execute_cmd"}}($cmd);
+    @lines = &{$vcs_used->{execute_cmd}}($cmd);
 
     foreach my $line (@lines) {
-	if ($line =~ m/$VCS_cmds{"blame_commit_pattern"}/) {
+	if ($line =~ m/$vcs_used->{blame_commit_pattern}/) {
 	    push(@commits, $1);
 	}
     }
@@ -1357,10 +1357,10 @@ sub vcs_blame {
 
     return @commits if (!(-f $file));
 
-    if (@range && $VCS_cmds{"blame_range_cmd"} eq "") {
+    if (@range && $vcs_used->{blame_range_cmd} eq "") {
 	my @all_commits = ();
 
-	$cmd = $VCS_cmds{"blame_file_cmd"};
+	$cmd = $vcs_used->{blame_file_cmd};
 	$cmd =~ s/(\$\w+)/$1/eeg;		#interpolate $cmd
 	@all_commits = vcs_save_commits($cmd);
 
@@ -1381,12 +1381,12 @@ sub vcs_blame {
 	    my $diff_start = $2;
 	    my $diff_length = $3;
 	    next if ("$file" ne "$diff_file");
-	    $cmd = $VCS_cmds{"blame_range_cmd"};
+	    $cmd = $vcs_used->{blame_range_cmd};
 	    $cmd =~ s/(\$\w+)/$1/eeg;		#interpolate $cmd
 	    push(@commits, vcs_save_commits($cmd));
 	}
     } else {
-	$cmd = $VCS_cmds{"blame_file_cmd"};
+	$cmd = $vcs_used->{blame_file_cmd};
 	$cmd =~ s/(\$\w+)/$1/eeg;		#interpolate $cmd
 	@commits = vcs_save_commits($cmd);
     }
@@ -1400,11 +1400,10 @@ sub vcs_blame {
 
 my $printed_novcs = 0;
 sub vcs_exists {
-    %VCS_cmds = %VCS_cmds_git;
-    return 1 if eval $VCS_cmds{"available"};
-    %VCS_cmds = %VCS_cmds_hg;
-    return 2 if eval $VCS_cmds{"available"};
-    %VCS_cmds = ();
+    foreach my $vc (@VCS_avail) {
+	return $vc if eval $vc->{available};
+    }
+
     if (!$printed_novcs) {
 	warn("$P: No supported VCS found.  Add --nogit to options?\n");
 	warn("Using a git repository produces better results.\n");
@@ -1412,16 +1411,17 @@ sub vcs_exists {
 	warn("git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git\n");
 	$printed_novcs = 1;
     }
-    return 0;
+
+    return; #undef
 }
 
 sub vcs_is_git {
-    vcs_exists();
-    return $vcs_used == 1;
+    $vcs_used = vcs_exists();
+    return defined($vcs_used) && ($vcs_used->{name} eq 'git');
 }
 
 sub vcs_is_hg {
-    return $vcs_used == 2;
+    return defined($vcs_used) && ($vcs_used->{name} eq 'hg');
 }
 
 sub interactive_get_maintainers {
@@ -1752,13 +1752,13 @@ sub save_commits_by_author {
     my @subjects = ();
 
     foreach my $line (@lines) {
-	if ($line =~ m/$VCS_cmds{"author_pattern"}/) {
+	if ($line =~ m/$vcs_used->{author_pattern}/) {
 	    my $author = $1;
 	    $author = deduplicate_email($author);
 	    push(@authors, $author);
 	}
-	push(@commits, $1) if ($line =~ m/$VCS_cmds{"commit_pattern"}/);
-	push(@subjects, $1) if ($line =~ m/$VCS_cmds{"subject_pattern"}/);
+	push(@commits, $1) if ($line =~ m/$vcs_used->{commit_pattern}/);
+	push(@subjects, $1) if ($line =~ m/$vcs_used->{subject_pattern}/);
     }
 
     for (my $i = 0; $i < @authors; $i++) {
@@ -1784,8 +1784,8 @@ sub save_commits_by_signer {
     my $subject = "";
 
     foreach my $line (@lines) {
-	$commit = $1 if ($line =~ m/$VCS_cmds{"commit_pattern"}/);
-	$subject = $1 if ($line =~ m/$VCS_cmds{"subject_pattern"}/);
+	$commit = $1 if ($line =~ m/$vcs_used->{commit_pattern}/);
+	$subject = $1 if ($line =~ m/$vcs_used->{subject_pattern}/);
 	if ($line =~ /^[ \t]*${signature_pattern}.*\@.*$/) {
 	    my @signatures = ($line);
 	    my ($types_ref, $signers_ref) = extract_formatted_signatures(@signatures);
@@ -1870,7 +1870,7 @@ sub vcs_file_signoffs {
     $vcs_used = vcs_exists();
     return if (!$vcs_used);
 
-    my $cmd = $VCS_cmds{"find_signers_cmd"};
+    my $cmd = $vcs_used->{find_signers_cmd};
     $cmd =~ s/(\$\w+)/$1/eeg;		# interpolate $cmd
 
     ($commits, $signers_ref, $authors_ref, $stats_ref) = vcs_find_signers($cmd, $file);
@@ -1888,7 +1888,7 @@ sub vcs_file_signoffs {
     vcs_assign("commit_signer", $commits, @signers);
     vcs_assign("authored", $commits, @authors);
     if ($#authors == $#stats) {
-	my $stat_pattern = $VCS_cmds{"stat_pattern"};
+	my $stat_pattern = $vcs_used->{stat_pattern};
 	$stat_pattern =~ s/(\$\w+)/$1/eeg;	#interpolate $stat_pattern
 
 	my $added = 0;
@@ -1956,7 +1956,7 @@ sub vcs_file_blame {
 	    my $commit = join(" -r ", @commits);
 	    my $cmd;
 
-	    $cmd = $VCS_cmds{"find_commit_signers_cmd"};
+	    $cmd = $vcs_used->{find_commit_signers_cmd};
 	    $cmd =~ s/(\$\w+)/$1/eeg;	#substitute variables in $cmd
 
 	    ($commit_count, $commit_signers_ref, $commit_authors_ref, $stats_ref) = vcs_find_signers($cmd, $file);
@@ -1974,7 +1974,7 @@ sub vcs_file_blame {
 		my @commit_signers = ();
 		my $cmd;
 
-		$cmd = $VCS_cmds{"find_commit_signers_cmd"};
+		$cmd = $vcs_used->{find_commit_signers_cmd};
 		$cmd =~ s/(\$\w+)/$1/eeg;	#substitute variables in $cmd
 
 		($commit_count, $commit_signers_ref, $commit_authors_ref, $stats_ref) = vcs_find_signers($cmd, $file);
@@ -1997,12 +1997,12 @@ sub vcs_file_blame {
 		my $commit = join(" -r ", @commits);
 		my $cmd;
 
-		$cmd = $VCS_cmds{"find_commit_author_cmd"};
+		$cmd = $vcs_used->{find_commit_author_cmd};
 		$cmd =~ s/(\$\w+)/$1/eeg;	#substitute variables in $cmd
 
 		my @lines = ();
 
-		@lines = &{$VCS_cmds{"execute_cmd"}}($cmd);
+		@lines = &{$vcs_used->{execute_cmd}}($cmd);
 
 		if (!$email_git_penguin_chiefs) {
 		    @lines = grep(!/${penguin_chiefs}/i, @lines);
@@ -2012,7 +2012,7 @@ sub vcs_file_blame {
 
 		my @authors = ();
 		foreach my $line (@lines) {
-		    if ($line =~ m/$VCS_cmds{"author_pattern"}/) {
+		    if ($line =~ m/$vcs_used->{author_pattern}/) {
 			my $author = $1;
 			$author = deduplicate_email($author);
 			push(@authors, $author);
@@ -2027,7 +2027,7 @@ sub vcs_file_blame {
 	    else {
 		foreach my $commit (@commits) {
 		    my $i;
-		    my $cmd = $VCS_cmds{"find_commit_author_cmd"};
+		    my $cmd = $vcs_used->{find_commit_author_cmd};
 		    $cmd =~ s/(\$\w+)/$1/eeg;	#interpolate $cmd
 		    my @author = vcs_find_author($cmd);
 		    next if !@author;

[PATCH 4/4] get_maintainer: use anonymous function instead of eval

[Stephen Hemminger]

[-- Attachment #1: get_maintainer_lambda.patch --]
[-- Type: text/plain, Size: 1354 bytes --]

Change get_maintainer to use anonymous function instead of eval().
Using a anonymous function is safer than doing eval on a string.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>


--- a/scripts/get_maintainer.pl	2014-05-26 13:23:00.480939284 -0700
+++ b/scripts/get_maintainer.pl	2014-05-26 13:23:45.925291872 -0700
@@ -93,8 +93,9 @@ my $rfc822_char = '[\\000-\\377]';
 my %VCS_cmds_git = (
     name	=> 'git',
     execute_cmd => \&git_execute_cmd,
-    available   => '(which("git") ne "") && (-e ".git")',
-
+    available   => sub {
+	return (which("git") ne "") && (-e ".git");
+    },
     find_signers_cmd =>
       "git log --no-color --follow --since=\$email_git_since "
       . '--numstat --no-merges '
@@ -129,7 +130,9 @@ my %VCS_cmds_git = (
 my %VCS_cmds_hg = (
     name	=> 'hg',
     execute_cmd => \&hg_execute_cmd,
-    available   => '(which("hg") ne "" ) && ( -d ".hg" )',
+    available   => sub {
+	return (which("hg") ne "" ) && ( -d ".hg" );
+    },
     find_signers_cmd => "hg log --date=\$email_hg_since "
       . "--template='HgCommit: {node}\\n"
       . "HgAuthor: {author}\\n"
@@ -1401,7 +1404,7 @@ sub vcs_blame {
 my $printed_novcs = 0;
 sub vcs_exists {
     foreach my $vc (@VCS_avail) {
-	return $vc if eval $vc->{available};
+	return $vc if $vc->{available}();
     }
 
     if (!$printed_novcs) {

Re: [PATCH 0/4] get_maintainer script cleanups

[Joe Perches]

On Mon, 2014-05-26 at 13:33 -0700, Stephen Hemminger wrote:
> Some simple perl hacking changes.

I don't really care for these.

Why do you think these are better?

I think the first one breaks the code too.

Re: [PATCH 0/4] get_maintainer script cleanups

[Joe Perches]

On Mon, 2014-05-26 at 18:00 -0700, Joe Perches wrote:
> On Mon, 2014-05-26 at 13:33 -0700, Stephen Hemminger wrote:
> > Some simple perl hacking changes.
> I think the first one breaks the code too.

https://lkml.org/lkml/2010/3/10/50

Re: [PATCH 0/4] get_maintainer script cleanups

[Stephen Hemminger]

On Mon, 26 May 2014 18:00:49 -0700
Joe Perches <joe@perches.com> wrote:

> On Mon, 2014-05-26 at 13:33 -0700, Stephen Hemminger wrote:
> > Some simple perl hacking changes.
> 
> I don't really care for these.
> 
> Why do you think these are better?
> 
> I think the first one breaks the code too.
> 
> 

$ perlcritic scripts/get_maintainer.pl 
Use IO::Interactive::is_interactive() instead of -t at line 237, column 5.  See page 218 of PBP.  (Severity: 5)
Two-argument "open" used at line 429, column 2.  See page 207 of PBP.  (Severity: 5)
Expression form of "eval" at line 1410, column 17.  See page 161 of PBP.  (Severity: 5)
Expression form of "eval" at line 1412, column 17.  See page 161 of PBP.  (Severity: 5)

If you read Perl Best Practices you will see why these are issues.
The first is because the test for STDIN and ARGV is not sufficient to check for
interactive usage.

The next is because using quoted two arg open breaks if there is a space in name etc.

Lastly eval of a string leads to runtime errors versus compile time.

Yes, for a trivial program like this it really doesn't matter.
But this program is likely to grow and be copied by other developers
and I would like it to use current best practices. The same applies to
all the other parts of the kernel.

I tested each step, and they all work for me™ 

Re: [PATCH 0/4] get_maintainer script cleanups

[Stephen Hemminger]

On Mon, 26 May 2014 18:03:55 -0700
Joe Perches <joe@perches.com> wrote:

> On Mon, 2014-05-26 at 18:00 -0700, Joe Perches wrote:
> > On Mon, 2014-05-26 at 13:33 -0700, Stephen Hemminger wrote:
> > > Some simple perl hacking changes.
> > I think the first one breaks the code too.
> 
> https://lkml.org/lkml/2010/3/10/50
> 
> 

Ah forgot that. Never used that form in my usage.
Never mind.

[PATCH 5/4] get_maintainer: shut up perl critic

[Stephen Hemminger]

Use the no critic annotation, with comment, to silence perl critic
for places where the code is correct as is.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

---

Supersedes patch 1

--- a/scripts/get_maintainer.pl	2014-05-26 18:09:38.257783683 -0700
+++ b/scripts/get_maintainer.pl	2014-05-26 18:19:25.782478516 -0700
@@ -231,7 +231,10 @@ if ($version != 0) {
     exit 0;
 }
 
+# For this usage requiring IO::Interactive is overkill
+## no critic (ProhibitInteractiveTest)
 if (-t STDIN && !@ARGV) {
+    ## use critic
     # We're talking to a terminal, but have no command line arguments.
     die "$P: missing patchfile or -f file - use --help if necessary\n";
 }
@@ -423,8 +426,13 @@ foreach my $file (@ARGV) {
 	my $file_cnt = @files;
 	my $lastfile;
 
+	# Use two argument form of open because we want
+	# to allow using "-" to indicate standard input
+
+	## no critic (ProhibitTwoArgOpen)
 	open(my $patch, "< $file")
 	    or die "$P: Can't open $file: $!\n";
+	## use critic
 
 	# We can check arbitrary information before the patch
 	# like the commit message, mail headers, etc...

Re: [PATCH 5/4] get_maintainer: shut up perl critic

[Joe Perches]

On Mon, 2014-05-26 at 18:22 -0700, Stephen Hemminger wrote:
> Use the no critic annotation, with comment, to silence perl critic
> for places where the code is correct as is.

I think this uglifies the code and a more sensible solution
is not using perlcritic

Re: [PATCH 5/4] get_maintainer: shut up perl critic

[Stephen Hemminger]

On Mon, 26 May 2014 18:32:36 -0700
Joe Perches <joe@perches.com> wrote:

> On Mon, 2014-05-26 at 18:22 -0700, Stephen Hemminger wrote:
> > Use the no critic annotation, with comment, to silence perl critic
> > for places where the code is correct as is.
> 
> I think this uglifies the code and a more sensible solution
> is not using perlcritic
> 
> 

Perlcritic like checkpatch or sparse is a useful tool but often generates
false positives. Doing this allows shutting up the tool.