First let me say that gav^ is 100% correct in his assessment of your problem. You SHOULD use strict, and CGI and warnings. However, if you had they probably still wouldn't have found your bug.

gav^ also said you should use foreach to iterate over arrays and I think he hit the hammer on the head here when it comes to finding your problem.

However, your code suggests that you're not a particularly strong Perl programmer and whilst we'd HOPE that you'd be interested enough to go and read the books I realise that sometimes that doesn't really help you solve the problem you have NOW.

Being in a nice friendly mood, and not a little bored at work, I've spent a little time rewriting some aspects of your script. Read it carefully and test it HEAPS. It compiles, it follows most of gav^'s recommendations (but not the one about CGI - please consider making the changes to do this, it'll make your code much nicer) but I HAVE NOT TESTED IT. I don't have your game files, I can't be bothered working out what they should look like exactly. Even worse, I've made some pretty bold assumptions about what you want your code to be doing in certain circumstances. For example in your first while loop you have the following line:

if ($gmtitle eq $mygametitle) { $gamenumber = $i; }
and I wonder... shouldn't that cause you to step out of the while loop?

So read the code, learn from it, ask questions about it, and test it thoroughly before you upload it to your ISP! Oh, and next time, use strict, warnings and CGI.

I hope this helps.

jarich

#!/usr/bin/perl -w # sets a page's rank and returns a thank you message use strict; use Fcntl ':flock'; # import lock constants. # this should be tidied up by using CGI, Your query string sh +ould # take the form: ?vote=value&gametitle=value&begin=value&end=v +alue&bypass=value # but I suspect that it takes a form more like: # ?10&gametitle&0&10&1 or whatever. :( my $myquery = $ENV{'QUERY_STRING'}; my ($myvote,$mygametitle,$mybegin,$myend,$bypasscache) = split(/\&/,$m +yquery); $mygametitle =~ tr/\+/ /; my $mylink =~ tr/\~/\'/; # Pick the database number we're dealing with. my $dbnumber = ($mybegin == 0 ? 1 : 2); # lock the lockfile (and hope everything plays by the rules) open(FORLOCK,"gmlock.pl") or dienice("Rank set script could not open l +ock file: $!"); flock(FORLOCK, LOCK_EX) or dienice("Rank set script could not gain exc +lusive ". "lock on database: $!"); # open our dbfile open(INF, "gmlist$dbnumber.pl") or dienice("Rank set script could not open proper game databas +e for reading: $!"); # read it all in at once my @ary = <INF>; close(INF); # if mybegin is greater than zero then we'll want to skip some + records... my $skip = 0; if($mybegin > 0) { $skip = $mybegin - 2; } # why -2? $skip = $skip < 0 ? 0 : $skip; # sanity checking, skip should have mi +nimum 0 my ($line, $gamenumber) = ("", -1); # find our game number. foreach (@ary) { $gamenumber++; next if $skip--; # skip this line if we haven't reached + $mybegin. chomp; # $_ is the same as $ary[$gamenumber] +(well, unchomped) $line = $_; my ($gmtitle) = split(/\|/,$line); if ($gmtitle eq $mygametitle) { last; # we've found out gamenumber so end. } } # Pull stuff out from our line my ($gmtitle,$gmdescription,$gmurl,$totalvalue,$totalvotes,$averagevot +e, $clicks,$visibility,$siteid,$timesince,$lastip) = split(/\|/,$line +); # calculate new values my $myavg; if ($lastip ne $ENV{'REMOTE_ADDR'}) { $totalvalue = $totalvalue + $myvote; $totalvotes++; $averagevote = $totalvalue / $totalvotes; $myavg = $averagevote; } $lastip = $ENV{'REMOTE_ADDR'}; my $aflag = 0; # This flags whether we're switching databases # Delete the line for gamenumber from the first file if (($dbnumber == 1) && ($totalvotes > 2)) { $ary[$gamenumber] = ""; # This line position is now empty open(OUTF,">gmlist$dbnumber.pl") or dienice("Rank set script could not re-open proper database for + writing: $!"); print OUTF @ary; # everything except line with gamenumb +er close(OUTF); $dbnumber = 2; open(INF2,"gmlist$dbnumber.pl") or dienice("Rank set script could not open proper game database f +or reading: $!");; @ary = <INF2>; close(INF2); # our new @ary. $gamenumber = @ary; # the last position $aflag = 1; } # create our new gamennumber line. $ary[$gamenumber] = "$gmtitle|$gmdescription|$gmurl|$totalvalue|$total +votes|". "$averagevote|$clicks|$visibility|$siteid|$timesince|$ +lastip\n"; # if the game is switching databases and this isn't the first +entry in this db # OR if we're not switching databases but our vote is greater +than average and # this is not the first entry in this db then ... if(($aflag ==1 && $gamenumber != 0) || ($aflag != 1 && $myvote >= $mya +vg && $gamenumber != 0)) { my $i = $gamenumber; # find the correct place to insert this line. while ($i > 0) { $i--; $line = $ary[$i]; chomp($line); ($gmtitle,$gmdescription,$gmurl,$totalvalue,$totalvotes, $averagevote,$clicks,$visibility,$siteid,$timesince,$last +ip) = split(/\|/,$line); # if our vote passed in through the GET string is >= t +he # calculated average vote for this game... if ($myavg >= $averagevote) { # swap these two. my $holder = $ary[$i]; $ary[$i] = $ary[$gamenumber]; $ary[$gamenumber] = $holder; $gamenumber--; } else { # It must be in the correct position now. last; } } } else { my $i = $gamenumber; # find the correct place to insert this line. while ($i < @ary) { $i++; $line = $ary[$i]; chomp($line); ($gmtitle,$gmdescription,$gmurl,$totalvalue,$totalvotes,$a +veragevote, $clicks,$visibility,$siteid,$timesince,$lastip) = split( +/\|/,$line); # if our vote passed in through the GET string is < th +e # calculated average vote for this game... if ($myavg < $averagevote) { # swap these two. my $holder = $ary[$i]; $ary[$i] = $ary[$gamenumber]; $ary[$gamenumber] = $holder; $gamenumber++; } else { # it must be in the correct position now. last; } } } open(OUTF2,">gmlist$dbnumber.pl") or dienice("Rank set script could not re-open proper database for + writing: $!"); print OUTF2 @ary; close(OUTF2); # release the lock close(FORLOCK); # and all the printing out code goes below here

In reply to Re: Why Did This Stall the Server by jarich
in thread Why Did This Stall the Server by Anonymous Monk

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.