diff --git a/.perlcriticrc b/.perlcriticrc index ea6efc689a..496de0f7b0 100644 --- a/.perlcriticrc +++ b/.perlcriticrc @@ -44,3 +44,6 @@ severity = 5 [Variables::ProhibitUnusedVariables] severity = 5 + +[Subroutines::RequireFinalReturn] +terminal_funcs = Pod::Usage::pod2usage diff --git a/lib/Munin/Node/Config.pm b/lib/Munin/Node/Config.pm index 717df639d6..8b170a338d 100644 --- a/lib/Munin/Node/Config.pm +++ b/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(@_); } diff --git a/script/munin-run b/script/munin-run index 9ed89701e3..faca2c711c 100755 --- a/script/munin-run +++ b/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,7 +74,7 @@ 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"; @@ -85,70 +82,73 @@ sub main } -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', ); }