Digest-MD5-File reviews

cpanratings
 

RSS | Module Info | Add a review of Digest-MD5-File

Digest-MD5-File (0.06)

Thanks for your input. I've commented inline to your comments, I hope the response helps get better info and a higher rating :)

> Major Problems:
> No meaningful tests.

0.06 added a couple, more would be great!

> 'file_md5', 'url_md5', etc. reads entire files into memory rather
> than using Digest::MD5's addfile() function or reading the file in
> chunks and using Digest::MD5's add(). (Strangely, however,
> addpath() does not have this problem.)

Addressed in 0.06 thanks to a good rt report. URLs of course are slurped in still (ala LWP), patches to improve that are welcome.

> 'adddir' relies on the order of keys in a hash to produce
> consistent results, which is broken since 'identical' directories
> may be read in different orders and because of hash randomization.

Where does it relay on the order of keys? It works as outlined:

"Returns a hashref whose keys are files relative to the given path and the values are the MD5 sum of the file or and empty string if a directory. It recurses through the entire depth of the directory. Symlinks to files are just addpath()d and symlinks to directories are followed."

order is never mentioned.

Perhaps an enhancement can be made if you can be clearer on the problem you experienced. Code is required.

> Passing binmode argument to 'file_md5*' does not actually do
> anything.

It opens any filehandles in binmode, can you actually substantiate that accusation with some code or are you just confused as to it purpose? If it does not work as advertised we can fix it ASAP.

> Minor Problems:

these are good also, I have a suggestion though:

Instead of a mostly pointless rant that an author will probably never see (IE not alerted to to new posts) why not open an rt report so these things can be addressed (IE alerted with new or updated tickets)?

Dan Muey - 2007-11-24T15:29:31 (permalink)

2 out of 3 found this review helpful. Was this review helpful to you?  Yes No

Digest-MD5-File (0.05) **

Major Problems:
No meaningful tests.
'file_md5', 'url_md5', etc. reads entire files into memory rather than using Digest::MD5's addfile() function or reading the file in chunks and using Digest::MD5's add(). (Strangely, however, addpath() does not have this problem.)
'adddir' relies on the order of keys in a hash to produce consistent results, which is broken since 'identical' directories may be read in different orders and because of hash randomization.
Passing binmode argument to 'file_md5*' does not actually do anything.

Minor Problems:
Documentation uses 'Digest::Md5' where it means 'Digest::MD5'.
Probably should subclass Digest::MD5, not add methods in its namespace.
Probably should have BINMODE, etc. be settable as per-object properties in its OO extensions.
Using readline() with a default $/ to read chunks of input is a poor choice for binary files.

Charles Reiss - 2006-12-10T12:03:13 (permalink)

6 out of 7 found this review helpful. Was this review helpful to you?  Yes No

Digest-MD5-File (0.04) *****

Should be part of Digest::MD5.

نديم الخمير - 2006-03-17T09:58:37 (permalink)

2 out of 6 found this review helpful. Was this review helpful to you?  Yes No