From f162521444148d622df3b42a8304b6cce8f2150e Mon Sep 17 00:00:00 2001 From: "mkanat%bugzilla.org" <> Date: Thu, 6 Jul 2006 13:12:04 +0000 Subject: Bug 173629: Clean up "my" variable scoping issues for mod_perl Patch By Max Kanat-Alexander r=LpSolit, a=myk --- attachment.cgi | 10 +++++++--- buglist.cgi | 4 ++-- chart.cgi | 10 +++++++--- duplicates.cgi | 2 +- editclassifications.cgi | 6 ++++-- editflagtypes.cgi | 15 ++++++++------- editsettings.cgi | 2 +- editusers.cgi | 8 ++++++-- editwhines.cgi | 1 + enter_bug.cgi | 2 +- process_bug.cgi | 43 ++++++++++++++++++++++++++++++------------- query.cgi | 2 +- reports.cgi | 11 +++++++---- request.cgi | 7 ++++--- sanitycheck.cgi | 14 ++++++++++---- showdependencygraph.cgi | 5 +---- token.cgi | 6 +++--- userprefs.cgi | 2 +- votes.cgi | 8 ++++++-- 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 " AS "), 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.

\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{Click here to rebuild the vote cache

\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 -- cgit v1.2.3-24-g4f1b