Opened 15 years ago

Closed 15 years ago

Last modified 15 years ago

#2115 closed defect (fixed)

ZAK: Invalid memory read access during intro

Reported by: eriktorbjorn Owned by: eriktorbjorn
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Zak McKracken

Description

Latest ScummVM CVS snapshot English V2 Enhanced version of Zak McKracken

During the intro of the game, Valgrind will warn about invalid read access in Player_V2::execute_cmd(). This appears to be because the sound resource for the intro song is being nuked and reloaded while the music is playing. (This happens at the point in the intro where Zak sees himself.)

Inserting a trivial test, such as this, into createResource() appears to fix the immediate problem:

if (type == rtSound && isResourceLoaded(type, idx)) return address[type][idx] + sizeof(MemBlkHeader);

But this does not seem particularly elegant to me. While sound resources should stay the same throughout the game, I image there may be cases where a game really does want to "reset" a resource by reloading it.

For whatever it's worth, the resource is locked at the time so perhaps the test can be generalized into something like this instead:

if (isResourceLoaded(type, idx) && (flags[type][idx] & RF_LOC)) return address[type][idx] + sizeof(MemBlkHeader);

But is this what resource locking is for? I don't know enough about the resource management...

Ticket imported from: #1253171. Ticket imported from: bugs/2115.

Change History (3)

comment:1 by cyxx, 15 years ago

Actually your first solution seems fine, Erik. That's how the original interpreter behaves apparently : if a resource has already been loaded and its type is rtSound, rtScript or rtCostume, then it just re-uses the same pointer to the data. So, I think it's pretty safe to commit this modification as long as there is a (_version <= 2) check...

comment:2 by eriktorbjorn, 15 years ago

Ok, I've committed a modified version of my first change. Let's hope that fixes it. (I don't have Valgrind on this computer.)

Thanks for the help!

comment:3 by eriktorbjorn, 15 years ago

Owner: set to eriktorbjorn
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.