Opened 18 years ago
Closed 18 years ago
Last modified 18 years ago
#2115 closed defect (fixed)
ZAK: Invalid memory read access during intro
|Reported by:||eriktorbjorn||Owned by:||eriktorbjorn|
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 , 18 years ago
comment:2 by , 18 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 , 18 years ago
|Status:||new → closed|
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...