in reply to Adding directory to code

Do you know how -- or whether -- the thumbnail file will be identified in the QUERY_STRING parameters? (It looks like the full image file is named by a parameter called "FILE1" ("FILE2",...) -- will the name of the thumbnail file be predictable from the values of these parameters, or are there other parameters for the thumbnail files? Or are the names of full and thumbnail files interleaved in this set of parameters, e.g. FILE1 is full, FILE2 is thumbnail?) If you don't know, you need to figure that out. Then the rest will be pretty easy.

As it stands, the code is reading a full-image file from whatever path is provided in the "FILE$a" parameters, and copying it to the directory specified by "$ENV{DOCUMENT_ROOT}", which is the value being assigned to $Photo_Dir.

Let's assume that a thumbnail file ends up being one of the "FILE$a" parameters. If you declare a variable like "$Thumb_Dir", with a suitable value, and figure out how to identify an input thumbnail file, you'll probably end up altering the "open(OUTFILE...)" line a bit and adding one extra instruction just before that, something like the following:

... my $filename = ( $full_image ) ? "$Photo_Dir/$filename" : "$Thumb_dir/th_$filename"; if(open(OUTFILE, ">$filename")) { while (my $bytesread = read($file, my $buffer, 1024)) { print OUTFILE $buffer; } close (OUTFILE); ...

P.S.: Lighten up on merlyn. What he said was true -- you were presenting code written by someone else (though not all of it -- you missed a closing curly brace), and you did not state the problem clearly enough for anyone to give you a solid answer (that's why my response had so many questions in it.) It's both entirely possible and fully correct to interpret his response as meaning no personal offense. (For that matter, considering the amount of redundant and/or unnecessary code in this script, saying that someone else must have written it could have be taken as a compliment...)

Replies are listed 'Best First'.
Re: Re: Adding directory to code
by cal (Beadle) on Nov 04, 2002 at 04:39 UTC
    Thanks Graff,
    Actually I thought the full image files would be prefixed with a "P_" and the thumbnail files would be prefixed with "T_". This would allow them to be identified if sets e.g. P_FILE1 is full and T_FILE1 is thumbnail. However may be this does not make sense.

    So should there be an additional line
    $Thumb_Dir                 ||= $ENV{'DOCUMENT_ROOT'};

    I get a little lost in altering the "open(OUTFILE...)".

    Programmers often use code not written by themselves. It is sometimes a good basis for learning and not making the same mistakes. Merlyn and I would have a great time chatting about the Australian Aborigine Dreamtime which should be included in the Mysterious Places website.

    Cala
      Actually I thought the full image files would be prefixed with a "P_" and the thumbnail files would be prefixed with "T_". This would allow them to be identified if sets e.g. P_FILE1 is full and T_FILE1 is thumbnail.

      This makes sense if you are updating the form that invokes this script at the same time that you are updating the script. I assume you are referring to parameter names here (strings that will be passed to the CGI object's "param()" call).

      This will entail changing a little more than just the "open()" call, and it this point, it's worthwhile to go over the whole thing to resolve some other issues. I'm no CGI guru, but the opening part seems very strange and basically wrong:

      $max_num_files ||= 1; $Photo_Dir ||= $ENV{'DOCUMENT_ROOT'}; undef @bad_extensions if @good_extensions; for(my $a = 1; $a <= $max_num_files; $a++) { my $req = new CGI; ...
      That's testing an array and two scalars for previous values when nothing could have been assigned to them before this; so $max_num_files is always 1 and @bad_extensions is always undef. Then, why create a new CGI object on each iteration of a loop (even given that the loop only operates once)?

      Let me suggest the following -- with a grain of salt since I can't test it (and I'm not a CGI guru) -- which is based on assuming that the original script you posted was all there is to it. I'm adding commentary, eliminating redundant instructions, restructuring the loop, adding some different error checking on output, and noting where large blocks of code were removed because they would never execute anyway (being based on testing uninitialized variables):

      use strict; use CGI; my $req = new CGI; foreach my $type ( qw/P T/ ) # 'P' for Photo, 'T' for Thumbnail { my $indx = 0; while ( 1 ) # we'll exit when $indx gets high enough { my $paramName = sprintf( "%s_FILE%d", $type, ++$indx ); last unless ( $req->param( $paramName )); my $outname = my $inname = $req->param( $paramName ); $outname =~ s/^.*(\\|\/)//; # remove leading path $outname =~ s/ +/\_/g; # don't want spaces $outname =~ s/\"//g; # don't want quotes $outname = "th_$outname" if ( $type eq "T" ); # removed long portion that depended on @good_extensions and/or # @bad_extensions having contents : no evidence that they ever did # removed long portion that depended on $auto_rename having a value of # 1 or 2 and checking whether "$Photo_Dir/$outname" already exists : # no evidence that $auto_rename is ever non-zero # ( I think the original posted code forgot to open the input file ) my $insize = ( -s $inname ); ( $insize && open( INFILE, $inname )) or next; if ( open( OUTFILE, ">$ENV{DOCUMENT_ROOT}/$outname")) { my $outsize = 0; while ( my $nr = sysread( INFILE, my $buffer, 1024 )) { my $nw = syswrite( OUTFILE, $buffer ); last if ( $nr != $nw ); $outsize += $nw; } close OUTFILE; if ( $outsize != $insize ) { last; # might want to report this somehow... } # removed long portion that depended on $max_size being non-zero : # no evidence that it ever was } close INFILE; } }
      Programmers often use code not written by themselves. It is sometimes a good basis for learning...

      I certainly do agree, and I endorse the practice. I think the point is simply that when you say "I'm working on this code that someone else left behind...", readers will understand the situation better (and not blame you for lousy coding).