[munin-run] cleanup

- format with perltidy
- fix some perlciritc warnings

Author: cgzones

.perlcriticrc

@@ -44,3 +44,6 @@ severity = 5
 
 [Variables::ProhibitUnusedVariables]
 severity = 5
+
+[Subroutines::RequireFinalReturn]
+terminal_funcs = Pod::Usage::pod2usage

lib/Munin/Node/Config.pm

@@ -49,7 +49,7 @@ sub parse_config_from_file
 
     # Check permissions of configuration
     unless (Munin::Node::OS->check_perms_if_paranoid($file)) {
-        croak "Fatal error. Bailing out.";
+        croak "Permissions of file '$file' failed. Bailing out.";
     }
     return $self->SUPER::parse_config_from_file(@_);
 }

script/munin-run

@@ -34,7 +34,6 @@ use Munin::Node::Service;
 
 use English qw(-no_match_vars);
 
-my $services;
 my $servicedir;
 my $conffile = "$Munin::Common::Defaults::MUNIN_CONFDIR/munin-node.conf";
 my $DEBUG    = 0;
@@ -44,31 +43,29 @@ my $paranoia = 0;
 
 my $config = Munin::Node::Config->instance();
 
-sub main
-{
-    my ($plugin, $arg) = parse_args();
+sub main {
+    my ( $plugin, $arg ) = parse_args();
 
     # Loads the settings from munin-node.conf.
     # Ensures that, where options can be set both in the config and in
     # @ARGV, the latter takes precedence.
     $paranoia = $config->{paranoia};
 
-    my $config = Munin::Node::Config->instance();
     $config->parse_config_from_file($conffile);
 
-    $services = Munin::Node::Service->new(
+    my $services = Munin::Node::Service->new(
         servicedir => $servicedir,
         defuser    => $config->{defuser},
         defgroup   => $config->{defgroup},
         pidebug    => $PIDEBUG,
     );
 
-    $config->reinitialize({
-        %$config,
-        paranoia   => $paranoia,
-    });
+    $config->reinitialize( {
+        %{$config},    ## no critic qw(ValuesAndExpressions::ProhibitCommaSeparatedStatements)
+        paranoia => $paranoia,
+    } );
 
-    unless ($services->is_a_runnable_service($plugin)) {
+    unless ( $services->is_a_runnable_service($plugin) ) {
         print STDERR "# Unknown service '$plugin'\n";
         exit 1;
     }
@@ -77,78 +74,81 @@ sub main
 
     # no need for a timeout -- the user can kill this process any
     # time they want.
-    $services->exec_service($plugin, $arg);
+    $services->exec_service( $plugin, $arg );
 
     # Never reached, but just in case...
     print STDERR "# FATAL: Failed to exec.\n";
     exit 42;
 }
 
 
-sub parse_args
-{
+sub parse_args {
+
     # Default configuration values
-    my $sconfdir   = "$Munin::Common::Defaults::MUNIN_CONFDIR/plugin-conf.d";
+    my $sconfdir = "$Munin::Common::Defaults::MUNIN_CONFDIR/plugin-conf.d";
     my $sconffile;
 
-    my ($plugin, $arg);
-
-    print_usage_and_exit() unless GetOptions(
-            "config=s"     => \$conffile,
-            "debug!"       => \$DEBUG,
-            "verbose!"     => \$VERBOSE,
-            "pidebug!"     => \$PIDEBUG,
-            "servicedir=s" => \$servicedir,
-            "sconfdir=s"   => \$sconfdir,
-            "sconffile=s"  => \$sconffile,
-            "paranoia!"    => \$paranoia,
-            "version"      => \&print_version_and_exit,
-            "help"         => \&print_usage_and_exit,
-    );
+    my ( $plugin, $arg );
+
+    print_usage_and_exit()
+      unless GetOptions(
+        "config=s"     => \$conffile,
+        "debug!"       => \$DEBUG,
+        "verbose!"     => \$VERBOSE,
+        "pidebug!"     => \$PIDEBUG,
+        "servicedir=s" => \$servicedir,
+        "sconfdir=s"   => \$sconfdir,
+        "sconffile=s"  => \$sconffile,
+        "paranoia!"    => \$paranoia,
+        "version"      => \&print_version_and_exit,
+        "help"         => \&print_usage_and_exit,
+      );
 
     print_usage_and_exit() unless $ARGV[0];
 
     # Detaint the plugin name
-    ($plugin) = ($ARGV[0] =~ m/^([-\w.:]+)$/) or die "# ERROR: Invalid plugin name '$ARGV[0].\n";
-    if ($ARGV[1]) {
-        ($arg) = ($ARGV[1] =~ m/^(\w+)$/)
-            or die "# ERROR: Invalid characters in argument '$ARGV[1]'.\n";
+    ($plugin) = ( $ARGV[0] =~ m/^([-\w.:]+)$/x )
+      or die "# ERROR: Invalid plugin name '$ARGV[0]'.\n";
+    if ( $ARGV[1] ) {
+        ($arg) = ( $ARGV[1] =~ m/^(\w+)$/x )
+          or die "# ERROR: Invalid characters in argument '$ARGV[1]'.\n";
+        die "# ERROR: Invalid plugin argument '$ARGV[1]'.\n"
+          unless grep( /^$ARGV[1]$/x, qw(autoconf config suggest) );
     }
 
-    # Detaint service directory.  FIXME: do more strict detainting?
+    # Detaint service directory.
     if ($servicedir) {
-        $servicedir =~ /(.*)/;
-        $servicedir = $1;
+        die "# ERROR: Invalid servicedir supplied.\n" unless -d $servicedir;
+        $servicedir =~ /(.*)/x;
+        $servicedir = $1;    ## no critic qw(RegularExpressions::ProhibitCaptureWithoutTest)
     }
 
     # Update the config
-    $config->reinitialize({
-        %$config,
-
-        sconfdir   => $sconfdir,
-        conffile   => $conffile,
-        sconffile  => $sconffile,
-        VERBOSE    => $VERBOSE,
-        DEBUG      => $DEBUG,
-        paranoia   => $paranoia,
-    });
-
-    return ($plugin, $arg);
+    $config->reinitialize( {
+            %{$config},      ## no critic qw(ValuesAndExpressions::ProhibitCommaSeparatedStatements)
+
+            sconfdir  => $sconfdir,
+            conffile  => $conffile,
+            sconffile => $sconffile,
+            VERBOSE   => $VERBOSE,
+            DEBUG     => $DEBUG,
+            paranoia  => $paranoia,
+    } );
+
+    return ( $plugin, $arg );
 }
 
 
-sub print_usage_and_exit
-{
+sub print_usage_and_exit {
     require Pod::Usage;
-    Pod::Usage::pod2usage(-verbose => 1);
+    Pod::Usage::pod2usage( -verbose => 1 );
 }
 
 
-sub print_version_and_exit
-{
+sub print_version_and_exit {
     require Pod::Usage;
     Pod::Usage::pod2usage(
-        -verbose => 99,
+        -verbose  => 99,
         -sections => 'VERSION|COPYRIGHT',
     );
 }