Add "md5" command to the debugger

[eriktorbjorn]

Prompted by a recent discussion on the ScummVM forum, it occurred to me that perhaps it would be useful if the debugger had a command to print the MD5 sum of a file. Here's one possible implementation.

[clone2727]

This won't work for games using resource forks. Might not be necessary from the get-go, but it could come back to haunt us.

[eriktorbjorn]

Something like this? I can't test it very well.

[clone2727]

Yes, but that might confuse people expecting the full file name. You should be able to test with Loom Mac.

[eriktorbjorn]

I meant that I was able to test it, but I wasn't sure if it displayed the correct result or not.

I'm not sure what you mean by "the full file name".

[digitall]

@clone2727 : Do you have Loom Mac and thus could give the correct md5sum from a datafile for @eriktorbjorn to compare against his operating system "md5sum" command output and "md5" command output from this code?

If not, then maybe you can do this with a Mac demo such as http://sourceforge.net/projects/scummvm/files/demos/sword1/sword1-mac-demo-en.zip/download

[eriktorbjorn]

If memory serves me, Broken Sword 1 doesn't use any Macintosh resource forks, so the Broken Sword 1 demo probably isn't a useful test case for "md5mac". I think the point was that extracting a Macintosh file might not always produce the same MD5 sum for the resulting file (MacBinary, or whatever), but the MD5 sum for the resource fork should still be uniquely identifiable.

For instance, I have two versions of Monkey Island 1 for the Mac. The MD5 sum reported by "md5" for Monkey Island.bin differs, but "md5mac" reports 9febd4ec8df312e69bd9bb5a6953930e for both of them.

And since we were talking about Loom, the "md5mac" for mine is c9904095c326d626c581c53c790b2d61.

(Right now, I really wish the ScummVM console had clipboard support. I hope I didn't mistype! :-)

[clone2727]

@eriktorbjorn The file name scheme of the Mac resource fork container files might still trip this up. It expects the actual file name (ie. "Loom") instead of what the container might be (ie. "Loom.bin", "._Loom"). Maybe just specifying "base file name" or something.

[eriktorbjorn]

At least for me, either works. E.g. both "md5mac iMuse Setups.bin" and "md5mac iMuse Setups" finds the same file. (Unlike the "md5" command, the "md5mac" command doesn't handle wildcards, though.)

[clone2727]

Mac binary is the only case that works for, and I wish it didn't work in
that case either.

[eriktorbjorn]

Ah. Hmm... Can't think of any good way around that at the moment, really.

[bluegr]

This looks good to be merged, IMHO. Are there any other pending issues blocking its merge?

[digitall]

+1 from me on the merge... though if we do, we should comment a FIXME regarding the md5mac issues..

[eriktorbjorn]

I've added a FIXME comment. I assume that's what was referred to.

[digitall]

This has now been inactive since September... I don't see any major blockers here, apart from possibly dealing with Mac resource forks which will not affect the majority of users. I suggest we merge and then deal with an FIXME in tree later... as this provides a much easier way of getting md5sums from users for detection entries.

[bluegr]

Agree with @digitall, this looks good to be merged

[digitall]

OK. Core Team ( @sev- , @lordhoto , @wjp ), any objections to merging as-is?

[wjp]

as this provides a much easier way of getting md5sums from users for detection entries.

Doesn't it compute the full md5 of the file?

[digitall]

@wjp: Ah yes... Sorry, had been so long that until I read the commits in detail, I didn't note that.

This was intended to do a full MD5sum of files on the platform to remove the need to use external tools and otherwise make it easier to identify variant versions or corrupted datafiles.

We can easily modify this with an extra option later to allow limiting to bytes i.e. 5000 to create detection entry compatible MD5sums...

Anyway, not a blocker to merging this...

[wjp]

Ok, just checking the intended functionality matches the actual functionality.

[digitall]

@wjp Thanks for checking... :)

[digitall]

If no objections are raised by @sev- or @lordhoto before Friday, I will merge then.

[digitall]

As 2 days have passed with no further comments, merging.