Confusing Syntax

eoin has asked for the wisdom of the Perl Monks concerning the following question:

Greetings Bretherin,

I've currently been working on a online photo album "sort of thing". I'm using cgi scripts to do everything from generate pages to resizing images. its a large enough project for some one as inexperienced as me but I'm glad to say that its coming along nicely.
I've only had one main problem so far and thats been the file upload script. Seeing as I've never done one before or even seen one, I was bound to have trouble with it. I've been a while at it and I think I have it but...
Theres always a but!!!
Perl keeps throwing up these silly, confusing syntax errors and I'm stumped. I'm probably just not seeing whats wrong.
Also if you could tell me whether my way of uploading is, first of all, right and/or is there another "better" way to do this.

These are the errors Perl is giving me:

syntax error at upload.cgi line 18, near "%pics("
syntax error at upload.cgi line 36, near "@pic_filehandles("
syntax error at upload.cgi line 73, near "while <$pic_filehandles[$cnt
+r]>"
syntax error at upload.cgi line 80, near "}"
[download]

And heres the code
I've inserted a line of hashes just before the error lines just to save you counting.

#!/usr/bin/perl -w 

use CGI;
$q = new CGI;

my $album     =     $q->param("T1");    

unless ( -d "./albums/$album" ) {

 mkdir( "./albums/$album", 0775 ) || die "Can't create $webpage_dir: (
+$!)\n";

}
    


my $upload_dir = "albums/$album";
############################################
%pics(
pic0    =>     $q->param("photo")    ,
pic1    =>     $q->param("photo1")    ,
pic2    =>     $q->param("photo2")    ,
pic3    =>     $q->param("photo3")    ,
pic4    =>     $q->param("photo4")    ,
pic5    =>     $q->param("photo5")    ,
pic6    =>     $q->param("photo6")    ,
pic7    =>     $q->param("photo7")    ,
pic8    =>     $q->param("photo8")    ,
pic9    =>     $q->param("photo9")    ,
pic10    =>     $q->param("photo10")    ,
pic11    =>     $q->param("photo11")    ,
pic12    =>     $q->param("photo12")    ,
pic13    =>     $q->param("photo13")    ,
pic14    =>     $q->param("photo14")    
);
#########################################
@pic_filehandles(
$q->upload("photo")    ,
$q->upload("photo1")    ,
$q->upload("photo2")    ,
$q->upload("photo3")    ,
$q->upload("photo4")    ,
$q->upload("photo5")    ,
$q->upload("photo6")    ,
$q->upload("photo7")    ,
$q->upload("photo8")    ,
$q->upload("photo9")    ,
$q->upload("photo10")    ,
$q->upload("photo11")    ,
$q->upload("photo12")    ,
$q->upload("photo13")    ,
$q->upload("photo14")    
);

strip_filename();

sub strip_filename{

my @allkeys = sort(keys(%pics));
foreach $key (@allkeys){
$pics{$key} =~ s/.*[\/\\](.*)/$1/;
}
}


my $cntr = "0";
@files = sort(keys(%pics));
foreach $key (@files){

open IMAGE, ">$upload_dir/$pics{$key}";

binmode IMAGE;
########################################
while <$pic_filehandles[$cntr]>
{
print IMAGE;
}

close IMAGE;
$cntr += 1;
#############################################
}


 print $q->header ( ); 
 print <<END_HTML; 

 <HTML> 
#RANDOM HTML
 </HTML> 

 END_HTML
[download]

