Bug 173629: Clean up "my" variable scoping issues for mod_perl

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=myk

19 files changed, 101 insertions, 57 deletions

diff --git a/attachment.cgi b/attachment.cgi
index fcbd10020..94f4fbe5c 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -52,9 +52,13 @@ use Bugzilla::Token;
 
 Bugzilla->login();
 
-my $cgi = Bugzilla->cgi;
-my $template = Bugzilla->template;
-my $vars = {};
+# For most scripts we don't make $cgi and $template global variables. But
+# when preparing Bugzilla for mod_perl, this script used these
+# variables in so many subroutines that it was easier to just
+# make them globals.
+local our $cgi = Bugzilla->cgi;
+local our $template = Bugzilla->template;
+local our $vars = {};
 
 ################################################################################
 # Main Body Execution
diff --git a/buglist.cgi b/buglist.cgi
index 937749d28..1a7ffc316 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -186,7 +186,7 @@ foreach my $chart (@charts) {
 # Utilities
 ################################################################################
 
-my @weekday= qw( Sun Mon Tue Wed Thu Fri Sat );
+local our @weekday= qw( Sun Mon Tue Wed Thu Fri Sat );
 sub DiffDate {
     my ($datestr) = @_;
     my $date = str2time($datestr);
@@ -520,7 +520,7 @@ if (!$params->param('query_format')) {
 # Note: For column names using aliasing (SQL "<field> AS <alias>"), the column
 #       ID needs to be identical to the field ID for list ordering to work.
 
-my $columns = {};
+local our $columns = {};
 sub DefineColumn {
     my ($id, $name, $title) = @_;
     $columns->{$id} = { 'name' => $name , 'title' => $title };
diff --git a/chart.cgi b/chart.cgi
index fd7025855..680eeb7b5 100755
--- a/chart.cgi
+++ b/chart.cgi
@@ -53,9 +53,13 @@ use Bugzilla::Chart;
 use Bugzilla::Series;
 use Bugzilla::User;
 
-my $cgi = Bugzilla->cgi;
-my $template = Bugzilla->template;
-my $vars = {};
+# For most scripts we don't make $cgi and $template global variables. But
+# when preparing Bugzilla for mod_perl, this script used these
+# variables in so many subroutines that it was easier to just
+# make them globals.
+local our $cgi = Bugzilla->cgi;
+local our $template = Bugzilla->template;
+local our $vars = {};
 
 # Go back to query.cgi if we are adding a boolean chart parameter.
 if (grep(/^cmd-/, $cgi->param())) {
diff --git a/duplicates.cgi b/duplicates.cgi
index 7934c5c8f..94b35df97 100755
--- a/duplicates.cgi
+++ b/duplicates.cgi
@@ -72,7 +72,7 @@ my %before;
 # Get params from URL
 sub formvalue {
     my ($name, $default) = (@_);
-    return $cgi->param($name) || $default || "";
+    return Bugzilla->cgi->param($name) || $default || "";
 }
 
 my $sortby = formvalue("sortby");
diff --git a/editclassifications.cgi b/editclassifications.cgi
index 70aa256fb..706d68918 100755
--- a/editclassifications.cgi
+++ b/editclassifications.cgi
@@ -29,13 +29,15 @@ use Bugzilla::Util;
 use Bugzilla::Error;
 use Bugzilla::Classification;
 
-my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
+my $cgi = Bugzilla->cgi;
 my $template = Bugzilla->template;
-my $vars = {};
+local our $vars = {};
 
 sub LoadTemplate {
     my $action = shift;
+    my $cgi = Bugzilla->cgi;
+    my $template = Bugzilla->template;
 
     $action =~ /(\w+)/;
     $action = $1;
diff --git a/editflagtypes.cgi b/editflagtypes.cgi
index 825fa1eb5..61dafa923 100755
--- a/editflagtypes.cgi
+++ b/editflagtypes.cgi
@@ -44,8 +44,9 @@ use Bugzilla::Attachment;
 
 use List::Util qw(reduce);
 
-my $template = Bugzilla->template;
-my $vars = {};
+local our $cgi = Bugzilla->cgi;
+local our $template = Bugzilla->template;
+local our $vars = {};
 
 # Make sure the user is logged in and is an administrator.
 my $user = Bugzilla->login(LOGIN_REQUIRED);
@@ -54,8 +55,6 @@ $user->in_group('editcomponents')
                                      action => "edit",
                                      object => "flagtypes"});
 
-my $cgi = Bugzilla->cgi;
-
 ################################################################################
 # Main Body Execution
 ################################################################################
@@ -75,9 +74,9 @@ if (@categoryActions = grep(/^categoryAction-.+/, $cgi->param())) {
 }
 
 if    ($action eq 'list')           { list();           }
-elsif ($action eq 'enter')          { edit();           }
-elsif ($action eq 'copy')           { edit();           }
-elsif ($action eq 'edit')           { edit();           }
+elsif ($action eq 'enter')          { edit($action);    }
+elsif ($action eq 'copy')           { edit($action);    }
+elsif ($action eq 'edit')           { edit($action);    }
 elsif ($action eq 'insert')         { insert();         }
 elsif ($action eq 'update')         { update();         }
 elsif ($action eq 'confirmdelete')  { confirmDelete();  } 
@@ -167,6 +166,7 @@ sub list {
 
 
 sub edit {
+    my ($action) = @_;
     $action eq 'enter' ? validateTargetType() : (my $id = validateID());
     my $dbh = Bugzilla->dbh;
 
@@ -365,6 +365,7 @@ sub update {
     validateGroups();
 
     my $dbh = Bugzilla->dbh;
+    my $user = Bugzilla->user;
     $dbh->bz_lock_tables('flagtypes WRITE', 'products READ',
                          'components READ', 'flaginclusions WRITE',
                          'flagexclusions WRITE');
diff --git a/editsettings.cgi b/editsettings.cgi
index 85332a39a..6d7fffdfa 100755
--- a/editsettings.cgi
+++ b/editsettings.cgi
@@ -26,7 +26,7 @@ use Bugzilla::Error;
 use Bugzilla::User::Setting;
 
 my $template = Bugzilla->template;
-my $vars = {};
+local our $vars = {};
 
 ###############################
 ###  Subroutine Definitions ###
diff --git a/editusers.cgi b/editusers.cgi
index 86e2cf424..facb46600 100755
--- a/editusers.cgi
+++ b/editusers.cgi
@@ -37,10 +37,10 @@ my $user = Bugzilla->login(LOGIN_REQUIRED);
 
 my $cgi       = Bugzilla->cgi;
 my $template  = Bugzilla->template;
-my $vars      = {};
 my $dbh       = Bugzilla->dbh;
 my $userid    = $user->id;
 my $editusers = $user->in_group('editusers');
+local our $vars     = {};
 
 # Reject access if there is no sense in continuing.
 $editusers
@@ -778,6 +778,7 @@ sub check_user {
 
 # Copy incoming list selection values from CGI params to template variables.
 sub mirrorListSelectionValues {
+    my $cgi = Bugzilla->cgi;
     if (defined($cgi->param('matchtype'))) {
         foreach ('matchvalue', 'matchstr', 'matchtype', 'grouprestrict', 'groupid') {
             $vars->{'listselectionvalues'}{$_} = $cgi->param($_);
@@ -791,6 +792,7 @@ sub userDataToVars {
     my $otheruserid = shift;
     my $otheruser = new Bugzilla::User($otheruserid);
     my $query;
+    my $user = Bugzilla->user;
     my $dbh = Bugzilla->dbh;
 
     my $grouplist = $otheruser->groups_as_string;
@@ -846,8 +848,10 @@ sub userDataToVars {
 
 sub edit_processing {
     my $otherUser = shift;
+    my $user = Bugzilla->user;
+    my $template = Bugzilla->template;
 
-    $editusers || $user->can_see_user($otherUser)
+    $user->in_group('editusers') || $user->can_see_user($otherUser)
         || ThrowUserError('auth_failure', {reason => "not_visible",
                                            action => "modify",
                                            object => "user"});
diff --git a/editwhines.cgi b/editwhines.cgi
index f0fddeb5b..0aed19b7a 100755
--- a/editwhines.cgi
+++ b/editwhines.cgi
@@ -444,6 +444,7 @@ $template->process("whine/schedule.html.tmpl", $vars)
 # the subject and body of each event that user owns
 sub get_events {
     my $userid = shift;
+    my $dbh = Bugzilla->dbh;
     my $events = {};
 
     my $sth = $dbh->prepare("SELECT DISTINCT id, subject, body " .
diff --git a/enter_bug.cgi b/enter_bug.cgi
index e5463b501..1315ca927 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -145,7 +145,7 @@ $user->can_enter_product($product ? $product->name : $product_name, THROW_ERROR)
 ##############################################################################
 sub formvalue {
     my ($name, $default) = (@_);
-    return $cgi->param($name) || $default || "";
+    return Bugzilla->cgi->param($name) || $default || "";
 }
 
 # Takes the name of a field and a list of possible values for that 
diff --git a/process_bug.cgi b/process_bug.cgi
index fa34ae709..b1a53a3da 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -60,13 +60,13 @@ use Bugzilla::Flag;
 use Bugzilla::FlagType;
 
 my $user = Bugzilla->login(LOGIN_REQUIRED);
-my $whoid = $user->id;
+local our $whoid = $user->id;
 my $grouplist = $user->groups_as_string;
 
 my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
 my $template = Bugzilla->template;
-my $vars = {};
+local our $vars = {};
 $vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count();
 
 my @editable_bug_fields = editable_bug_fields();
@@ -269,6 +269,7 @@ foreach my $field_name ('product', 'component', 'version') {
 #
 sub CheckonComment {
     my ($function) = (@_);
+    my $cgi = Bugzilla->cgi;
     
     # Param is 1 if comment should be added !
     my $ret = Bugzilla->params->{ "commenton" . $function };
@@ -413,6 +414,10 @@ if (((defined $cgi->param('id') && $cgi->param('product') ne $oldproduct)
 
 # Confirm that the reporter of the current bug can access the bug we are duping to.
 sub DuplicateUserConfirm {
+    my $cgi = Bugzilla->cgi;
+    my $dbh = Bugzilla->dbh;
+    my $template = Bugzilla->template;
+
     # if we've already been through here, then exit
     if (defined $cgi->param('confirm_add_duplicate')) {
         return;
@@ -586,10 +591,11 @@ if ($action eq Bugzilla->params->{'move-button-text'}) {
 
 $::query = "UPDATE bugs SET";
 $::comma = "";
-my @values;
+local our @values;
 umask(0);
 
 sub _remove_remaining_time {
+    my $cgi = Bugzilla->cgi;
     if (UserInGroup(Bugzilla->params->{'timetrackinggroup'})) {
         if ( defined $cgi->param('remaining_time') 
              && $cgi->param('remaining_time') > 0 )
@@ -611,9 +617,12 @@ sub DoComma {
 
 # $everconfirmed is used by ChangeStatus() to determine whether we are
 # confirming the bug or not.
-my $everconfirmed;
+local our $everconfirmed;
 sub DoConfirm {
-    if ($bug->check_can_change_field("canconfirm", 0, 1, \$PrivilegesRequired)) {
+    my $bug = shift;
+    if ($bug->check_can_change_field("canconfirm", 0, 1, 
+                                     \$PrivilegesRequired)) 
+    {
         DoComma();
         $::query .= "everconfirmed = 1";
         $everconfirmed = 1;
@@ -622,6 +631,9 @@ sub DoConfirm {
 
 sub ChangeStatus {
     my ($str) = (@_);
+    my $cgi = Bugzilla->cgi;
+    my $dbh = Bugzilla->dbh;
+
     if (!$cgi->param('dontchange')
         || $str ne $cgi->param('dontchange')) {
         DoComma();
@@ -683,6 +695,9 @@ sub ChangeStatus {
 
 sub ChangeResolution {
     my ($str) = (@_);
+    my $dbh = Bugzilla->dbh;
+    my $cgi = Bugzilla->cgi;
+
     if (!$cgi->param('dontchange')
         || $str ne $cgi->param('dontchange'))
     {
@@ -986,12 +1001,12 @@ SWITCH: for ($cgi->param('knob')) {
         last SWITCH;
     };
     /^confirm$/ && CheckonComment( "confirm" ) && do {
-        DoConfirm();
+        DoConfirm($bug);
         ChangeStatus('NEW');
         last SWITCH;
     };
     /^accept$/ && CheckonComment( "accept" ) && do {
-        DoConfirm();
+        DoConfirm($bug);
         ChangeStatus('ASSIGNED');
         if (Bugzilla->params->{"usetargetmilestone"} 
             && Bugzilla->params->{"musthavemilestoneonaccept"}) 
@@ -1034,7 +1049,7 @@ SWITCH: for ($cgi->param('knob')) {
     };
     /^reassign$/ && CheckonComment( "reassign" ) && do {
         if ($cgi->param('andconfirm')) {
-            DoConfirm();
+            DoConfirm($bug);
         }
         ChangeStatus('NEW');
         DoComma();
@@ -1066,7 +1081,7 @@ SWITCH: for ($cgi->param('knob')) {
     };
     /^reassignbycomponent$/  && CheckonComment( "reassignbycomponent" ) && do {
         if ($cgi->param('compconfirm')) {
-            DoConfirm();
+            DoConfirm($bug);
         }
         ChangeStatus('NEW');
         last SWITCH;
@@ -1210,13 +1225,13 @@ if (UserInGroup(Bugzilla->params->{'timetrackinggroup'})) {
 }
 
 my $basequery = $::query;
-my $delta_ts;
-
 
+local our $delta_ts;
 sub SnapShotBug {
     my ($id) = (@_);
+    my $dbh = Bugzilla->dbh;
     my @row = $dbh->selectrow_array(q{SELECT delta_ts, } .
-                join(',', @editable_bug_fields).q{ FROM bugs WHERE bug_id = ?},
+                join(',', editable_bug_fields()).q{ FROM bugs WHERE bug_id = ?},
                 undef, $id);
     $delta_ts = shift @row;
 
@@ -1226,6 +1241,7 @@ sub SnapShotBug {
 
 sub SnapShotDeps {
     my ($i, $target, $me) = (@_);
+    my $dbh = Bugzilla->dbh;
     my $list = $dbh->selectcol_arrayref(qq{SELECT $target FROM dependencies
                                         WHERE $me = ? ORDER BY $target},
                                         undef, $i);
@@ -1234,10 +1250,11 @@ sub SnapShotDeps {
 
 
 my $timestamp;
-my $bug_changed;
 
+local our $bug_changed;
 sub LogDependencyActivity {
     my ($i, $oldstr, $target, $me, $timestamp) = (@_);
+    my $dbh = Bugzilla->dbh;
     my $newstr = SnapShotDeps($i, $target, $me);
     if ($oldstr ne $newstr) {
         # Figure out what's really different...
diff --git a/query.cgi b/query.cgi
index 9c2067d3e..57f965377 100755
--- a/query.cgi
+++ b/query.cgi
@@ -103,7 +103,7 @@ if ($userid) {
          undef, ($userid, DEFAULT_QUERY_NAME));
 }
 
-my %default;
+local our %default;
 
 # We pass the defaults as a hash of references to arrays. For those
 # Items which are single-valued, the template should only reference [0]
diff --git a/reports.cgi b/reports.cgi
index 6c7a4ea39..9df5824d7 100755
--- a/reports.cgi
+++ b/reports.cgi
@@ -47,8 +47,8 @@ $@ && ThrowCodeError("gd_not_installed");
 eval "use Chart::Lines";
 $@ && ThrowCodeError("chart_lines_not_installed");
 
-my $dir = bz_locations()->{'datadir'} . "/mining";
-my $graph_dir = "graphs";
+local our $dir = bz_locations()->{'datadir'} . "/mining";
+local our $graph_dir = bz_locations()->{'libpath'} . "/graphs";
 
 # If we're using bug groups for products, we should apply those restrictions
 # to viewing reports, as well.  Time to check the login in that case.
@@ -58,7 +58,7 @@ Bugzilla->switch_to_shadow_db();
 
 my $cgi = Bugzilla->cgi;
 my $template = Bugzilla->template;
-my $vars = {};
+local our $vars = {};
 
 # We only want those products that the user has permissions for.
 my @myproducts;
@@ -101,7 +101,9 @@ if (! defined $cgi->param('product')) {
 
 sub choose_product {
     my @myproducts = (@_);
-    
+    my $cgi = Bugzilla->cgi;
+    my $template = Bugzilla->template;
+
     my $datafile = daily_stats_filename('-All-');
 
     # Can we do bug charts?  
@@ -182,6 +184,7 @@ sub daily_stats_filename {
 
 sub show_chart {
     my ($product) = @_;
+    my $cgi = Bugzilla->cgi;
 
     if (! defined $cgi->param('datasets')) {
         ThrowUserError("missing_datasets", $vars);
diff --git a/request.cgi b/request.cgi
index 3057eebd6..aee9c0387 100755
--- a/request.cgi
+++ b/request.cgi
@@ -41,11 +41,9 @@ use Bugzilla::Component;
 
 # Make sure the user is logged in.
 my $user = Bugzilla->login();
-my $userid = $user->id;
 
 my $cgi = Bugzilla->cgi;
-my $template = Bugzilla->template;
-my $vars = {};
+local our $vars = {};
 
 
 ################################################################################
@@ -74,6 +72,9 @@ exit;
 sub queue {
     my $cgi = Bugzilla->cgi;
     my $dbh = Bugzilla->dbh;
+    my $template = Bugzilla->template;
+    my $user = Bugzilla->user;
+    my $userid = $user->id;
 
     my $status = validateStatus($cgi->param('status'));
     my $form_group = validateGroup($cgi->param('group'));
diff --git a/sanitycheck.cgi b/sanitycheck.cgi
index f0f794194..cfe5e1fe3 100755
--- a/sanitycheck.cgi
+++ b/sanitycheck.cgi
@@ -333,6 +333,7 @@ print "OK, now running sanity checks.<p>\n";
 sub CrossCheck {
     my $table = shift @_;
     my $field = shift @_;
+    my $dbh = Bugzilla->dbh;
 
     Status("Checking references to $table.$field");
 
@@ -509,6 +510,7 @@ sub DoubleCrossCheck {
     my $table = shift @_;
     my $field1 = shift @_;
     my $field2 = shift @_;
+    my $dbh = Bugzilla->dbh;
  
     Status("Checking references to $table.$field1 / $table.$field2");
  
@@ -582,12 +584,9 @@ while (my ($id, $email) = $sth->fetchrow_array) {
 # Perform vote/keyword cache checks
 ###########################################################################
 
-my $offervotecacherebuild = 0;
-
 sub AlertBadVoteCache {
     my ($id) = (@_);
     Alert("Bad vote cache for bug " . BugLink($id));
-    $offervotecacherebuild = 1;
 }
 
 $sth = $dbh->prepare(q{SELECT bug_id, votes, keywords
@@ -615,21 +614,25 @@ $sth = $dbh->prepare(q{SELECT bug_id, SUM(vote_count)
                        $dbh->sql_group_by('bug_id'));
 $sth->execute;
 
+my $offer_votecache_rebuild = 0;
+
 while (my ($id, $v) = $sth->fetchrow_array) {
     if ($v <= 0) {
         Alert("Bad vote sum for bug $id");
     } else {
         if (!defined $votes{$id} || $votes{$id} != $v) {
             AlertBadVoteCache($id);
+            $offer_votecache_rebuild = 1;
         }
         delete $votes{$id};
     }
 }
 foreach my $id (keys %votes) {
     AlertBadVoteCache($id);
+    $offer_votecache_rebuild = 1;
 }
 
-if ($offervotecacherebuild) {
+if ($offer_votecache_rebuild) {
     print qq{<a href="sanitycheck.cgi?rebuildvotecache=1">Click here to rebuild the vote cache</a><p>\n};
 }
 
@@ -752,6 +755,7 @@ if (defined $cgi->param('rebuildkeywordcache')) {
 
 sub BugCheck {
     my ($middlesql, $errortext, $repairparam, $repairtext) = @_;
+    my $dbh = Bugzilla->dbh;
  
     my $badbugs = $dbh->selectcol_arrayref(qq{SELECT DISTINCT bugs.bug_id
                                                 FROM $middlesql 
@@ -817,6 +821,8 @@ BugCheck("bugs INNER JOIN products ON bugs.product_id = products.id " .
 sub DateCheck {
     my $table = shift @_;
     my $field = shift @_;
+    my $dbh = Bugzilla->dbh;
+
     Status("Checking dates in $table.$field");
     my $c = $dbh->selectrow_array(qq{SELECT COUNT($field)
                                        FROM $table
diff --git a/showdependencygraph.cgi b/showdependencygraph.cgi
index c797e2b12..00442c4f3 100755
--- a/showdependencygraph.cgi
+++ b/showdependencygraph.cgi
@@ -42,10 +42,7 @@ my $vars = {};
 # performance.
 my $dbh = Bugzilla->switch_to_shadow_db();
 
-my %seen;
-my %edgesdone;
-my %bugtitles; # html title attributes for imagemap areas
-
+local our (%seen, %edgesdone, %bugtitles);
 
 # CreateImagemap: This sub grabs a local filename as a parameter, reads the 
 # dot-generated image map datafile residing in that file and turns it into
diff --git a/token.cgi b/token.cgi
index dcde8c108..44e456710 100755
--- a/token.cgi
+++ b/token.cgi
@@ -36,10 +36,10 @@ use Bugzilla::Error;
 use Bugzilla::Token;
 use Bugzilla::User;
 
-my $cgi = Bugzilla->cgi;
 my $dbh = Bugzilla->dbh;
-my $template = Bugzilla->template;
-my $vars = {};
+local our $cgi = Bugzilla->cgi;
+local our $template = Bugzilla->template;
+local our $vars = {};
 
 Bugzilla->login(LOGIN_OPTIONAL);
 
diff --git a/userprefs.cgi b/userprefs.cgi
index 5c1a85e32..15ca800b1 100755
--- a/userprefs.cgi
+++ b/userprefs.cgi
@@ -34,7 +34,7 @@ use Bugzilla::Error;
 use Bugzilla::User;
 
 my $template = Bugzilla->template;
-my $vars = {};
+local our $vars = {};
 
 ###############################################################################
 # Each panel has two functions - panel Foo has a DoFoo, to get the data 
diff --git a/votes.cgi b/votes.cgi
index 6f01f6345..016e6ad67 100755
--- a/votes.cgi
+++ b/votes.cgi
@@ -38,8 +38,7 @@ use Bugzilla::Product;
 use List::Util qw(min);
 
 my $cgi = Bugzilla->cgi;
-my $template = Bugzilla->template;
-my $vars = {};
+local our $vars = {};
 
 # If the action is show_bug, you need a bug_id.
 # If the action is show_user, you can supply a userid to show the votes for
@@ -94,6 +93,8 @@ exit;
 sub show_bug {
     my $cgi = Bugzilla->cgi;
     my $dbh = Bugzilla->dbh;
+    my $template = Bugzilla->template;
+    my $bug_id = $cgi->param('bug_id');
 
     ThrowCodeError("missing_bug_id") unless defined $bug_id;
 
@@ -117,6 +118,8 @@ sub show_user {
     my $cgi = Bugzilla->cgi;
     my $dbh = Bugzilla->dbh;
     my $user = Bugzilla->user;
+    my $template = Bugzilla->template;
+    my $bug_id = $cgi->param('bug_id');
 
     # If a bug_id is given, and we're editing, we'll add it to the votes list.
     $bug_id ||= "";
@@ -215,6 +218,7 @@ sub record_votes {
 
     my $cgi = Bugzilla->cgi;
     my $dbh = Bugzilla->dbh;
+    my $template = Bugzilla->template;
 
     # Build a list of bug IDs for which votes have been submitted.  Votes
     # are submitted in form fields in which the field names are the bug