[ scummvm-Patches-3324322 ] Tools: compress_sci "Pharkas Fixup" incorrect

[SourceForge.net]

Patches item #3324322, was opened at 2011-06-21 20:29
Message generated for change (Tracker Item Submitted) made by agf863
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=418822&aid=3324322&group_id=37116

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: agf (agf863)
Assigned to: Nobody/Anonymous (nobody)
Summary: Tools: compress_sci "Pharkas Fixup" incorrect

Initial Comment:
On line 67 of engines/sci/compress_sci.cpp,
if ((memcmp(buffer + 1, "RIFF", 4) == 0) || (memcmp(buffer + 1, "\x8d\x0bSOL", 4) == 0)) {
compares the RIFF header correctly but not the SOL header: the given string is 5 characters not 4, and the correct string, "\x8d\x0bSOL\x00" is 6.

The comparison probably still will always give correct results, but the offset will only be correctly adjusted if the header is length 4:
for (int i = 0; i < 5; i++)
    buffer[i] = buffer[i + 1];
_input.read_throwsOnError(&buffer[5], 1);

This patch fixes the comparison and adjustment for SOL files / 6 byte headers. I assume that there aren't actually any SOL resources with the header offset by 1 byte, or else this bug would have been spotted previously, so alternatively the
if ((memcmp(buffer + 1, "RIFF", 4) == 0) || (memcmp(buffer + 1, "\x8d\x0bSOL", 4) == 0)) {
line could just be reduced to 
if (memcmp(buffer + 1, "RIFF", 4) == 0) {
if checking for an offset RIFF header is adequate.

I'm on the road so I don't have my Freddy Pharkas CD to test this patch right now but it "looks right" and compiles/links.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=418822&aid=3324322&group_id=37116