Re: Advice on a CGI script

I knew I was missing something. So here is my revised script:

#!/usr/bin/perl -Tw
use strict;
use CGI;
use Fcntl qw(:flock);

my $CGI = new CGI;
my $file;
my $countfile="downloadcounts.txt";
my $OS_Type=$CGI->param("OS");
my %OS_Count=();
my $OS_Name;
my $count;

open(COUNT,"$countfile")||die"Can't open: $!\n";
flock(COUNT,LOCK_SH);
while(<COUNT>){
  if(/^$/){}else{
  ($OS_Name,$count)=split /=/;
  chomp($OS_Name);
  chomp($count);
  $OS_Count{$OS_Name}=$count;
}
flock(COUNT,LOCK_UN);
close COUNT;

if($OS_Type=~/Linux/ || $OS_Type=~/Windows/){
  if(exists $OS_Count{$OS_Type}){
    $OS_Count{$OS_Type}++;
  }
  &write(\%OS_Count);
  if($OS_Type=~/Windows/){
    $file="http://tstanley.perlmonk.org/QuizTaker32-V108.zip";
    $CGI->redirect($file);
  }elsif($OS_Type=~/Linux/){
    $file="http://tstanley.perlmonk.org/QuizTaker-V1.08.tar.gz";
    $CGI->redirect($file);
  }else{
    print $CGI->header();
    print $CGI->start_html('Try Again!');
    print $CGI->h1(-align=>'center','Incorrect OS!!');
    print $CGI->end_html();
  }
}elsif($OS_Type=~/Show/){
  print $CGI->header();
  print $CGI->start_html('Number of Downloads');
  
  foreach my $key(keys %OS_Count){
    print $CGI->h3("$key = $OS_Count{$key} downloads");
  }

  print $CGI->end_html();
}

sub write{
  my $Hash=shift;

  open(COUNT,"+<$countfile")||die"Can't open: $!\n";
  flock(COUNT,LOCK_EX);
  seek(COUNT,0,0);
  truncate(COUNT,0);
  
  foreach my $key (keys %$Hash){
    print COUNT "$key=$$Hash{$key}\n";
  }
  flock(COUNT,LOCK_UN);
  close COUNT;
}
[download]

Thanks for all the great advice!

TStanley
--------
There's an infinite number of monkeys outside who want to talk to us
about this script for Hamlet they've worked out
-- Douglas Adams/Hitchhiker's Guide to the Galaxy
UPDATE: I loaded the above script, and am getting Internal server 500 errors. My error log shows the following:

[Tue Aug 21 19:54:01 2001] [error] [client 63.57.209.20] Premature end
+ of script
 headers: /home/tstanley/public_html/cgi-bin/download.cgi
Missing right bracket at /home/tstanley/public_html/cgi-bin/download.c
+gi line 68
, at end of line
syntax error at /home/tstanley/public_html/cgi-bin/download.cgi line 6
+8, at EOF
Execution of /home/tstanley/public_html/cgi-bin/download.cgi aborted d
+ue to comp
ilation errors.
[Tue Aug 21 19:54:11 2001] [error] [client 63.57.209.20] Premature end
+ of script
 headers: /home/tstanley/public_html/cgi-bin/download.cgi
Missing right bracket at /home/tstanley/public_html/cgi-bin/download.c
+gi line 68
, at end of line
syntax error at /home/tstanley/public_html/cgi-bin/download.cgi line 6
+8, at EOF 
Execution of /home/tstanley/public_html/cgi-bin/download.cgi aborted d
+ue to comp
ilation errors.
[Tue Aug 21 19:55:09 2001] [error] [client 63.57.209.20] Premature end
+ of script
 headers: /home/tstanley/public_html/cgi-bin/download.cgi
[download]

The links that call the script look like this:

<A HREF="http://tstanley.perlmonk.org/cgi-bin/download.cgi?OS=Linux">L
+inux</A>
[download]

UPDATE 2: I fixed the syntax error(forgot a } in the else statement where I read the file), but am still getting the premature end of script headers. I get these when I click on the links for Linux or Windows. Otherwise the script works when I show the number of downloads.

Comment on Re: Advice on a CGI script Select or Download Code

Replies are listed 'Best First'.
(Ovid) Re(2): Advice on a CGI script by Ovid (Cardinal) on Aug 22, 2001 at 04:48 UTC
Your problem is in this part: `while(<COUNT>){ if(/^$/){}else{ ($OS_Name,$count)=split /=/; chomp($OS_Name); chomp($count); $OS_Count{$OS_Name}=$count; }` [download] You missed a bracket for the "else". I'd probably code this something like this: `while(<COUNT>){ next if /^\s#?\s$/; # skip blank lines (the hash mark lets you a +dd comments) chomp; ($OS_Name,$count) = split /=/; $OS_Count{$OS_Name} = $count; }` [download] As for the "premature end of script headers", when you click links for Linux or Windows, you forgot to print the redirect :) `print $CGI->redirect($file);` [download] Cheers, Ovid Vote for paco! Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.	[reply] [d/l] [select]

Replies are listed 'Best First'.

(Ovid) Re(2): Advice on a CGI script
by Ovid (Cardinal) on Aug 22, 2001 at 04:48 UTC

Your problem is in this part:

while(<COUNT>){
  if(/^$/){}else{
  ($OS_Name,$count)=split /=/;
  chomp($OS_Name);
  chomp($count);
  $OS_Count{$OS_Name}=$count;
}
[download]

You missed a bracket for the "else". I'd probably code this something like this:

while(<COUNT>){
    next if /^\s*#?\s*$/; # skip blank lines (the hash mark lets you a
+dd comments)
    chomp;
    ($OS_Name,$count)   = split /=/;
    $OS_Count{$OS_Name} = $count;
}
[download]

As for the "premature end of script headers", when you click links for Linux or Windows, you forgot to print the redirect :)

    print $CGI->redirect($file);
[download]

Cheers,
Ovid

Vote for paco!

Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

[reply]
[d/l]
[select]