From 2889c43fc43748553be2846944e4ff028d8d0004 Mon Sep 17 00:00:00 2001
From: Joe Cooper <joe@virtualmin.com>
Date: Thu, 30 Jun 2016 19:34:54 -0700
Subject: [PATCH] strict/warn

---
 cgi_args.pl            |   4 +
 edit.cgi               |  33 ++++---
 edit_tmpl.cgi          |  14 ++-
 index.cgi              |  33 ++++---
 mass.cgi               |  26 +++---
 save.cgi               |  30 +++---
 save_tmpl.cgi          |   9 +-
 virtual_feature.pl     |  41 +++++----
 virtualmin-init-lib.pl | 202 ++++++++++++++++++++++-------------------
 9 files changed, 225 insertions(+), 167 deletions(-)

diff --git a/cgi_args.pl b/cgi_args.pl
index ca98fe2..71baf14 100755
--- a/cgi_args.pl
+++ b/cgi_args.pl
@@ -1,3 +1,7 @@
+use strict;
+use warnings;
+our (%access);
+our $module_name;
 
 do 'virtualmin-init-lib.pl';
 
diff --git a/edit.cgi b/edit.cgi
index 1d5b485..a79a522 100755
--- a/edit.cgi
+++ b/edit.cgi
@@ -1,16 +1,22 @@
 #!/usr/local/bin/perl
 # Show a page for creating or editing a bootup action associated with
 # some domain.
+use strict;
+use warnings;
+our (%text, %in, %config);
+our $module_name;
 
 require './virtualmin-init-lib.pl';
 &ReadParse();
-$d = $in{'dom'} ? &virtual_server::get_domain($in{'dom'}) : undef;
+my $d = $in{'dom'} ? &virtual_server::get_domain($in{'dom'}) : undef;
 &ui_print_header($d ? &virtual_server::domain_in($d) : undef,
 		 $in{'new'} ? $text{'edit_title1'} : $text{'edit_title2'}, "");
 
+my $init;
+my $tmpl;
 if (!$in{'new'}) {
 	# Get the existing action
-	($init) = grep { $_->{'id'} eq $in{'id'} } 
+	($init) = grep { $_->{'id'} eq $in{'id'} }
 		       &list_domain_actions($d);
 	$init || &error($text{'edit_egone'});
 	}
