Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl-Sensitive Sunglasses
 
PerlMonks  

Re^6: Poorly written script

by Baffled (Acolyte)
on Feb 11, 2008 at 16:25 UTC ( #667410=note: print w/ replies, xml ) Need Help??


in reply to Re^5: Poorly written script
in thread Poorly written script

Problem is there isnt always four picture files, sometimes its 3, maybe 1,2 or even none. Doesnt your loop assume there is always going to be all four image files?

Also I do believe it does keep those upto 4 image files in RAM for the next page to be completed. This is the first page to list items where they upload the images, after image upload is done they go on to the item listing form, where they choose thier category and it isnt until they submit the next page do these images get assigned thier item number and appropiate destination in the file tree.

Also, the script use to have the "use strict;" at the top of all the files, about 2 years ago when I moved the site to the dedicated server the hosting company (godaddy) told me I had to change it to "use Carp;" to work on thier systems. It worked fine for two years until last week when these problems started.


Comment on Re^6: Poorly written script
Re^7: Poorly written script
by amarquis (Curate) on Feb 11, 2008 at 21:04 UTC

    Check for the files the same way you do now, just do it in the loop. Maybe something like:

    while ($filehandle = get_next_file()) { ... }

    So you spin off your "check if the file exists" and "open the file" code into a new function, and keep doing the loop while there are more files to find.

Re^7: Poorly written script
by graff (Chancellor) on Feb 12, 2008 at 04:29 UTC
    the script use to have the "use strict;" at the top of all the files, about 2 years ago when I moved the site to the dedicated server the hosting company (godaddy) told me I had to change it to "use Carp;" to work on thier systems.

    use strict and use Carp are not mutually exclusive, they do not conflict with each other, you can have them both in the same script at the same time, and it would be a good idea to do so.

    I admire any hosting operation that recommends using Carp. (Whether you also use the "fatals to browser" option is something you should be allowed to decide on your own, but normally you only use that during development, not in a "production" version). I cannot understand (and would hardly believe) that they would also insist on removing "use strict".

    By the way, if they want you to use Carp, you should actually invoke its functions at suitable points in your script (i.e. where errors occur) -- just putting use Carp; at the top accomplishes nothing unless you actually call some of its functions.

    I do believe it does keep those upto 4 image files in RAM for the next page to be completed.

    You should probably study the manual for CGI more closely, and focus on the section that describes "CREATING A FILE UPLOAD FIELD". You should also check to see what version of CGI is being used by the host server -- your code may have been written for an older version of CGI (pre-2.47), because you include your own "upload()" function, but as of v2.47, there is an "upload()" function defined within CGI.pm (I'm looking at v3.33 that came with perl 5.8.8). Make sure you study the manual that goes with the version you are using.

    I'm not personally familiar with file-upload operations in CGI, and I don't quite follow what your "upload" subroutine is doing (or whether the CGI built-in "upload()" is supposed to do the same thing), but it should not be the case that the image files being uploaded by your clients would all be memory resident while the script is running. The server should be storing the uploaded files to a temp directory, and your method of copying their contents to permanent files appears to be keeping the memory load to a minimum.

    Still, you need to check the CGI manual very carefully about file uploads. (The inclusion of "use strict" has an important impact, too. Read about it.)

    Problem is there isnt always four picture files, sometimes its 3, maybe 1,2 or even none. Doesnt your loop assume there is always going to be all four image files?

    As suggested by the previous reply, this is not a problem. In fact, here's what I was thinking of when I made the suggestion about the loop -- this is a version of (most of) your code, with "use strict" put back in, a nod to "File::Basename" (which is handy for your case), and a pass through perltidy (though I have made some of my own adjustments to some of the indents, and fiddled with the arrangements of some of the print statements):

    That is based on the version you posted, it compiles with strict and warnings enabled, and it covers everything except your "upload()" and "error()" subroutines. (Your "upload()" sub would undergo a similar amount of shortening by using a loop over the elements of the "%uploads_done" hash, which is populated in the "for" loop that I'm suggesting here.)

    There are still likely to be problems -- the code you posted seems to be using bunch of different stuff in a "%config" hash even though you only assign one thing to that hash ($config{header}); there are a few other variables being used without anything being assigned to them ($method and $action -- but CGI probably provides sensible defaults for these). And assuming your "upload()" sub does something different from the CGI built-in "upload()", your sub should probably be given a different name. Also, the idea of using a template system of some sort would help.

    Final summary: I'm still not sure any of this addresses your problem with memory consumption, unless you find something that needs fixing about your handling of the file uploads. You may still need to add some diagnostics to find out what's going on with the memory usage.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://667410]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others exploiting the Monastery: (5)
As of 2014-07-26 14:01 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    My favorite superfluous repetitious redundant duplicative phrase is:









    Results (177 votes), past polls