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

Can't update file

by TClayJ (Initiate)
on Dec 01, 2016 at 16:30 UTC ( [id://1177063]=perlquestion: print w/replies, xml ) Need Help??

TClayJ has asked for the wisdom of the Perl Monks concerning the following question:

I am trying to update a file that contains the md5 checksum for a file if the value in the file is different from the new md5checksum but it is not updating. Here is the code, any suggestions are greatly appreciated.

if ( &download( $url, $save_file ) ) { # Successfully downloaded, now compair previous md5 value # and if different move to the transfer folder my ( $guidId, $ext ) = split ( '.zip', $guid ); my $md5File = "${hifld_md5_folder}/${guidId}.md5"; open ( MD5, '+>>', $md5File ) || &loggingError("Couldn't open md5 file ($md5File) for + reading : $!"); $md5row = <MD5>; close( MD5 ); my $md5chksum = &calcMD5( $save_file ); print "md5row = ${md5row}\n"; print "md5chksum = ${md5chksum}\n\n"; # check to see if the file has changed since the last down +load # if not then delete the downloaded file otherwise # update the md5 file and rename the downloaded file for m +oving if ( $md5row = $md5chksum ) { my $cmd = "rm $save_file"; # `$cmd`; } else { # Update the md5 file open ( MD5OUT, '>', $md5File ) || &loggingError("Couldn't open md5 file ($md5File) +for writing : $!"); print MD5OUT "${md5chksum}"; close( MD5OUT );

Replies are listed 'Best First'.
Re: Can't update file
by hippo (Bishop) on Dec 01, 2016 at 16:36 UTC
    if ( $md5row = $md5chksum ) {

    That's an assignment in there (=) whereas what you probably want is a comparison (==).

    Addendum: warnings would have alerted you to that.

Re: Can't update file
by davido (Cardinal) on Dec 01, 2016 at 16:38 UTC

    On this line:

    if ( $md5row = $md5chksum ) {

    ... you just assigned the value that $md5chksum contains to $md5row, because = is an assignment operator, not a relational equality operator. The relational equality operators are == for numeric comparisons, and eq for string comparisons. See perlop for details. This is also covered briefly in perlintro.

    Also, please remove every & that precedes a function call in your script unless you have a good reason for keeping it there. It's a bad habit because it actually alters the function call characteristics. This will be discussed in perlsub.


    Dave

Re: Can't update file
by kennethk (Abbot) on Dec 01, 2016 at 17:06 UTC
    my $md5chksum = &calcMD5( $save_file );
    How do you calculate MD5? You don't show that subroutine. Are you aware that Digest::MD5 is CORE?

    You should also be aware that invoking subroutines with the & sigil shows different behavior than calling a subroutine without it -- see Prototypes in perlsub. It can also interfere with optimization -- see Constant Functions in the same document.

    my $cmd = "rm $save_file"; # `$cmd`;
    Rather than invoking a system command, you can use unlink.
    open ( MD5, '+>>', $md5File ) || &loggingError("Couldn't open md5 file ($md5File) for + reading : $!"); $md5row = <MD5>; close( MD5 );
    It's generally good practice to use lexical file handles, since barewords are globals. Search for "lexical" in open.

    #11929 First ask yourself `How would I do this without a computer?' Then have the computer do it the same way.

Re: Can't update file
by Marshall (Canon) on Dec 01, 2016 at 17:49 UTC
    A few comments:

    For deleting a file, instead of shelling out to "rm", use the built in Perl function unlink. It is much faster and is portable across OS'es. Update: unlink returns the number of files deleted. So you can easily tell if it worked or not.

    This open, open ( MD5, '+>>', $md5File ), looks weird to me. I'd have to dig around a bit in the docs and do some experiments to figure exactly what that is going to do. I guess that means, read / append. In any event, you are just reading the file. Use open ( MD5, '<', $md5File ) instead. That more accurately describes what you are doing with the file.

    BTW, '+<' would be the normal way to open an existing file for read/write. '>+' is also read/write, but clobbers the file when opened initially (starts with a blank file). Its been awhile since I worked with a read/write file. Usually you need an intervening seek to flush buffers when switching between read and write. I'm not sure about '+>>'. In any event, you are not doing both reading and writing on this file, so there is no need to open with a mode that would allow that.

    print "md5row    = ${md5row}\n"; There is no need for the {} here. &loggingError(...) There is no need for the &. You can read more at perlsub.

    Update: This code, $md5row = <MD5>; reads a line including the line ending character(s) from the file handle MD5. I think that you need a chomp $md5row; to get rid of the line ending character(s).

Re: Can't update file
by stevieb (Canon) on Dec 01, 2016 at 16:37 UTC

    One issue I spotted immediately is this line:

    if ( $md5row = $md5chksum )

    The = is the assignment operator (which will return true, hence the else isn't being triggered). You need an equality operator (==) which checks whether the value on the left is equal to the value on the right. However, in this case, you're dealing with strings, so you need the eq operator. == is for numeric comparisons. So you need to change the above line to:

    if ( $md5row eq $md5chksum )

    Note that there may be other issues with the code, this was just a glaring one.

Re: Can't update file
by TClayJ (Initiate) on Dec 01, 2016 at 17:05 UTC

    Thanks guys I really appreciate the help.

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://1177063]
Approved by marto
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others exploiting the Monastery: (4)
As of 2024-03-29 10:28 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found