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


In reply to Re: upload mod insert by Abigail-II
in thread upload mod insert by arfa

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.