@@ -34,7 +40,7 @@ print &ui_table_start($text{'edit_header'}, undef, 2);
 
 # Domain selector, if needed
 if (!$in{'dom'}) {
-	@doms = grep { $_->{$module_name} &&
+	my @doms = grep { $_->{$module_name} &&
 		       &virtual_server::can_edit_domain($_) }
 		     &virtual_server::list_domains();
 	print &ui_table_row($text{'edit_dom'},
@@ -77,11 +83,11 @@ if ($tmpl) {
 	print &ui_table_row($text{'edit_tmpl'}, $tmpl->{'desc'});
 
 	# Show parameters
-	for($i=0; defined($tmpl->{'pname_'.$i}); $i++) {
-		$tt = $tmpl->{'ptype_'.$i};
-		$tn = 'param_'.$tmpl->{'pname_'.$i};
-		$of = $tmpl->{'popts_'.$i};
-		@opts = $of ? &read_opts_file($of) : ( );
+	for(my $i=0; defined($tmpl->{'pname_'.$i}); $i++) {
+		my $tt = $tmpl->{'ptype_'.$i};
+		my $tn = 'param_'.$tmpl->{'pname_'.$i};
+		my $of = $tmpl->{'popts_'.$i};
+		my @opts = $of ? &read_opts_file($of) : ( );
 		print &ui_table_row($tmpl->{'pdesc_'.$i},
 			$tt == 0 ? &ui_textbox($tn, undef, 50) :
 			$tt == 1 ? &ui_textbox($tn, undef, 10) :
@@ -93,12 +99,12 @@ if ($tmpl) {
 		}
 
 	# Show fixed code for start and stop
-	$start = $tmpl->{'start'};
+	my $start = $tmpl->{'start'};
 	$start = &substitute_template($start, $d) if ($d);
 	print &ui_table_row($text{'edit_start'},
 			    "<pre>".&html_escape($start)."</pre>");
 
-	$stop = $tmpl->{'stop'};
+	my $stop = $tmpl->{'stop'};
 	$stop = &substitute_template($stop, $d) if ($d);
 	print &ui_table_row($text{'edit_stop'},
 			    $stop eq ":kill" ? $text{'edit_stopkill'} :
@@ -110,6 +116,7 @@ else {
 		&ui_textarea("start", $init->{'start'}, 5, 80));
 
 	# Stop code
+	my $stopdef;
 	if ($config{'mode'} eq 'smf') {
 		$stopdef = &ui_radio(
 			     "stop_def", $init->{'stop'} eq ':kill' ? 1 : 0,
@@ -123,9 +130,9 @@ else {
 
 	# Current processes
 	if (!$in{'new'}) {
-		@procs = &get_started_processes($init);
+		my @procs = &get_started_processes($init);
 		if (@procs) {
-			@table = ( );
+			my @table = ( );
 			foreach my $p (@procs) {
 				push(@table, [ $p->{'pid'}, $p->{'cpu'},
 					       $p->{'size'}, $p->{'args'} ]);
@@ -162,5 +169,3 @@ else {
 	}
 
 &ui_print_footer("index.cgi?dom=$in{'dom'}", $text{'index_return'});
-
-
diff --git a/edit_tmpl.cgi b/edit_tmpl.cgi
index 125664c..9f38971 100755
--- a/edit_tmpl.cgi
+++ b/edit_tmpl.cgi
@@ -1,5 +1,8 @@
 #!/usr/local/bin/perl
 # Show a page for creating or editing an action template
+use strict;
+use warnings;
+our (%access, %text, %in, %config);
 
 require './virtualmin-init-lib.pl';
 &ReadParse();
@@ -7,6 +10,7 @@ $access{'templates'} || &error($text{'tmpl_ecannot'});
 &ui_print_header(undef,
 		 $in{'new'} ? $text{'tmpl_title1'} : $text{'tmpl_title2'}, "");
 
+my $tmpl;
 if (!$in{'new'}) {
 	# Get the existing template
 	($tmpl) = grep { $_->{'id'} == $in{'id'} } &list_action_templates();
@@ -30,6 +34,7 @@ print &ui_table_row($text{'edit_start'},
 	&ui_textarea("start", $tmpl->{'start'}, 5, 80));
 
 # Stop code
+my $stopdef;
 if ($config{'mode'} eq 'smf') {
 	$stopdef = &ui_radio("stop_def", $tmpl->{'stop'} eq ':kill' ? 1 : 0,
 			     [ [ 1, $text{'edit_stopkill'} ],
@@ -54,9 +59,10 @@ print &ui_table_end();
 # Section for additional parameters
 print &ui_table_start($text{'tmpl_header2'}, undef, 2);
 
-@table = ( );
+my @table = ( );
+my $pmax;
 for($pmax=0; defined($tmpl->{'pname_'.$pmax}); $pmax++) { }
-for($i=0; $i < $pmax+3; $i++) {
+for(my $i=0; $i < $pmax+3; $i++) {
 	push(@table, [
 		&ui_textbox("pname_$i", $tmpl->{'pname_'.$i}, 10),
 		&ui_select("ptype_$i", $tmpl->{'ptype_'.$i},
@@ -73,7 +79,7 @@ for($i=0; $i < $pmax+3; $i++) {
 		&ui_textbox("pdesc_$i", $tmpl->{'pdesc_'.$i}, 50),
 		]);
 	}
-$ptable = &ui_columns_table(
+my $ptable = &ui_columns_table(
 	[ $text{'tmpl_pname'}, $text{'tmpl_ptype'},
 	  $text{'tmpl_popts'}, $text{'tmpl_pdesc'} ],
 	100,
@@ -92,5 +98,3 @@ else {
 	}
 
 &ui_print_footer("", $text{'index_return'});
-
-
diff --git a/index.cgi b/index.cgi
index c943583..fe98c1e 100755
--- a/index.cgi
+++ b/index.cgi
@@ -1,14 +1,21 @@
 #!/usr/local/bin/perl
 # Show a list of bootup scripts that the user already has configured
+use strict;
+use warnings;
+our (%access, %text, %in);
+our $module_name;
 
 require './virtualmin-init-lib.pl';
 &ReadParse();
-$d = $in{'dom'} ? &virtual_server::get_domain($in{'dom'}) : undef;
+my $d = $in{'dom'} ? &virtual_server::get_domain($in{'dom'}) : undef;
 &ui_print_header($d ? &virtual_server::domain_in($d) : undef,
 		 $text{'index_title'}, "", undef, 0, 1);
-@templates = &list_action_templates();
+my @templates = &list_action_templates();
 
 # Work out domains to work on
+my @doms;
+my $many;
+
 if ($d) {
 	# Just one
 	@doms = ( $d );
@@ -22,6 +29,7 @@ else {
 	}
 
 # Show existing init scripts for all domains
+my @allinits;
 foreach my $d (@doms) {
 	foreach my $i (&list_domain_actions($d)) {
 		$i->{'dom'} = $d->{'id'};
@@ -31,8 +39,9 @@ foreach my $d (@doms) {
 	}
 
 # Work out limit
+my $no_create;
 if ($access{'max'}) {
-	$c = &count_user_actions();
+	my $c = &count_user_actions();
 	if ($c >= $access{'max'}) {
 		print "<b>",&text('index_hitmax', $access{'max'}),"</b><p>\n";
 		$no_create = 1;
@@ -46,20 +55,20 @@ if ($access{'max'}) {
 	}
 
 # Build contents for table of actions
-@links = ( );
+my @links;
 if (!$no_create) {
 	push(@links, [ "edit.cgi?new=1&dom=".&urlize($in{'dom'}),
 		       $text{'index_add'} ]);
-	foreach $tmpl (@templates) {
+	foreach my $tmpl (@templates) {
 		push(@links, [ "edit.cgi?new=1&dom=".&urlize($in{'dom'}).
 			       "&tmpl=$tmpl->{'id'}",
 			       &text('index_add2', $tmpl->{'desc'}) ]);
 		}
 	}
-$green = "<font color=#00aa00>$text{'yes'}</font>";
-$red = "<font color=#ff0000>$text{'no'}</font>";
-$orange = "<font color=#ffaa00>$text{'index_maint'}</font>";
-@table = ( );
+my $green = "<font color=#00aa00>$text{'yes'}</font>";
+my $red = "<font color=#ff0000>$text{'no'}</font>";
+my $orange = "<font color=#ffaa00>$text{'index_maint'}</font>";
+my @table;
 foreach my $i (sort { $a->{'name'} cmp $b->{'name'} } @allinits) {
 	push(@table, [
 		{ 'type' => 'checkbox', 'name' => 'd',
@@ -97,14 +106,14 @@ print &ui_form_columns_table(
 	undef,
 	$in{'dom'} ? $text{'index_none'} : $text{'index_none2'}
 	);
-	
+
 
 # Show list of templates
 if ($access{'templates'}) {
 	print &ui_hr();
 	print $text{'index_tdesc2'},"<p>\n";
 	@table = ( );
-	foreach $t (@templates) {
+	foreach my $t (@templates) {
 		push(@table, [
 			"<a href='edit_tmpl.cgi?id=$t->{'id'}'>".
 			"$t->{'desc'}</a>",
@@ -129,7 +138,7 @@ if ($access{'templates'}) {
 
 sub shorten_command
 {
-local ($cmd) = @_;
+my ($cmd) = @_;
 if ($cmd eq ":kill") {
 	return $text{'index_kill'};
 	}
diff --git a/mass.cgi b/mass.cgi
index f429434..53228bd 100755
--- a/mass.cgi
+++ b/mass.cgi
@@ -1,25 +1,30 @@
 #!/usr/local/bin/perl
 # Start, stop or delete multiple actions
+use strict;
+use warnings;
+our (%text, %in);
 
 require './virtualmin-init-lib.pl';
 &ReadParse();
-@d = split(/\0/, $in{'d'});
+my @d = split(/\0/, $in{'d'});
 @d || &error($text{'mass_enone'});
 
 # Get the actions
-foreach $di (@d) {
-	($did, $name) = split(/\//, $di);
-	local $d = &virtual_server::get_domain($did);
-	local ($init) = grep { $_->{'name'} eq $name } &list_domain_actions($d);
+my @dominits;
+foreach my $di (@d) {
+	my ($did, $name) = split(/\//, $di);
+	my $d = &virtual_server::get_domain($did);
+	my ($init) = grep { $_->{'name'} eq $name } &list_domain_actions($d);
 	if ($d && $init) {
 		push(@dominits, [ $d, $init ]);
 		}
 	}
 
-$idx = "index.cgi?dom=$dominits[0]->[0]->{'id'}";
+my $idx = "index.cgi?dom=$dominits[0]->[0]->{'id'}";
+my $ex;
 if ($in{'delete'}) {
 	# Delete them all
-	foreach $di (@dominits) {
+	foreach my $di (@dominits) {
 		&delete_domain_action($di->[0], $di->[1]);
 		}
 	&redirect($idx);
@@ -28,7 +33,7 @@ elsif ($in{'startnow'}) {
 	# Start them in series
 	&ui_print_unbuffered_header(undef, $text{'start_titles'}, "");
 
-	foreach $di (@dominits) {
+	foreach my $di (@dominits) {
 		print &text('start_starting',
 			    "<tt>$di->[1]->{'name'}</tt>"),"\n";
 		print "<pre>";
@@ -42,7 +47,7 @@ elsif ($in{'stopnow'}) {
 	# Stop them in series
 	&ui_print_unbuffered_header(undef, $text{'stop_titles'}, "");
 
-	foreach $di (@dominits) {
+	foreach my $di (@dominits) {
 		print &text('stop_stopping',
 			    "<tt>$di->[1]->{'name'}</tt>"),"\n";
 		print "<pre>";
@@ -56,7 +61,7 @@ elsif ($in{'restartnow'}) {
 	# Restart them in series
 	&ui_print_unbuffered_header(undef, $text{'restart_titles'}, "");
 
-	foreach $di (@dominits) {
+	foreach my $di (@dominits) {
 		print &text('restart_restarting',
 			    "<tt>$di->[1]->{'name'}</tt>"),"\n";
 		print "<pre>";
@@ -69,4 +74,3 @@ elsif ($in{'restartnow'}) {
 else {
 	&error("No button clicked!");
 	}
-
diff --git a/save.cgi b/save.cgi
index 93e0efd..33b8b34 100755
--- a/save.cgi
+++ b/save.cgi
@@ -1,13 +1,19 @@
 #!/usr/local/bin/perl
 # Create, update, delete, start or stop some action
+use strict;
+use warnings;
+our (%access, %text, %in);
 
 require './virtualmin-init-lib.pl';
 &ReadParse();
-$d = &virtual_server::get_domain($in{'dom'});
+my $d = &virtual_server::get_domain($in{'dom'});
 &virtual_server::can_edit_domain($d) || &error($text{'save_ecannot'});
 
 # Get the current boot action
-@inits = &list_domain_actions($d);
+my @inits = &list_domain_actions($d);
+my $init;
+my $oldinit;
+my $ex; # XXX This is never used, but is captured as a result.
 if ($in{'id'}) {
 	($init) = grep { $_->{'id'} eq $in{'id'} } @inits;
 	$init || &error($text{'edit_egone'});
@@ -15,7 +21,7 @@ if ($in{'id'}) {
 	}
 elsif ($access{'max'}) {
 	# Check if limit was hit
-	$c = &count_user_actions();
+	my $c = &count_user_actions();
 	if ($c >= $access{'max'}) {
 		&error(&text('save_etoomany', $access{'max'}));
 		}
@@ -56,23 +62,24 @@ else {
 	$in{'name'} =~ /^[a-z0-9\.\-\_]+$/i || &error($text{'save_ename'});
 	if ($in{'new'} || $in{'name'} ne $init->{'name'}) {
 		# Check for clash
-		($clash) = grep { $_->{'name'} eq $in{'name'} } @inits;
+		my ($clash) = grep { $_->{'name'} eq $in{'name'} } @inits;
 		$clash && &error($text{'save_eclash'});
 		}
 	$init->{'name'} = $in{'name'};
 	$in{'desc'} =~ /\S/ || &error($text{'save_edesc'});
 	$init->{'desc'} = $in{'desc'};
 	$init->{'status'} = $in{'status'};
-	%tparams = ( );
+	my %tparams;
+	my $tmpl;
 	if ($in{'new'} && $in{'tmpl'}) {
 		# From template
 		($tmpl) = grep { $_->{'id'} == $in{'tmpl'} }
 				&list_action_templates();
-		for($i=0; defined($tmpl->{'pname_'.$i}); $i++) {
-			$td = $tmpl->{'pdesc_'.$i};
-			$tt = $tmpl->{'ptype_'.$i};
-			$tn = $tmpl->{'pname_'.$i};
-			$tv = $in{'param_'.$tn};
+		for(my $i=0; defined($tmpl->{'pname_'.$i}); $i++) {
+			my $td = $tmpl->{'pdesc_'.$i};
+			my $tt = $tmpl->{'ptype_'.$i};
+			my $tn = $tmpl->{'pname_'.$i};
+			my $tv = $in{'param_'.$tn};
 			if ($tt == 0 || $tt == 2) {
 				$tv =~ /\S/ ||
 					&error(&text('save_eptype0', $td));
@@ -87,7 +94,7 @@ else {
 				$tparams{$tn} = $tv;
 				}
 			}
-		%thash = ( %$d, %tparams );
+		my %thash = ( %$d, %tparams );
 		$thash{'name'} = $init->{'name'};
 		$init->{'start'} = &substitute_template(
 					$tmpl->{'start'}, \%thash);
@@ -122,4 +129,3 @@ else {
 		}
 	&redirect("index.cgi?dom=$in{'dom'}");
 	}
-
diff --git a/save_tmpl.cgi b/save_tmpl.cgi
index eedc21f..91964a0 100755
--- a/save_tmpl.cgi
+++ b/save_tmpl.cgi
@@ -1,10 +1,14 @@
 #!/usr/local/bin/perl
 # Create, update or delete some action template
+use strict;
+use warnings;
+our (%access, %text, %in, %config);
 
 require './virtualmin-init-lib.pl';
 &ReadParse();
 $access{'templates'} || &error($text{'tmpl_ecannot'});
 
+my $tmpl;
 if (!$in{'new'}) {
 	# Get the existing template
 	($tmpl) = grep { $_->{'id'} == $in{'id'} } &list_action_templates();
@@ -40,12 +44,12 @@ else {
 		}
 
 	# Validate user-definable parameters
-	for($i=0; defined($tmpl->{'pname_'.$i}); $i++) {
+	for(my $i=0; defined($tmpl->{'pname_'.$i}); $i++) {
 		delete($tmpl->{'pname_'.$i});
 		delete($tmpl->{'ptype_'.$i});
 		delete($tmpl->{'pdesc_'.$i});
 		}
-	for($i=0; defined($in{'pname_'.$i}); $i++) {
+	for(my $i=0; defined($in{'pname_'.$i}); $i++) {
 		next if (!$in{'pname_'.$i});
 		$in{'pname_'.$i} =~ /^[a-z0-9_]+$/i ||
 			&error(&text('tmpl_epname', $i+1));
@@ -68,4 +72,3 @@ else {
 	&save_action_template($tmpl);
 	&redirect("");
 	}
-
diff --git a/virtual_feature.pl b/virtual_feature.pl
index 1f46c8d..0e96e61 100755
--- a/virtual_feature.pl
+++ b/virtual_feature.pl
@@ -1,7 +1,11 @@
 # Defines functions for this feature
+use strict;
+use warnings;
+our (%text);
+our $module_name;
 
 do 'virtualmin-init-lib.pl';
-$input_name = $module_name;
+my $input_name = $module_name;
 $input_name =~ s/[^A-Za-z0-9]/_/g;
 
 # feature_name()
@@ -23,7 +27,7 @@ sub feature_losing
 # editing form
 sub feature_label
 {
-local ($edit) = @_;
+my ($edit) = @_;
 return $edit ? $text{'feat_label2'} : $text{'feat_label'};
 }
 
@@ -79,14 +83,14 @@ sub feature_setup
 # user or domain is changed
 sub feature_modify
 {
-local ($d, $oldd) = @_;
+my ($d, $oldd) = @_;
 if ($d->{'dom'} ne $oldd->{'dom'} ||
     $d->{'user'} ne $oldd->{'user'}) {
 	# Need to re-save all actions under the new user or domain name
 	&$virtual_server::first_print($text{'feat_rename'});
-	local $c = 0;
+	my $c = 0;
 	foreach my $init (&list_domain_actions($oldd)) {
-		local $oldinit = { %$init };
+		my $oldinit = { %$init };
 		$init->{'user'} = $d->{'user'};
 		&modify_domain_action($d, $oldd, $init, $oldinit);
 		$c++;
@@ -106,9 +110,9 @@ sub feature_modify
 # Removes all bootup scripts for the domain.
 sub feature_delete
 {
-local ($d) = @_;
+my ($d) = @_;
 &$virtual_server::first_print($text{'feat_delete'});
-local $c = 0;
+my $c = 0;
 foreach my $init (&list_domain_actions($d)) {
 	&delete_domain_action($d, $init);
 	$c++;
@@ -128,8 +132,8 @@ sub feature_delete
 # (optional)
 sub feature_webmin
 {
-local ($d, $doms) = @_;
-local @doms = &unique(map { $_->{'id'} } grep { $_->{$module_name} } @$doms);
+my ($d, $doms) = @_;
+my @doms = &unique(map { $_->{'id'} } grep { $_->{$module_name} } @$doms);
 if (@doms) {
 	return ( [ $module_name,
 		   { 'doms' => join(' ', @doms),
@@ -145,7 +149,7 @@ sub feature_webmin
 # Returns HTML for editing limits related to this plugin
 sub feature_limits_input
 {
-local ($d) = @_;
+my ($d) = @_;
 return undef if (!$d->{$module_name});
 return &ui_table_row(&hlink($text{'limits_max'}, "limits_max"),
 	&ui_opt_textbox($input_name."limit", $d->{$module_name."limit"},
@@ -157,7 +161,7 @@ sub feature_limits_input
 # Updates the domain with limit inputs generated by feature_limits_input
 sub feature_limits_parse
 {
-local ($d, $in) = @_;
+my ($d, $in) = @_;
 return undef if (!$d->{$module_name});
 if ($in->{$input_name."limit_def"}) {
 	delete($d->{$module_name."limit"});
@@ -173,7 +177,7 @@ sub feature_limits_parse
 # Returns an array of link objects for webmin modules for this feature
 sub feature_links
 {
-local ($d) = @_;
+my ($d) = @_;
 return ( { 'mod' => $module_name,
 	   'desc' => $text{'links_link'},
 	   'page' => 'index.cgi?dom='.$d->{'id'},
@@ -192,12 +196,14 @@ sub feature_modules
 # Gets all action objects for the domain, and serializes them to the file.
 sub feature_backup
 {
-local ($d, $file) = @_;
+my ($d, $file) = @_;
 &$virtual_server::first_print($text{'feat_backup'});
-local $actions = [ &list_domain_actions($d) ];
+my $actions = [ &list_domain_actions($d) ];
+no strict "subs";
 &virtual_server::open_tempfile_as_domain_user($d, INIT, ">$file") || return 0;
 &print_tempfile(INIT, &serialise_variable($actions));
 &virtual_server::close_tempfile_as_domain_user($d, INIT);
+use strict "subs";
 if (@$actions) {
 	&$virtual_server::second_print($virtual_server::text{'setup_done'});
 	}
@@ -214,11 +220,11 @@ sub feature_backup
 # re-creates them.
 sub feature_restore
 {
-local ($d, $file) = @_;
-local $data = &read_file_contents($file);
+my ($d, $file) = @_;
+my $data = &read_file_contents($file);
 if ($data) {
 	&$virtual_server::first_print($text{'feat_restore'});
-	local $actions = &unserialise_variable($data);
+	my $actions = &unserialise_variable($data);
 	foreach my $init (&list_domain_actions($d)) {
 		&delete_domain_action($d, $init);
 		}
@@ -238,4 +244,3 @@ sub feature_backup_name
 }
 
 1;
-
diff --git a/virtualmin-init-lib.pl b/virtualmin-init-lib.pl
index 40e760b..a4f2236 100755
--- a/virtualmin-init-lib.pl
+++ b/virtualmin-init-lib.pl
@@ -1,11 +1,18 @@
 # Functions for domain-level init scripts
+use strict;
+use warnings;
+our (%text, %config);
+our $module_name;
+our $module_config_directory;
+our $module_root_directory;
+our $remote_user;
 
 BEGIN { push(@INC, ".."); };
 eval "use WebminCore;";
 &init_config();
 &foreign_require("virtual-server", "virtual-server-lib.pl");
-%access = &get_module_acl();
-$action_templates_dir = "$module_config_directory/templates";
+our %access = &get_module_acl();
+my $action_templates_dir = "$module_config_directory/templates";
 
 # virtualmin_init_check()
 # Returns an error if some required config is missing
@@ -38,8 +45,8 @@ sub can_start_actions
 # are determined from the script prefix.
 sub list_domain_actions
 {
-local ($d) = @_;
-local @rv;
+my ($d) = @_;
+my @rv;
 if ($config{'mode'} eq 'init') {
 	# Use init scripts
 	&foreign_require("init", "init-lib.pl");
@@ -47,13 +54,13 @@ sub list_domain_actions
 		$a =~ s/\s+\d+$//;
 		if ($a =~ /^\Q$d->{'dom'}\E_(\S+)$/) {
 			# Found one for the domain
-			local $init = { 'type' => 'init',
+			my $init = { 'type' => 'init',
 					'name' => $1,
 					'id' => $1 };
 			$init->{'desc'} = &init::init_description(
 					    &init::action_filename($a), { });
 			$init->{'status'} = &init::action_status($a) == 2;
-			local $data = &read_file_contents(
+			my $data = &read_file_contents(
 					&init::action_filename($a));
 			($init->{'user'}, $init->{'start'}) =
 				&extract_action_command('start', $data);
@@ -65,16 +72,16 @@ sub list_domain_actions
 	}
 else {
 	# Use SMF. First find virtualmin services, then get their details
-	open(SVCS, "svcs -a |");
-	while(<SVCS>) {
+	open(my $SVCS, "<", "svcs -a |");
+	while(<$SVCS>) {
 		s/\r|\n//g;
-		local ($state, $when, $fmri) = split(/\s+/, $_);
-		local $usdom = &make_fmri_domain($d->{'dom'});
+		my ($state, $when, $fmri) = split(/\s+/, $_);
+		my $usdom = &make_fmri_domain($d->{'dom'});
 		if ($fmri =~ /^svc:\/virtualmin\/\Q$usdom\E\/(.+):default$/ ||
 		    $fmri =~ /^svc:\/virtualmin\/[^\/]+\/\Q$usdom\E\/([^:]+):([^:]+)/) {
 			# Found one for the domain .. get the commands
 			# and user
-			local $init = { 'type' => 'smf',
+			my $init = { 'type' => 'smf',
 					'name' => $2 && $2 ne "default" ? $2 : $1,
 					'fmri' => $fmri,
 					'id' => $fmri,
@@ -95,7 +102,7 @@ sub list_domain_actions
 			push(@rv, $init);
 			}
 		}
-	close(SVCS);
+	close($SVCS);
 	}
 return @rv;
 }
@@ -104,33 +111,37 @@ sub list_domain_actions
 # Creates the init script or SMF service for some new action
 sub create_domain_action
 {
-local ($d, $init, $tmpl, $tparams) = @_;
+my ($d, $init, $tmpl, $tparams) = @_;
 if ($config{'mode'} eq 'init') {
 	# Add init script
 	&foreign_require("init", "init-lib.pl");
-	local $start = &make_action_command('start', $init, $d->{'home'});
-	local $stop = &make_action_command('stop', $init, $d->{'home'});
-	local $init::init_mode = "init";
+	my $start = &make_action_command('start', $init, $d->{'home'});
+	my $stop = &make_action_command('stop', $init, $d->{'home'});
+	no warnings "once";
+	$init::init_mode = "init";
+	use warnings "once";
 	&init::enable_at_boot($d->{'dom'}."_".$init->{'name'},
 			      $init->{'desc'}, $start, $stop);
-	local $if = &init::action_filename($d->{'dom'}."_".$init->{'name'});
-	local $data = &read_file_contents($if);
+	my $if = &init::action_filename($d->{'dom'}."_".$init->{'name'});
+	my $data = &read_file_contents($if);
 	$data =~ s/[ \t]+(VIRTUALMINEOF)/$1/g;	# Remove tab at start
+	no strict "subs";
 	&open_tempfile(INIT, ">$if");
 	&print_tempfile(INIT, $data);
 	&close_tempfile(INIT);
+	use strict "subs";
 	if (!$init->{'status'}) {
 		&init::disable_at_boot($d->{'dom'}."_".$init->{'name'});
 		}
 	}
 else {
 	# Add SMF, by taking XML template and subbing it
-	local $xml = $tmpl->{'xml'} ||
+	my $xml = $tmpl->{'xml'} ||
 		     &read_file_contents(
 			$config{'xml'} ||
 			"$module_root_directory/template.xml");
-	local $usdom = &make_fmri_domain($d->{'dom'});
-	local %hash = ( 'DOM' => $usdom,
+	my $usdom = &make_fmri_domain($d->{'dom'});
+	my %hash = ( 'DOM' => $usdom,
 			'DESC' => $init->{'desc'},
 			'NAME' => $init->{'name'},
 			'START' => join(';', split(/\n/, $init->{'start'})),
@@ -142,11 +153,13 @@ sub create_domain_action
 	$hash{'START'} =~ s/\n*$//g;
 	$hash{'STOP'} =~ s/\n*$//g;
 	$xml = &substitute_template($xml, \%hash);
-	local $temp = &transname();
+	my $temp = &transname();
+	no strict "subs";
 	&open_tempfile(TEMP, ">$temp", 0, 1);
 	&print_tempfile(TEMP, $xml);
 	&close_tempfile(TEMP);
-	local $out = `svccfg -v import $temp 2>&1`;
+	use strict "subs";
+	my $out = `svccfg -v import $temp 2>&1`;
 	if ($? || $out =~ /failed/) {
 		&error("<pre>".&html_escape($out)."</pre>");
 		}
@@ -170,7 +183,7 @@ sub create_domain_action
 # Modifies the init script or SMF service for some action
 sub modify_domain_action
 {
-local ($d, $oldd, $init, $oldinit) = @_;
+my ($d, $oldd, $init, $oldinit) = @_;
 if ($config{'mode'} eq 'init') {
 	# Just delete old init script and re-create
 	&delete_domain_action($oldd, $oldinit);
@@ -180,14 +193,14 @@ sub modify_domain_action
 	# For SMF, if the domain or service name has changed, then the
 	# FMRI may have too ... so we need to export the XML, patch it,
 	# delete the service, then re-create.
-	local $stopped;
+	my $stopped;
 	if ($d->{'dom'} ne $oldd->{'dom'} ||
 	    $d->{'user'} ne $oldd->{'user'} ||
 	    $init->{'name'} ne $oldinit->{'name'}) {
 		# Export XML
-		local $fmri = $oldinit->{'fmri'};
+		my $fmri = $oldinit->{'fmri'};
 		$fmri =~ s/:[^:\/]+$//;
-		local $xml = `svccfg export $fmri`;
+		my $xml = `svccfg export $fmri`;
 		if ($?) {
 			&error("SMF XML export failed : $xml");
 			}
@@ -201,29 +214,32 @@ sub modify_domain_action
 
 		# Replace service name, domain name and user
 		if ($d->{'dom'} ne $oldd->{'dom'}) {
-			local $usdom = &make_fmri_domain($d->{'dom'});
-			local $oldusdom = &make_fmri_domain($oldd->{'dom'});
+			my $usdom = &make_fmri_domain($d->{'dom'});
+			my $oldusdom = &make_fmri_domain($oldd->{'dom'});
 			$xml =~ s/\Q$oldusdom\E/$usdom/g;
 			}
 		if ($d->{'user'} ne $oldd->{'user'}) {
-			local $user = $d->{'user'};
-			local $olduser = $oldd->{'user'};
+			my $user = $d->{'user'};
+			my $olduser = $oldd->{'user'};
 			$xml =~ s/\/\Q$olduser\E\//\/$user\//g;
 			}
 		if ($init->{'name'} ne $oldinit->{'name'}) {
-			local $name = $init->{'name'};
-			local $oldname = $oldinit->{'name'};
+			my $name = $init->{'name'};
+			my $oldname = $oldinit->{'name'};
 			$xml =~ s/\/\Q$oldname\E'/\/$name'/g;
 			$xml =~ s/'\Q$oldname\E'/'$name'/g;
 			}
 
 		# Delete and re-import
-		local $out = `svccfg delete -f $init->{'fmri'} 2>&1`;
-		local $temp = &transname();
+		# XXX This is overwritten before checked.
+		my $out = `svccfg delete -f $init->{'fmri'} 2>&1`;
+		my $temp = &transname();
+		no strict "subs";
 		&open_tempfile(TEMP, ">$temp", 0, 1);
 		&print_tempfile(TEMP, $xml);
 		&close_tempfile(TEMP);
-		local $out = `svccfg -v import $temp 2>&1`;
+		use strict "subs";
+		$out = `svccfg -v import $temp 2>&1`;
 		if ($? || $out =~ /failed/) {
 			&error("SMF XML import failed : $out");
 			}
@@ -263,22 +279,22 @@ sub modify_domain_action
 
 	if ($init->{'status'} == 1 && $oldinit->{'status'} == 0) {
 		# Enable service
-		local $out = `svcadm enable $init->{'fmri'} 2>&1`;
+		my $out = `svcadm enable $init->{'fmri'} 2>&1`;
 		$? && &error("<pre>".&html_escape($out)."</pre>");
 		}
 	elsif ($init->{'status'} == 0 && $oldinit->{'status'} == 1) {
 		# Disable service
-		local $out = `svcadm disable $init->{'fmri'} 2>&1`;
+		my $out = `svcadm disable $init->{'fmri'} 2>&1`;
 		$? && &error("<pre>".&html_escape($out)."</pre>");
 		}
 	elsif ($init->{'status'} == 1 && $oldinit->{'status'} == 2) {
 		# Clear and enable
-		local $out = `svcadm clear $init->{'fmri'} && svcadm enable $init->{'fmri'} 2>&1`;
+		my $out = `svcadm clear $init->{'fmri'} && svcadm enable $init->{'fmri'} 2>&1`;
 		$? && &error("<pre>".&html_escape($out)."</pre>");
 		}
 	elsif ($init->{'status'} == 0 && $oldinit->{'status'} == 2) {
 		# Just clear
-		local $out = `svcadm clear $init->{'fmri'} 2>&1`;
+		my $out = `svcadm clear $init->{'fmri'} 2>&1`;
 		$? && &error("<pre>".&html_escape($out)."</pre>");
 		}
 	}
@@ -288,11 +304,11 @@ sub modify_domain_action
 # Deletes the init script or SMF service for some action
 sub delete_domain_action
 {
-local ($d, $init) = @_;
+my ($d, $init) = @_;
 if ($config{'mode'} eq 'init') {
 	# Delete init script and links
 	&foreign_require("init", "init-lib.pl");
-	local $name = $d->{'dom'}.'_'.$init->{'name'};
+	my $name = $d->{'dom'}.'_'.$init->{'name'};
 	foreach my $l (&init::action_levels('S', $name)) {
 		$l =~ /^(\S+)\s+(\S+)\s+(\S+)$/;
 		&init::delete_rl_action($name, $1, 'S');
@@ -307,15 +323,15 @@ sub delete_domain_action
 	# Delete SMF service
 	&execute_command("svcadm disable $init->{'fmri'}");
 	sleep(2);	# Wait for disable
-	local $out = `svccfg delete -f $init->{'fmri'} 2>&1`;
+	my $out = `svccfg delete -f $init->{'fmri'} 2>&1`;
 	&error("<pre>".&html_escape($out)."</pre>") if ($? || $out =~ /failed/);
 
 	# If there are no more services with the same base fmri (ie. without the
 	# :whatever suffix), delete the base too
 	if ($init->{'fmri'} =~ /^(.*):([^:]+)$/) {
-		local $basefmri = $1;
-		local @others = &list_domain_actions($d);
-		@others = grep { $_->{'fmri'} =~ /^\Q$basefmi\E:/ } @others;
+		my $basefmri = $1;
+		my @others = &list_domain_actions($d);
+		@others = grep { $_->{'fmri'} =~ /^\Q$basefmri\E:/ } @others;
 		if (!@others) {
 			$out = `svccfg delete -f $basefmri 2>&1`;
 			}
@@ -327,25 +343,25 @@ sub delete_domain_action
 # Start some init script, and output the results
 sub start_domain_action
 {
-local ($d, $init) = @_;
+my ($d, $init) = @_;
 if ($config{'mode'} eq 'init') {
 	# Run init script
 	&foreign_require("init", "init-lib.pl");
-	local $cmd = &init::action_filename($d->{'dom'}."_".$init->{'name'});
-	open(OUT, "$cmd start 2>&1 |");
-	while(<OUT>) {
+	my $cmd = &init::action_filename($d->{'dom'}."_".$init->{'name'});
+	open(my $OUT, "<", "$cmd start 2>&1 |");
+	while(<$OUT>) {
 		print &html_escape($_);
 		}
-	close(OUT);
+	close($OUT);
 	}
 else {
 	# Change status to enabled
 	if ($init->{'status'} == 2) {
 		# Clear maintenance mode first
-		local $out = `svcadm clear $init->{'fmri'} 2>&1`;
+		my $out = `svcadm clear $init->{'fmri'} 2>&1`;
 		print &html_escape($out);
 		}
-	local $out = `svcadm enable $init->{'fmri'} 2>&1`;
+	my $out = `svcadm enable $init->{'fmri'} 2>&1`;
 	print &html_escape($out);
 	sleep(5);	# Wait for log
 	print &html_escape(&get_smf_log_tail($init));
@@ -356,20 +372,20 @@ sub start_domain_action
 # Start some init script, and output the results
 sub stop_domain_action
 {
-local ($d, $init) = @_;
+my ($d, $init) = @_;
 if ($config{'mode'} eq 'init') {
 	# Run init script
 	&foreign_require("init", "init-lib.pl");
-	local $cmd = &init::action_filename($d->{'dom'}."_".$init->{'name'});
-	open(OUT, "$cmd stop 2>&1 |");
-	while(<OUT>) {
+	my $cmd = &init::action_filename($d->{'dom'}."_".$init->{'name'});
+	open(my $OUT, "<", "$cmd stop 2>&1 |");
+	while(<$OUT>) {
 		print &html_escape($_);
 		}
-	close(OUT);
+	close($OUT);
 	}
 else {
 	# Change status to disabled
-	local $out = `svcadm disable $init->{'fmri'} 2>&1`;
+	my $out = `svcadm disable $init->{'fmri'} 2>&1`;
 	print &html_escape($out);
 	sleep(5);	# Wait for log
 	print &html_escape(&get_smf_log_tail($init));
@@ -380,14 +396,14 @@ sub stop_domain_action
 # Stop and then start some init script, and output the results
 sub restart_domain_action
 {
-local ($d, $init) = @_;
+my ($d, $init) = @_;
 if ($config{'mode'} eq 'init') {
 	&stop_domain_action($d, $init);
 	&start_domain_action($d, $init);
 	}
 else {
 	# Use SMF's restart feature
-	local $out = `svcadm restart $init->{'fmri'} 2>&1`;
+	my $out = `svcadm restart $init->{'fmri'} 2>&1`;
 	print &html_escape($out);
 	sleep(5);	# Wait for log
 	print &html_escape(&get_smf_log_tail($init));
@@ -398,10 +414,10 @@ sub restart_domain_action
 # Returns the last N (10 by default) lines from an action's SMF log
 sub get_smf_log_tail
 {
-local ($init, $lines) = @_;
+my ($init, $lines) = @_;
 $lines ||= 10;
 if (!$init->{'startlogfile'}) {
-	local $out = `svcs -l $init->{'fmri'}`;
+	my $out = `svcs -l $init->{'fmri'}`;
 	if ($out =~ /logfile\s+(\S+)/) {
 		$init->{'startlogfile'} = $1;
 		}
@@ -416,10 +432,10 @@ sub get_smf_log_tail
 # Returns the number of actions the current user has across all domains
 sub count_user_actions
 {
-local @doms = grep { $_->{$module_name} &&
+my @doms = grep { $_->{$module_name} &&
 		     &virtual_server::can_edit_domain($_) }
 		   &virtual_server::list_domains();
-local $c = 0;
+my $c = 0;
 foreach my $d (@doms) {
 	foreach my $i (&list_domain_actions($d)) {
 		$c++;
@@ -431,13 +447,13 @@ sub count_user_actions
 # extract_action_command(section, script)
 sub extract_action_command
 {
-local ($section, $data) = @_;
+my ($section, $data) = @_;
 if ($data =~ /'\Q$section\E'\)\n([\000-\377]*?);;/) {
 	# Found the section .. get out the su command
-	local $script = $1;
+	my $script = $1;
 	$script =~ s/\s+$//;
 	if ($script =~ /^\s*su\s+\-\s+(\S+)\s*<<'VIRTUALMINEOF'\n\s*cd\s*(\S+)\n([\000-\377]*)VIRTUALMINEOF/) {
-		local @rv = ($1, $3, $2);
+		my @rv = ($1, $3, $2);
 		$rv[1] =~ s/(^|\n)\s*/$1/g;	# strip spaces at start of lines
 		return @rv;
 		}
@@ -452,7 +468,7 @@ sub extract_action_command
 
 sub make_action_command
 {
-local ($section, $init, $dir) = @_;
+my ($section, $init, $dir) = @_;
 if ($init->{$section}) {
 	$init->{$section} =~ /VIRTUALMINEOF/ && &error($text{'save_eeof'});
 	return "su - $init->{'user'} <<'VIRTUALMINEOF'\n".
@@ -467,10 +483,10 @@ sub make_action_command
 
 sub get_smf_prop
 {
-local ($fmri, $name) = @_;
-local $qname = quotemeta($name);
-local $qfmri = quotemeta($fmri);
-local $out = `svcprop -p $qname $qfmri`;
+my ($fmri, $name) = @_;
+my $qname = quotemeta($name);
+my $qfmri = quotemeta($fmri);
+my $out = `svcprop -p $qname $qfmri`;
 $out =~ s/\r|\n//g;
 if ($out eq '""') {
 	# Empty string
@@ -483,7 +499,7 @@ sub get_smf_prop
 # set_smf_prop(fmri, name, value, type)
 sub set_smf_prop
 {
-local ($fmri, $name, $value, $type) = @_;
+my ($fmri, $name, $value, $type) = @_;
 if ($fmri =~ /:default$/ || $name eq "tm_common_name/C") {
 	$fmri =~ s/:[^\/:]+$//;
 	}
@@ -493,9 +509,9 @@ sub set_smf_prop
 	$value =~ s/'/\\'/g;
 	$value = "\"$value\"";
 	}
-local $qfmri = quotemeta($fmri);
-local $qset = quotemeta("setprop $name = $type: $value");
-local $out = `svccfg -s $qfmri $qset 2>&1`;
+my $qfmri = quotemeta($fmri);
+my $qset = quotemeta("setprop $name = $type: $value");
+my $out = `svccfg -s $qfmri $qset 2>&1`;
 if ($? || $out =~ /failed/) {
 	&error("Failed to set SMF property $name to $value : $out");
 	}
@@ -506,18 +522,21 @@ sub set_smf_prop
 # works with SMF.
 sub get_started_processes
 {
-local ($init) = @_;
+my ($init) = @_;
 return ( ) if ($config{'mode'} ne 'smf');
+no strict "subs";
 &open_execute_command(PROCS, "svcs -p ".quotemeta($init->{'fmri'}), 1);
+my @pids;
 while(<PROCS>) {
 	if (/^\s+\S+\s+(\d+)\s+(\S.*)/) {
 		push(@pids, $1);
 		}
 	}
 close(PROCS);
+use strict "subs";
 return ( ) if (!@pids);
 &foreign_require("proc", "proc-lib.pl");
-local %pids = map { $_, 1 } @pids;
+my %pids = map { $_, 1 } @pids;
 return grep { $pids{$_->{'pid'}} } &proc::list_processes();
 }
 
@@ -525,11 +544,11 @@ sub get_started_processes
 # Returns an array of hash refs, each contain the details of one action template
 sub list_action_templates
 {
-local @rv;
+my @rv;
 opendir(DIR, $action_templates_dir) || return ( );
 foreach my $f (readdir(DIR)) {
 	if ($f =~ /^\d+$/) {
-		local %tmpl;
+		my %tmpl;
 		&read_file("$action_templates_dir/$f", \%tmpl);
 		$tmpl{'start'} =~ s/\t/\n/g;
 		$tmpl{'stop'} =~ s/\t/\n/g;
@@ -545,9 +564,9 @@ sub list_action_templates
 # Create or update an action template
 sub save_action_template
 {
-local ($tmpl) = @_;
+my ($tmpl) = @_;
 $tmpl->{'id'} ||= time();
-local %savetmpl = %$tmpl;
+my %savetmpl = %$tmpl;
 $savetmpl{'start'} =~ s/\n/\t/g;
 $savetmpl{'stop'} =~ s/\n/\t/g;
 $savetmpl{'xml'} =~ s/\n/\t/g;
@@ -558,7 +577,7 @@ sub save_action_template
 # delete_action_template(&tmpl)
 sub delete_action_template
 {
-local ($tmpl) = @_;
+my ($tmpl) = @_;
 unlink("$action_templates_dir/$tmpl->{'id'}");
 }
 
@@ -566,7 +585,7 @@ sub delete_action_template
 # Removes _ and leading numbers from a domain name
 sub make_fmri_domain
 {
-local ($usdom) = @_;
+my ($usdom) = @_;
 $usdom =~ s/\./_/g;
 $usdom =~ s/^0/zero/g;
 $usdom =~ s/^1/one/g;
@@ -585,16 +604,16 @@ sub make_fmri_domain
 # Returns an array of option names and values
 sub read_opts_file
 {
-local @rv;
-local $file = $_[0];
+my @rv;
+my $file = $_[0];
 if ($file !~ /^\// && $file !~ /\|\s*$/) {
-	local @uinfo = getpwnam($remote_user);
+	my @uinfo = getpwnam($remote_user);
 	if (@uinfo) {
 		$file = "$uinfo[7]/$file";
 		}
 	}
-open(FILE, $file);
-while(<FILE>) {
+open(my $FILE, "<", $file);
+while(<$FILE>) {
 	s/\r|\n//g;
 	if (/^"([^"]*)"\s+"([^"]*)"$/) {
 		push(@rv, [ $1, $2 ]);
@@ -609,9 +628,8 @@ sub read_opts_file
 		push(@rv, [ $_, $_ ]);
 		}
 	}
-close(FILE);
+close($FILE);
 return @rv;
 }
 
 1;
-