SPUG: Another program head scratcher
Tom Legrady
legrady at earthlink.net
Sat Jul 13 16:37:15 CDT 2002
In line 5, you read the file into an array.
I wonder whether you are using this for a few hundred
visitors or a few million. Reading in a file and doign a linear
search on a file containing thousands or millions of IP numbers
will take a long time.
And, of course, duplicate IPs widely separated in time are
probably different people. When you log on to your AOL,
you get an IP number. When you log out, the number is
used for some new login. Even with DSL or cable model,
you only have an IP number for a period of time. Then you
have a different IP number, and someone else has the
number you had.
While I don't claim to completely understand your problem, I
would suggest storing the time, along with the IP number. If a
match is found, compare the time with the current time to determine
how long ago the IP was last seen: accept it if 'long ago', for some
definition of 'long ago', reject it if 'recent'. Or to make things fast and
simple, you could define 'long ago, as more than 24-48 hours. Once
a day a cron job script could remove all entries more than a day old,
and the file would only contain 'recent' entries.
In line 8, you split a single @ipx entry based on new-lines, and on line 9
you process each line. However, a single @ipx entry only contains one
line, so the inner loop is unneccessary, though you might want to use
'chomp' to get rid of the newline.
In line 11, you do a numeric comparison on the IP numbers, but IP
numbers are neither integers nor floating point numbers, but strings
made up of numerals and dots.
Inside the loop you have:
if ( IP # matches ) {
} elsif ( count == 2 ) {
} else { # IP is new
}
Now, if the IP number matches once, the first block will tell the visitor
they cannot win again. Howver, the file is not updated.
If the IP numbers differ, but the current line of
the file has a count of 2, the elsif block matches and tells the visitor
they have won the maximum number of times. Even if this worked
properly, I don't understand the concept of a disallowing a winner
because they have already won, and also allowing them to win twice
but not three times. In any case, this would only disallow winning three
times, but then the next time they would presumably win again, assuming
the count were still updated. 'elsif' should check if ( $cnt >= 2 ).
But the real problem comes if the first IP number does not match the visitor's
IP number. In that case, the 'else' clause kicks in and attempts (incorrectly)
to add the current visitor to the file. You should not be adding the visitor
until all the entries in the file have been examined and have failed ..
foreach $line ( @ipx ) {
chomp $line;
( $cnt, $ipadd ) = split ' ', $line; # use a space as separator, don't bother with '|'
if ( IP matches ) {
if ( $cnt < $macWins ) {
# do something
} else {
# do soemthing different, or
# save IP and count in a variable
# for processign at end of loop
} # end if ( $cnt ...
} # end if ( IP ...
} # end foreach ( ...
# The IP wasn't found in the file, visitor hasn't been here before.
# Add the IP to the file and give them a prize.
In your else block, you only ever read in the first line of the file, obtain
the count from the first line, and increment it. Then you append a line
to the file with the same IP number and an incremented count. Perhaps
what you would like to do at this point is to copy the file, line by line,
until you reach the matching line, and copy an updated count. ( And continue
copying the rest of the file, so you don't lose entries. )
Of course there remains the question of several threads trying to update
the file at the same time.
A much better solution would be to use a database for the IP numbers and
counts. In that case, determining whether the IP number has been seen
before takes constant time, instead of being dependent on the number of prior
visitors, as does updating the count or adding new entries. At the very least,
use a DBM file with a hash interface ...
# Perl CookBook, Section 14.1
#
use DB_File;
dbmopen %ALL_IPS, $dbmFile, 066
or die( "Cannot open $dbmFile: $!" );
$cnt = 0;
if ( defined $ALL_IPS{$ipadd} ) { # is IP already in file?
$cnt = $ALL_IPS{$ipadd}; # get count for IP
}
$cnt++;
$ALL_IPS{$ipadd} = $cnt # add/update IP/count entry.
That will be $50. Please pay the secretary on yur way out ...
Good luck
Tom legrady
> If you guys are sick of me, let me know LOL (I've been learning a lot
> the past week or so, PHP and MySQL are more my forte, thank you for your
> help lately!). I have a program (see near bottom) that is supposed to
>
> open a file, read it, check to see if an ip is already entered or if the
> count has reached 2 and if so, print out a quick message saying they
> can't enter as they already have. Then, if it's a new ip and the count
> is less than 2, it will record the ip address, increment the count then
>
> (and this isn't in there yet) proceed to the "contest" page. It's only
> doing the count to 1 over and over and it's not picking up that an ip
> has been there before. Here are the errors I see when trying to process
> it:
>
> Use of uninitialized value in numeric eq (==) at test2.cgi line 15.
> Argument "67.40.29.147" isn't numeric in numeric eq (==) at test2.cgi
> line 15.
> Argument "0|\n" isn't numeric in numeric eq (==) at test2.cgi line 15.
> Use of uninitialized value in string ne at test2.cgi line 75, <RDIR>
> line 1.
> And here is what the ipdata.txt file records (not actual ip address
> obviously - and the 0| line was manually put in place to allow
> incrementing from 0):
>
>
>
> 0|
> 1|12.12.12.12
>
> 1|12.12.12.12
> 1|12.12.12.12
>
> Here is the program:
>
>
>
1 > #!/usr/bin/perl -w
2 > require('vars.pl');
3 > $TheWhere = $ENV{REMOTE_ADDR};
4 > open (RDIR, "$datafile") || die "Can't open '$datafile' $!\n";
5 > @ipx=<RDIR>;
6 > close (RDIR);
7 > foreach $line(@ipx) {
8 > @ipstuff = split(/.\\n/, $line);
9 > foreach $ipl (@ipstuff) {
10 > ($cnt, $ipadd) = split(/.\|./, $ipl);
11> if ($TheWhere == $ipadd) {
12> $BODY .= qq~ <font size=2 face="Verdana,Arial,Helvetica">You have
> already been to this page - you cannot win again.</font><p>~;
13> $BODY .= qq~<p><center><a href=http://www.somedomain.com>Back to main
> page</center>
> ~;
14> print "Content-type: text/html\n\n";
15> print <<ERRORHTML;
> <html>
> <head>
> <title>You've won already</title>
> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
> </head>
> <body bgcolor="#FFFFFF" text="#000000">
> <br><br><br><br>
> <table width="600" border="0" cellspacing="1" cellpadding="4"
> bgcolor="#333333" align="center">
> <tr>
> <td bgcolor="#EEEEEE"><font color="#FF0000" size="2" face="Arial,
> Helvetica, sans-serif"><b>Here is the problem:</b></font></td>
> </tr>
> <tr>
> <td bgcolor="#EEEEEE">$BODY</td>
> </tr>
> </table>
> </body>
> </html>
> ERRORHTML
> exit;
> }
> elsif ($cnt == 2) {
> $BODY .= qq~ <font size=2 face="Verdana,Arial,Helvetica">This prize has
> been won the maximum number of times.</font><p>~;
> $BODY .= qq~<p><center><a href=http://www.somedomain.com>Back to main
> page</center>
> ~;
> print "Content-type: text/html\n\n";
> print <<ERRORHTML;
> <html>
> <head>
> <title>Max Winners</title>
> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
> </head>
> <body bgcolor="#FFFFFF" text="#000000">
> <br><br><br><br>
> <table width="600" border="0" cellspacing="1" cellpadding="4"
> bgcolor="#333333" align="center">
> <tr>
> <td bgcolor="#EEEEEE"><font color="#FF0000" size="2" face="Arial,
> Helvetica, sans-serif"><b>Here is the problem:</b></font></td>
> </tr>
> <tr>
> <td bgcolor="#EEEEEE">$BODY</td>
> </tr>
> </table>
> </body>
> </html>
> ERRORHTML
> exit;
> }
> else {
> open(RDIR, "$datafile");
> @ipx = reverse(split (/.\|\\n/, <RDIR>));
> if ($ipx[1] ne $TheWhere) {
> $cnt = $ipx[0];
> $cnt++;
> } else {
> $cnt = 1;
> }
> close(RDIR);
> open(INF,">>$datafile");
> print INF "$cnt|$TheWhere\n";
> close(INF);
> }
> }
> }
> www.web-dev-design.com
> ------------------------------------
> Susanne Bullo, Developer Web Dev Design
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
POST TO: spug-list at pm.org PROBLEMS: owner-spug-list at pm.org
Subscriptions; Email to majordomo at pm.org: ACTION LIST EMAIL
Replace ACTION by subscribe or unsubscribe, EMAIL by your Email-address
For daily traffic, use spug-list for LIST ; for weekly, spug-list-digest
Seattle Perl Users Group (SPUG) Home Page: http://seattleperl.org
More information about the spug-list
mailing list