in reply to upload mod insert

Let's first reformat your code to have proper indentation, and let's extract the sub from the code:
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
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.

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
    What's module Basename (I can't find it)?
      Basename.pm is included in the standard distro and will be located somewhere in your @INC directories in a directory called File which is its root namespace.

      Try this on the CLI:
      perl -e 'print join("\n", @INC);'

      And you will see that for example in my system it is here with /usr/share/perl/5.8.0 being in my @INC:

      barrd:~# locate Basename.pm /usr/share/perl/5.8.0/File/Basename.pm

      Update: Added more text to make a bit more sense

        The module isn't Basename, it is File::Basename. It lives in a file named File/Basename.pm. But the package is "File::Basename". It is used like this:
        use File::Basename; my $file = basename($path);