I'm stumped so any and all help will be greatly appriciated.
All the Best, Eoin... _{If everything seems to be going well, you obviously don't know what the hell is going on.}

Comment on Confusing Syntax Select or Download Code

Replies are listed 'Best First'.
Re: Confusing Syntax by Juerd (Abbot) on Aug 12, 2003 at 11:41 UTC
Besides the syntax errors that others have already corrected, I'd like to make some suggestions. You use 15 differently named input fields. Perhaps you don't know that when you create 15 that all have the same name, you can get a list from $q->param. `Photo 1: <input type=file name=photo><br> Photo 2: <input type=file name=photo><br> Photo 3: <input type=file name=photo><br> ...` [download] `#!/usr/bin/perl -w use strict; use CGI; my $cgi = CGI->new; my @pictures = $cgi->param('photo'); ...` [download] You don't check the value of $album and then you use it. This means that anyone can create directories anywhere the web server can. Very dangerous. The script doesn't check if open succeeded. When it can't create the file, it silently continues. open or die print or die close or die It is a very good idea to indent your code. Inside {} blocks, indent everything to visualize the block. This makes reading code much easier. Especially when you ask for help, you should do everything you can to make it easy to read. I forgot the most important mistake. The code doesn't use strict. Always use strict! `use strict;` [download] Your code will not work with strict without modification. A good Perl book or tutorial will tell you how to change your code. If you don't already have a good Perl book, try Beginning Perl. It's free. Juerd # { site => 'juerd.nl', plp_site => 'plp.juerd.nl', do_not_use => 'spamtrap' }	[reply] [d/l] [select]
Re: Re: Confusing Syntax by eoin (Monk) on Aug 12, 2003 at 11:57 UTC
Thanks for the help. I never knew about getting a list from param. Thats pretty handy. I still have problems though. The error `syntax error at upload.cgi line 73, near "while <$pic_filehandles[$cntr]>"` is still present and I can't figure it out. Also could you please elaborate on your comment about checking the folder before using it. All the Best, Eoin... _{If everything seems to be going well, you obviously don't know what the hell is going on.}	[reply] [d/l]
Re: Re: Re: Confusing Syntax by Juerd (Abbot) on Aug 12, 2003 at 12:09 UTC
I never knew about getting a list from param. Reading the documentation really helps. syntax error at upload.cgi line 73, near "while <$pic_filehandles[$cntr]>" You should use parentheses with while. Again, you should read the documentation. Also could you please elaborate on your comment about checking the folder before using it. Try to see what happens if the web users specifies "../hello_world" as the album. Juerd # { site => 'juerd.nl', plp_site => 'plp.juerd.nl', do_not_use => 'spamtrap' }	[reply]
Re: Re: Re: Re: Confusing Syntax by eoin (Monk) on Aug 12, 2003 at 18:50 UTC
Re: Confusing Syntax by larsen (Parson) on Aug 12, 2003 at 10:38 UTC
Your code: `%pics( pic0 => $q->param("photo") , ... omissis ...` [download] Try this instead (note the `=`): `%pics = ( ... omissis ...` [download] Update: For the `while` part, you should look in `perlsyn`.	[reply] [d/l] [select]
Re: Confusing Syntax by ctilmes (Vicar) on Aug 12, 2003 at 11:03 UTC
You also forgot `use strict;` and `use warnings;` at the top. BTW, you could simplify some of those long similar lists by looping over 1..14: `my %pics = (pic0 => $q->param("photo")); $pics{"pic$_"} = $q->param("photo$_") for 1..14; my @pic_filehandles = map { $q->upload("photo$_") } '',1..14;` [download] (It would get even simpler if you used "photo0" for your first name.)	[reply] [d/l] [select]
Re: Confusing Syntax by Skeeve (Parson) on Aug 12, 2003 at 10:42 UTC
syntax error at upload.cgi line 18, near "%pics(" What should that line do? Did you mean: "my %pics= (...);"? syntax error at upload.cgi line 36, near "@pic_filehandles(" I think you meant: "my @pic_filehandles= (...); syntax error at upload.cgi line 73, near "while <$pic_filehandles$cntr>" Try () around your <$pic...> syntax error at upload.cgi line 80, near "}" Don't know. Maybe gone away if previous errror is fixed.	[reply]