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
In reply to Re: upload mod insert
by Abigail-II
in thread upload mod insert
by arfa
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |