Closed
Bug 173629
Opened 22 years ago
Closed 18 years ago
Clean up "my" variable scoping issues for mod_perl
Categories
(Bugzilla :: Bugzilla-General, defect, P1)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: bbaetz, Assigned: mkanat)
References
()
Details
Attachments
(1 file, 6 obsolete files)
21.92 KB,
patch
|
Details | Diff | Splinter Review |
The following script will not work correctly under mod_perl: my $foo; sub bar { $foo = $foo + 42; print $foo; } See the URL for why. We need to fix this before we can use mod_perl in bugzilla.
Comment 1•21 years ago
|
||
For the record, this does generate a compile-time warning when the file is first loaded, if you try to run it under mod_perl, so that'll be the easiest way to fix this is just make things run under mod_perl and chase the warnings :) [Tue Nov 18 02:20:33 2003] process_bug.cgi: Variable "$UserInEditGroupSet" will not stay shared at /var/www/html/bugzilla/process_bug.cgi line 764. etc.
Updated•20 years ago
|
Summary: Clean up scripts for mod_perl → Clean up "my" variable scoping issues for mod_perl
Assignee | ||
Comment 2•20 years ago
|
||
For the record, I'm pretty sure that the code should instead be: our $foo = 0; sub bar { $foo = $foo + 42; print $foo; } Note that the "= 0" part is important.
Comment 3•19 years ago
|
||
Reassigning bugs that I'm not actively working on to the default component owner in order to try to make some sanity out of my personal buglist. This doesn't mean the bug isn't being dealt with, just that I'm not the one doing it. If you are dealing with this bug, please assign it to yourself.
Assignee: justdave → general
QA Contact: mattyt-bugzilla → default-qa
Comment 4•19 years ago
|
||
(In reply to comment #2) > For the record, I'm pretty sure that the code should instead be: > > our $foo = 0; > Note that the "= 0" part is important. Looks correct. See also the comment following the multirun1.pl script, reported below: http://perl.apache.org/docs/general/perl_reference/perl_reference.html#Remedies_for_Inner_Subroutines "To avoid this you can put _local_ in front of the _our_ declaration of all variables other than simple scalars. This has the effect of restoring the variable to its previous value (usually undefined) upon exit from the current scope. As a side-effect _local_ also initializes the variables to _undef_. So, if you recall that thing I said about adding explicit initialization when you replace _my_ by _our_, well, you can forget it again if you replace _my_ with _local our_."
Assignee | ||
Comment 5•18 years ago
|
||
As far as I know, this should fix *most* of the places where the "my" thing is a problem. It might even fix them all. If we have any remaining after this patch, we can start to file bugs for individual problems.
Assignee | ||
Updated•18 years ago
|
Attachment #226642 -
Flags: review? → review?(wicked+bz)
Assignee | ||
Updated•18 years ago
|
OS: Linux → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → Bugzilla 2.24
Version: unspecified → 2.23
Assignee | ||
Comment 6•18 years ago
|
||
I just read the mod_perl docs again, and they point out that "our" variables stick around until the next page call, which means that we'd be wasting memory with them unless we made them "local our" which causes them to die at the end of the CGI.
Attachment #226642 -
Attachment is obsolete: true
Attachment #226645 -
Flags: review?(wicked+bz)
Attachment #226642 -
Flags: review?(wicked+bz)
Comment 7•18 years ago
|
||
Comment on attachment 226645 [details] [diff] [review] v2 wrong patch!
Attachment #226645 -
Attachment is obsolete: true
Attachment #226645 -
Flags: review?(wicked+bz)
Assignee | ||
Comment 8•18 years ago
|
||
Oh, you're right! Here's the right patch.
Attachment #226902 -
Flags: review?(LpSolit)
Assignee | ||
Comment 9•18 years ago
|
||
I found a few others that I had missed, by clicking around on my mod_perl installation.
Attachment #226902 -
Attachment is obsolete: true
Attachment #227239 -
Flags: review?(LpSolit)
Attachment #226902 -
Flags: review?(LpSolit)
Assignee | ||
Comment 10•18 years ago
|
||
There was a typo in sanitycheck.cgi. I fixed it.
Attachment #227239 -
Attachment is obsolete: true
Attachment #227824 -
Flags: review?(LpSolit)
Attachment #227239 -
Flags: review?(LpSolit)
Comment 11•18 years ago
|
||
Comment on attachment 227824 [details] [diff] [review] v3.1 The part for process_bug.cgi is completely bitrotten. CheckCanChangeField is no longer in process_bug.cgi. 5 out of 12 hunks FAILED -- saving rejects to file process_bug.cgi.rej
Attachment #227824 -
Flags: review?(LpSolit) → review-
Assignee | ||
Comment 12•18 years ago
|
||
Okay, I fixed the bitrot. I also noticed a few more places in process_bug.cgi that I forgot to fix, and I fixed them.
Attachment #227824 -
Attachment is obsolete: true
Attachment #227844 -
Flags: review?(LpSolit)
Comment 13•18 years ago
|
||
Comment on attachment 227844 [details] [diff] [review] v4 >Index: editusers.cgi > sub edit_processing { >- $editusers || $user->can_see_user($otherUser) >+ user->in_group('editusers') || $user->can_see_user($otherUser) Must be $user->in_group, not user->in_group. Else it looks good. I did a few tests on a non mod_perl installation and that's the only problem I found. Must be fixed on checkin. r=LpSolit
Attachment #227844 -
Flags: review?(LpSolit) → review+
Updated•18 years ago
|
Flags: approval?
Updated•18 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 14•18 years ago
|
||
Okay, I did the checkin fix and un-bitrotted it. Checking in attachment.cgi; /cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v <-- attachment.cgi new revision: 1.116; previous revision: 1.115 done Checking in buglist.cgi; /cvsroot/mozilla/webtools/bugzilla/buglist.cgi,v <-- buglist.cgi new revision: 1.338; previous revision: 1.337 done Checking in chart.cgi; /cvsroot/mozilla/webtools/bugzilla/chart.cgi,v <-- chart.cgi new revision: 1.21; previous revision: 1.20 done Checking in duplicates.cgi; /cvsroot/mozilla/webtools/bugzilla/duplicates.cgi,v <-- duplicates.cgi new revision: 1.59; previous revision: 1.58 done Checking in editclassifications.cgi; /cvsroot/mozilla/webtools/bugzilla/editclassifications.cgi,v <-- editclassifications.cgi new revision: 1.23; previous revision: 1.22 done Checking in editflagtypes.cgi; /cvsroot/mozilla/webtools/bugzilla/editflagtypes.cgi,v <-- editflagtypes.cgi new revision: 1.40; previous revision: 1.39 done Checking in editsettings.cgi; /cvsroot/mozilla/webtools/bugzilla/editsettings.cgi,v <-- editsettings.cgi new revision: 1.8; previous revision: 1.7 done Checking in editusers.cgi; /cvsroot/mozilla/webtools/bugzilla/editusers.cgi,v <-- editusers.cgi new revision: 1.125; previous revision: 1.124 done Checking in editwhines.cgi; /cvsroot/mozilla/webtools/bugzilla/editwhines.cgi,v <-- editwhines.cgi new revision: 1.18; previous revision: 1.17 done Checking in enter_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/enter_bug.cgi,v <-- enter_bug.cgi new revision: 1.140; previous revision: 1.139 done Checking in process_bug.cgi; /cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v <-- process_bug.cgi new revision: 1.334; previous revision: 1.333 done Checking in query.cgi; /cvsroot/mozilla/webtools/bugzilla/query.cgi,v <-- query.cgi new revision: 1.168; previous revision: 1.167 done Checking in reports.cgi; /cvsroot/mozilla/webtools/bugzilla/reports.cgi,v <-- reports.cgi new revision: 1.86; previous revision: 1.85 done Checking in request.cgi; /cvsroot/mozilla/webtools/bugzilla/request.cgi,v <-- request.cgi new revision: 1.36; previous revision: 1.35 done Checking in sanitycheck.cgi; /cvsroot/mozilla/webtools/bugzilla/sanitycheck.cgi,v <-- sanitycheck.cgi new revision: 1.118; previous revision: 1.117 done Checking in showdependencygraph.cgi; /cvsroot/mozilla/webtools/bugzilla/showdependencygraph.cgi,v <-- showdependencygraph.cgi new revision: 1.53; previous revision: 1.52 done Checking in token.cgi; /cvsroot/mozilla/webtools/bugzilla/token.cgi,v <-- token.cgi new revision: 1.44; previous revision: 1.43 done Checking in userprefs.cgi; /cvsroot/mozilla/webtools/bugzilla/userprefs.cgi,v <-- userprefs.cgi new revision: 1.102; previous revision: 1.101 done Checking in votes.cgi; /cvsroot/mozilla/webtools/bugzilla/votes.cgi,v <-- votes.cgi new revision: 1.45; previous revision: 1.44 done
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 15•18 years ago
|
||
Here's the un-bitrotted and fixed version that I checked in.
Attachment #227844 -
Attachment is obsolete: true
Assignee | ||
Comment 16•17 years ago
|
||
Added to the release notes as part of bug 349423.
Keywords: relnote
You need to log in
before you can comment on or make changes to this bug.
Description
•