in reply to upload mod insert
I'm not sure if lines 2 and 3 do what you want to do. They seem to be an attempt to extract the filename component from a full path, however, if the full path ends with a forward or backward slash, you keep the full path (and then you end up trying to open a directory). Perhaps you want to use the Basename module instead. Furthermore, you do part of the filename manipulation in the upload_file sub, and part outside the sub (line 13). Also, you use global variables to pass arguments to the sub, instead of just using parameters.1: sub upload_file { 2: $filepath=$filename; 3: if ($filepath =~ /([^\/\\]+)$/) {$filename="$1";} 4: else {$filename="$filepath";} 5: $filename =~ s/\s+//g; 6: open UPLOADFILE, ">$upload_dir/$filename"; 7: while (<$upload_filehandle>) { 8: print UPLOADFILE; 9: } 10: close UPLOADFILE; 11: } 12: $filename = $formdata{"file"}; 13: $filename =~ s/(\s+|\n)?,(\s+|\n)?/,/g; 14: $upload_filehandle = upload($formdata{"file"}); 15: &upload_file; 16: if ($to_upload == 1) { 17: $formdata{'file'} = $filename 18: }; #ensures just filename written to database
In line 6, you open the file you want to write to. But what if the open fails? You assume it will succeed, but opening might fail. You should check for that, and not blindly proceed, assuming it succeeded.
In line 13, there's no need to write (\s+|\n)?. Since \s covers \n, the second clause will never match. Furthermore, no need to have an optional 1 or more times; zero or more times means the same. This means that line 13 could be written as:
$filename =~ s/\s*,\s*/,/g;
In line 14, you assign the result of upload to $upload_filehandle, from which you read in the sub. But you never test whether upload actually returned a valid filehandle. It might have returned undef.
In line 16, you check whether $to_upload equals 1. But that variable is never set in your code fragment.
Abigail
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re: Re: upload mod insert
by Anonymous Monk on Dec 05, 2003 at 10:16 UTC | |
by barrd (Canon) on Dec 05, 2003 at 12:43 UTC | |
by iburrell (Chaplain) on Dec 05, 2003 at 21:46 UTC |