From f3e3b390234a08e91d14583b493131cf9f2c21b5 Mon Sep 17 00:00:00 2001 From: Christian Loos Date: Tue, 30 Aug 2016 13:16:33 +0200 Subject: [PATCH 1/8] fix 'Nested named subroutine' --- mysqltuner.pl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/mysqltuner.pl b/mysqltuner.pl index f77996f..356e249 100755 --- a/mysqltuner.pl +++ b/mysqltuner.pl @@ -366,12 +366,12 @@ sub pretty_uptime { # Retrieves the memory installed on this machine my ( $physical_memory, $swap_memory, $duflags ); -sub os_setup { +sub memerror { + badprint "Unable to determine total memory/swap; use '--forcemem' and '--forceswap'"; + exit 1; +} - sub memerror { - badprint "Unable to determine total memory/swap; use '--forcemem' and '--forceswap'"; - exit 1; - } +sub os_setup { my $os = `uname`; $duflags = ( $os =~ /Linux/ ) ? '-b' : ''; if ( $opt{'forcemem'} > 0 ) { From 90e82515233ee3180eaeb865791ec7afd01baa4f Mon Sep 17 00:00:00 2001 From: Christian Loos Date: Tue, 30 Aug 2016 13:27:13 +0200 Subject: [PATCH 2/8] fix 'Bareword file handle opened' --- mysqltuner.pl | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/mysqltuner.pl b/mysqltuner.pl index 356e249..9fd6708 100755 --- a/mysqltuner.pl +++ b/mysqltuner.pl @@ -1036,9 +1036,9 @@ sub remove_empty { sub get_file_contents { my $file = shift; - open( FH, "< $file" ) or die "Can't open $file for read: $!"; - my @lines = ; - close FH or die "Cannot close $file: $!"; + open(my $fh, "<", $file) or die "Can't open $file for read: $!"; + my @lines = <$fh>; + close $fh or die "Cannot close $file: $!"; remove_cr \@lines; return @lines; } @@ -1056,8 +1056,8 @@ sub cve_recommendations { #prettyprint "Look for related CVE for $myvar{'version'} or lower in $opt{cvefile}"; my $cvefound = 0; - open( FH, "<$opt{cvefile}" ) or die "Can't open $opt{cvefile} for read: $!"; - while ( my $cveline = ) { + open(my $fh, "<", $opt{cvefile}) or die "Can't open $opt{cvefile} for read: $!"; + while ( my $cveline = <$fh> ) { my @cve = split( ';', $cveline ); debugprint "Comparing $mysqlvermajor\.$mysqlverminor\.$mysqlvermicro with $cve[1]\.$cve[2]\.$cve[3] : ".(mysql_version_le( $cve[1], $cve[2], $cve[3] )?'<=':'>'); @@ -1069,7 +1069,7 @@ sub cve_recommendations { $cvefound++; } } - close FH or die "Cannot close $opt{cvefile}: $!"; + close $fh or die "Cannot close $opt{cvefile}: $!"; $result{'CVE'}{'nb'}=$cvefound; if ( $cvefound == 0 ) { goodprint "NO SECURITY CVE FOUND FOR YOUR VERSION"; From 7215e419dfd891be1ae07399e47ff504cf3e1abc Mon Sep 17 00:00:00 2001 From: Christian Loos Date: Tue, 30 Aug 2016 14:50:36 +0200 Subject: [PATCH 3/8] fix 'Expression form of "eval"' --- mysqltuner.pl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mysqltuner.pl b/mysqltuner.pl index 9fd6708..ac4bfa7 100755 --- a/mysqltuner.pl +++ b/mysqltuner.pl @@ -4038,7 +4038,7 @@ sub dump_result { debugprint "HTML REPORT: $opt{'reportfile'}"; if ( $opt{'reportfile'} ne 0 ) { - eval "{ use Text::Template }"; + eval {require Text::Template}; if ($@) { badprint "Text::Template Module is needed."; exit 1; @@ -4062,7 +4062,7 @@ sub dump_result { close $fh; } if ( $opt{'json'} ne 0 ) { - eval "{ use JSON }"; + eval {require JSON}; if ($@) { print "JSON Module is needed."; exit 1; From 1c13c66e36b150902a394158d6377baad2a457c5 Mon Sep 17 00:00:00 2001 From: Christian Loos Date: Tue, 30 Aug 2016 15:37:42 +0200 Subject: [PATCH 4/8] improve message if JSON isn't installed --- mysqltuner.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysqltuner.pl b/mysqltuner.pl index ac4bfa7..24e1184 100755 --- a/mysqltuner.pl +++ b/mysqltuner.pl @@ -4064,7 +4064,7 @@ sub dump_result { if ( $opt{'json'} ne 0 ) { eval {require JSON}; if ($@) { - print "JSON Module is needed."; + print "$bad JSON Module is needed.\n"; exit 1; } my $json = JSON->new->allow_nonref; From 600aad227dd78607207bbca4be39fcd89f440a28 Mon Sep 17 00:00:00 2001 From: Christian Loos Date: Tue, 30 Aug 2016 16:36:21 +0200 Subject: [PATCH 5/8] get_file_contents calls already remove_cr --- mysqltuner.pl | 4 ---- 1 file changed, 4 deletions(-) diff --git a/mysqltuner.pl b/mysqltuner.pl index 24e1184..0b133d6 100755 --- a/mysqltuner.pl +++ b/mysqltuner.pl @@ -1130,7 +1130,6 @@ sub get_other_process_memory { sub get_os_release { if ( -f "/etc/lsb-release" ) { my @info_release = get_file_contents "/etc/lsb-release"; - remove_cr @info_release; my $os_relase = $info_release[3]; $os_relase =~ s/.*="//; $os_relase =~ s/"$//; @@ -1139,13 +1138,11 @@ sub get_os_release { if ( -f "/etc/system-release" ) { my @info_release = get_file_contents "/etc/system-release"; - remove_cr @info_release; return $info_release[0]; } if ( -f "/etc/os-release" ) { my @info_release = get_file_contents "/etc/os-release"; - remove_cr @info_release; my $os_relase = $info_release[0]; $os_relase =~ s/.*="//; $os_relase =~ s/"$//; @@ -1154,7 +1151,6 @@ sub get_os_release { if ( -f "/etc/issue" ) { my @info_release = get_file_contents "/etc/issue"; - remove_cr @info_release; my $os_relase = $info_release[0]; $os_relase =~ s/\s+\\n.*//; return $os_relase; From 0cd7e93685a125f97b1f1f085c0b7ed6c70a2cb6 Mon Sep 17 00:00:00 2001 From: Christian Loos Date: Tue, 30 Aug 2016 17:20:56 +0200 Subject: [PATCH 6/8] fix "Don't modify $_ in list functions" --- mysqltuner.pl | 58 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/mysqltuner.pl b/mysqltuner.pl index 0b133d6..9e0d8eb 100755 --- a/mysqltuner.pl +++ b/mysqltuner.pl @@ -1026,8 +1026,12 @@ sub get_all_vars { } sub remove_cr { - map { s/\n$//g; } @_; - map { s/^\s+$//g; } @_; + return map { + my $line = $_; + $line =~ s/\n$//g; + $line =~ s/^\s+$//g; + $line; + } @_; } sub remove_empty { @@ -1039,7 +1043,7 @@ sub get_file_contents { open(my $fh, "<", $file) or die "Can't open $file for read: $!"; my @lines = <$fh>; close $fh or die "Cannot close $file: $!"; - remove_cr \@lines; + @lines = remove_cr @lines; return @lines; } @@ -1084,9 +1088,11 @@ sub cve_recommendations { sub get_opened_ports { my @opened_ports = `netstat -ltn`; - map { - s/.*:(\d+)\s.*$/$1/; - s/\D//g; + @opened_ports = map { + my $v = $_; + $v =~ s/.*:(\d+)\s.*$/$1/; + $v =~ s/\D//g; + $v; } @opened_ports; @opened_ports = sort { $a <=> $b } grep { !/^$/ } @opened_ports; debugprint Dumper \@opened_ports; @@ -1111,16 +1117,18 @@ sub get_process_memory { sub get_other_process_memory { my @procs = `ps eaxo pid,command`; - map { - s/.*PID.*//; - s/.*mysqld.*//; - s/.*\[.*\].*//; - s/^\s+$//g; - s/.*PID.*CMD.*//; - s/.*systemd.*//; + @procs = map { + my $v = $_; + $v =~ s/.*PID.*//; + $v =~ s/.*mysqld.*//; + $v =~ s/.*\[.*\].*//; + $v =~ s/^\s+$//g; + $v =~ s/.*PID.*CMD.*//; + $v =~ s/.*systemd.*//; + $v =~ s/\s*?(\d+)\s*.*/$1/g; + $v; } @procs; - map { s/\s*?(\d+)\s*.*/$1/g; } @procs; - remove_cr @procs; + @procs = remove_cr @procs; @procs = remove_empty @procs; my $totalMemOther = 0; map { $totalMemOther += get_process_memory($_); } @procs; @@ -1162,7 +1170,11 @@ sub get_fs_info() { my @sinfo = `df -P | grep '%'`; my @iinfo = `df -Pi| grep '%'`; shift @iinfo; - map { s/.*\s(\d+)%\s+(.*)/$1\t$2/g } @sinfo; + @sinfo = map { + my $v= $_; + $v =~ s/.*\s(\d+)%\s+(.*)/$1\t$2/g; + $v; + } @sinfo; foreach my $info (@sinfo) { next if $info =~ m{(\d+)\t/(run|dev|sys|proc)($|/)}; if ( $info =~ /(\d+)\t(.*)/ ) { @@ -1177,7 +1189,11 @@ sub get_fs_info() { } } - map { s/.*\s(\d+)%\s+(.*)/$1\t$2/g } @iinfo; + @iinfo = map { + my $v = $_; + $v =~ s/.*\s(\d+)%\s+(.*)/$1\t$2/g; + $v; + } @iinfo; foreach my $info (@iinfo) { next if $info =~ m{(\d+)\t/(run|dev|sys|proc)($|/)}; if ( $info =~ /(\d+)\t(.*)/ ) { @@ -1217,7 +1233,7 @@ sub infocmd { my $cmd = "@_"; debugprint "CMD: $cmd"; my @result = `$cmd`; - remove_cr @result; + @result = remove_cr @result; for my $l (@result) { infoprint "$l"; } @@ -1227,7 +1243,7 @@ sub infocmd_tab { my $cmd = "@_"; debugprint "CMD: $cmd"; my @result = `$cmd`; - remove_cr @result; + @result = remove_cr @result; for my $l (@result) { infoprint "\t$l"; } @@ -1236,7 +1252,7 @@ sub infocmd_tab { sub infocmd_one { my $cmd = "@_"; my @result = `$cmd`; - remove_cr @result; + @result = remove_cr @result; return join ', ', @result; } @@ -3197,7 +3213,7 @@ sub get_wsrep_options { return () unless defined $myvar{'wsrep_provider_options'}; my @galera_options = split /;/, $myvar{'wsrep_provider_options'}; - remove_cr @galera_options; + @galera_options = remove_cr @galera_options; @galera_options = remove_empty @galera_options; debugprint Dumper( \@galera_options ); return @galera_options; From 4fbc5ecb35f8afcf757b87bb23c6130bbf0b69ea Mon Sep 17 00:00:00 2001 From: Christian Loos Date: Tue, 30 Aug 2016 18:09:41 +0200 Subject: [PATCH 7/8] let travis run perlcritic --- .travis.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index e65059c..b7d44ef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,11 +35,14 @@ before_install: install: - cpanm --quiet --notest Data::Dumper - - cpanm --quiet --notest Text::Template - cpanm --quiet --notest JSON + - cpanm --quiet --notest Perl::Critic + - cpanm --quiet --notest Text::Template before_script: - echo -e "[client]\nuser=root\npassword=\"\"" > .my.cnf - chmod 600 .my.cnf -script: ./mysqltuner.pl --idxstat --dbstat +script: + - perlcritic mysqltuner.pl + - ./mysqltuner.pl --idxstat --dbstat From dddf7e2ba42f248d097dc050952fceec36dfc8bd Mon Sep 17 00:00:00 2001 From: Christian Loos Date: Tue, 30 Aug 2016 18:24:10 +0200 Subject: [PATCH 8/8] add perlcritic to CONTRIBUTING.md --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f6da536..3e913c2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -182,7 +182,7 @@ Copy of the license is available at [LICENSE](https://github.com/major/MySQLTune #### MySQLTuner Code Conventions -1. Check code convention using **perltidy** +1. Check code convention using **perltidy** and **perlcritic** 2. Don't manually update the version number in `mysqltuner.pl`.