Merge pull request #1049 from cgzones/master_perlcritic

fix more perlcritic warnings

.perlcriticrc

@@ -16,11 +16,31 @@ profile-strictness = quiet
 
 [Variables::ProhibitPunctuationVars]
 severity = 5
-allow = $_ $!
+allow = $_ $! $0 $@ $$
 
 [CodeLayout::RequireTidyCode]
 perltidyrc = perltidyrc
 
 [CodeLayout::ProhibitTrailingWhitespace]
 severity = 5
 
+[CodeLayout::ProhibitQuotedWordLists]
+severity = 5
+
+[CodeLayout::RequireTrailingCommas]
+severity = 5
+
+[ControlStructures::ProhibitUnreachableCode]
+severity = 5
+
+[Documentation::RequirePackageMatchesPodName]
+severity = 5
+
+[Miscellanea::ProhibitUnrestrictedNoCritic]
+severity = 5
+
+[TestingAndDebugging::ProhibitNoWarnings]
+severity = 5
+
+[Variables::ProhibitUnusedVariables]
+severity = 5

HACKING.pod

@@ -145,7 +145,7 @@ Exceptions are caught with an eval:
  eval {
      # Exceptionally scary code
  };
- if ($EVAL_ERROR) {
+ if ($@) {
      # Handle exception
  }
 

Makefile

@@ -54,14 +54,11 @@ install: $(BUILD_SCRIPT)
 		-e "$$(perl -I lib -M"Munin::Common::Defaults" \
 			-e "Munin::Common::Defaults->print_as_sed_substitutions();")"
 
-#TODO: merge with lint target, when done
-.PHONY: perlcritic
-perlcritic:
-	#TODO: apply to scripts/ and lib/Munin/Master/
-	perlcritic lib/Munin/Node
-
 .PHONY: lint
 lint:
+	# Scanning munin code
+	perlcritic lib/ script/
+
 	@# SC1008: ignore our weird shebang (substituted later)
 	@# SC1090: ignore sourcing of files with variable in path
 	@# SC2009: do not complain about "ps ... | grep" calls (may be platform specific)

lib/Munin/Common/Config.pm

@@ -177,8 +177,8 @@ sub parse_config_from_file {
     eval {
         $self->parse_config($file);
     };
-    if ($EVAL_ERROR) {
-        croak "ERROR: Failed to parse config file '$config_file': $EVAL_ERROR";
+    if ($@) {
+        croak "ERROR: Failed to parse config file '$config_file': $@";
     }
 
     close $file

lib/Munin/Common/Defaults.pm.PL

@@ -54,9 +54,9 @@ our \$MUNIN_SPOOLDIR   = '$MUNIN_SPOOLDIR';
 # our \$MUNIN_MANDIR     = '';
 our \$MUNIN_LOGDIR     = '$MUNIN_LOGDIR';
 our \$MUNIN_STATEDIR   = '$MUNIN_STATEDIR';
-our \$MUNIN_USER       = \$^O eq 'MSWin32' ? '' : getpwuid \$EUID;
-our \$MUNIN_GROUP      = \$^O eq 'MSWin32' ? '' : getgrgid \$EGID;
-our \$MUNIN_PLUGINUSER = \$^O eq 'MSWin32' ? '' : getpwuid \$EUID;
+our \$MUNIN_USER       = \$OSNAME eq 'MSWin32' ? '' : getpwuid \$EUID;
+our \$MUNIN_GROUP      = \$OSNAME eq 'MSWin32' ? '' : getgrgid \$EGID;
+our \$MUNIN_PLUGINUSER = \$OSNAME eq 'MSWin32' ? '' : getpwuid \$EUID;
 our \$MUNIN_VERSION    = '$MUNIN_VERSION';
 our \$MUNIN_PERL       = '/usr/bin/perl';
 # our \$MUNIN_PERLLIB    = '';

lib/Munin/Common/SyncDictFile.pm

@@ -1,4 +1,8 @@
 package Munin::Common::SyncDictFile;
+
+use strict;
+use warnings;
+
 require Tie::Hash;
 our @ISA = qw(Tie::Hash);
 
@@ -28,7 +32,7 @@ sub STORE {
 	my ($self, $key, $value) = @_;
 	DEBUG("STORE($key, $value)");
 	$key = escape_key($key);
-	
+
 
 	use IO::File;
 	my $fh = _lock_write($self->{filename}, "r");
@@ -41,7 +45,7 @@ sub STORE {
 		# Print the read line, but ignore the key we are currently storing
 		print $fh_tmp "$line\n" unless $line =~ m/^$key:/;
 	}
-	
+
 	# Print the stored key at the end
 	DEBUG("Print the stored $key:$value");
 	print $fh_tmp "$key:$value\n";
@@ -73,7 +77,7 @@ sub FETCH {
 
 # Return the first key in the hash.
 sub FIRSTKEY {
-	my ($self) = @_; 
+	my ($self) = @_;
 	DEBUG("FIRSTKEY()");
 	my $fh = _lock_read($self->{filename});
 
@@ -88,11 +92,11 @@ sub FIRSTKEY {
 
 # Return the next key in the hash.
 sub NEXTKEY {
-	my ($self, $lastkey) = @_;
-	DEBUG("NEXTKEY($lastkey)");
+	my ($self, $key) = @_;
+	DEBUG("NEXTKEY($key)");
 	$key = escape_key($key);
 	my $fh = _lock_read($self->{filename});
-	
+
 	# Read the file to find a key
 	while(my $line = <$fh>) {
 		chomp($line);
@@ -105,7 +109,7 @@ sub NEXTKEY {
 			return $1;
 		} else {
 			# EOF
-			return undef;
+			return;
 		}
 	}
 }
@@ -139,16 +143,16 @@ sub DELETE {
 
 # Clear all values from the tied hash this.
 sub CLEAR {
-	my ($self) = @_; 
+	my ($self) = @_;
 	DEBUG("CLEAR()");
 	my $fh = $self->_lock_write();
 }
 
 sub SCALAR {
-	my ($self) = @_; 
+	my ($self) = @_;
 	DEBUG("SCALAR()");
 	my $fh = _lock_read($self->{filename});
-	
+
 	# Read the file to read the number of lines
 	my $nb_lines = 0;
 	while(my $line = <$fh>) {
@@ -177,15 +181,15 @@ sub _lock_write {
 
 	use Fcntl qw(:flock);
 	use IO::File;
-	
-	my $fh = IO::File->new($filename, $mode) 
+
+	my $fh = IO::File->new($filename, $mode)
 		or die "Cannot open tied file '$filename' - $!";
 	flock($fh, LOCK_EX) or die "Cannot lock tied file '$filename' - $!";
 	return $fh;
 }
 
 sub DEBUG {
-	print STDOUT "[DEBUG] @_" . "\n" if $DEBUG_ENABLED; 
+	print STDOUT "[DEBUG] @_" . "\n" if $DEBUG_ENABLED;
 }
 
 # XXX - collision if there is a ____

lib/Munin/Common/TLS.pm

@@ -53,15 +53,15 @@ sub _start_tls {
     my $self = shift;
 
     my %tls_verified = (
-        level          => 0, 
+        level          => 0,
         cert           => "",
-        verified       => 0, 
-        required_depth => $self->{tls_vdepth}, 
+        verified       => 0,
+        required_depth => $self->{tls_vdepth},
         verify         => $self->{tls_verify},
     );
 
     DEBUG("[TLS] Enabling TLS.") if $self->{DEBUG};
-    
+
     $self->_load_net_ssleay()
         or return 0;
 
@@ -71,16 +71,16 @@ sub _start_tls {
 
     $self->_load_private_key()
         or return 0;
-    
+
     $self->_load_certificate();
 
     $self->_load_ca_certificate();
-    
+
     $self->_initial_communication()
         or return 0;
-    
+
     $self->_set_peer_requirements(\%tls_verified);
-    
+
     if (! ($self->{tls_session} = Net::SSLeay::new($self->{tls_context})))
     {
 	ERROR("Could not create TLS: $!");
@@ -146,8 +146,8 @@ sub _load_private_key {
 
     if (defined $self->{tls_priv} and length $self->{tls_priv}) {
     	if (-e $self->{tls_priv} or $self->{tls_paranoia} eq "paranoid") {
-	    if (Net::SSLeay::CTX_use_PrivateKey_file($self->{tls_context}, 
-                                                     $self->{tls_priv}, 
+	    if (Net::SSLeay::CTX_use_PrivateKey_file($self->{tls_context},
+                                                     $self->{tls_priv},
                                                      &Net::SSLeay::FILETYPE_PEM)) {
                 $self->{private_key_loaded} = 1;
             }
@@ -211,15 +211,15 @@ sub _set_peer_requirements {
     if (defined $err and length $err) {
         WARNING("in set_verify_depth: $err");
     }
-    Net::SSLeay::CTX_set_verify ($self->{tls_context}, 
+    Net::SSLeay::CTX_set_verify ($self->{tls_context},
                                  $self->{tls_verify}  ? &Net::SSLeay::VERIFY_PEER :
                                                         &Net::SSLeay::VERIFY_NONE,
                                  $self->_tls_verify_callback($tls_verified));
     $err = &Net::SSLeay::print_errs("");
     if (defined $err and length $err) {
         WARNING("in set_verify: $err");
     }
-    
+
     return 1;
 }
 
@@ -228,7 +228,7 @@ sub _tls_verify_callback {
     my ($self, $tls_verified) = @_;
 
     return sub {
-        my ($ok, $subj_cert, $issuer_cert, $depth, 
+        my ($ok, $subj_cert, $issuer_cert, $depth,
 	    $errorcode, $arg, $chain) = @_;
 
         $tls_verified->{"level"}++;
@@ -238,7 +238,7 @@ sub _tls_verify_callback {
             DEBUG("[TLS] Verified certificate.") if $self->{DEBUG};
             return 1;           # accept
         }
-        
+
         if (!($tls_verified->{"verify"})) {
             DEBUG("[TLS] Certificate failed verification, but we aren't verifying.") if $self->{DEBUG};
             $tls_verified->{"verified"} = 1;
@@ -321,7 +321,7 @@ sub _accept_or_connect {
     }
     elsif ($self->{"tls_match"} and
     	Net::SSLeay::dump_peer_certificate($self->{tls_session}) !~ /$self->{tls_match}/)
-    { 
+    {
 	ERROR("Could not match pattern \"" . $self->{tls_match} .
 		"\" in dump of certificate.");
 	$self->_on_unmatched_cert();
@@ -342,7 +342,7 @@ sub _accept_or_connect {
 sub _initial_communication {
     my ($self) = @_;
     croak "Abstract method called '_initial_communication', "
-        . "needs to be defined in child" 
+        . "needs to be defined in child"
             if ref $self eq __PACKAGE__;
 }
 
@@ -351,7 +351,7 @@ sub _initial_communication {
 sub _use_key_if_present {
     my ($self) = @_;
     croak "Abstract method called '_use_key_if_present', "
-        . "needs to be defined in child" 
+        . "needs to be defined in child"
             if ref $self eq __PACKAGE__;
 }
 
@@ -365,7 +365,7 @@ sub _on_unmatched_cert {}
 sub read {
     my ($self) = @_;
 
-    croak "Tried to do an encrypted read, but a TLS session is not started" 
+    croak "Tried to do an encrypted read, but a TLS session is not started"
         unless $self->session_started();
 
     my $read = Net::SSLeay::read($self->{tls_session});
@@ -384,7 +384,7 @@ sub read {
 sub write {
     my ($self, $text) = @_;
 
-    croak "Tried to do an encrypted write, but a TLS session is not started" 
+    croak "Tried to do an encrypted write, but a TLS session is not started"
         unless $self->session_started();
 
     DEBUG("DEBUG: > $text") if $self->{DEBUG};
@@ -395,7 +395,7 @@ sub write {
         ERROR("[TLS] Warning in write: $err");
         return 0;
     }
-    
+
     return 1;
 }
 
@@ -413,7 +413,7 @@ __END__
 
 =head1 NAME
 
-Munin::Node::TLS - Abstract base class implementing the STARTTLS protocol
+Munin::Common::TLS - Abstract base class implementing the STARTTLS protocol
 
 
 =head1 SYNOPSIS

lib/Munin/Common/TLSClient.pm

@@ -11,7 +11,7 @@ use Munin::Common::Logger;
 
 sub new {
     my ($class, $args) = @_;
-    
+
     my $self = $class->SUPER::new($args);
 
     $self->{remote_key} = 0;
@@ -28,7 +28,7 @@ sub start_tls {
 
 sub _initial_communication {
     my ($self) = @_;
-    
+
     $self->{write_func}("STARTTLS\n");
     my $tlsresponse = $self->{read_func}();
     if (!defined $tlsresponse) {
@@ -67,7 +67,7 @@ __END__
 
 =head1 NAME
 
-Munin::Node::TLSClient - Implements the client side of the STARTTLS protocol
+Munin::Common::TLSClient - Implements the client side of the STARTTLS protocol
 
 
 =head1 SYNOPSIS

lib/Munin/Common/TLSServer.pm

@@ -31,7 +31,7 @@ sub _initial_communication {
     else {
         $self->{write_func}("TLS MAYBE\n");
     }
-            
+
     return 1;
 }
 
@@ -50,7 +50,7 @@ __END__
 
 =head1 NAME
 
-Munin::Node::TLSServer - Implements the server side of the STARTTLS protocol
+Munin::Common::TLSServer - Implements the server side of the STARTTLS protocol
 
 
 =head1 SYNOPSIS