Opened 14 years ago

Closed 14 years ago

Last modified 14 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, 14 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, 14 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, 14 years ago

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