aboutsummaryrefslogtreecommitdiff
path: root/engines
diff options
context:
space:
mode:
authorFilippos Karapetis2012-07-02 12:48:02 +0300
committerFilippos Karapetis2012-07-02 12:49:10 +0300
commitf6e4312665614d7b5b626f997194fbbcb00ddbb0 (patch)
tree966096ebfb9dbbf4b695fefc27d7803a3f3c2de2 /engines
parent44935117f493b2e256b5a2c701b63cef57d872af (diff)
downloadscummvm-rg350-f6e4312665614d7b5b626f997194fbbcb00ddbb0.tar.gz
scummvm-rg350-f6e4312665614d7b5b626f997194fbbcb00ddbb0.tar.bz2
scummvm-rg350-f6e4312665614d7b5b626f997194fbbcb00ddbb0.zip
SCI: Add a hack for a bug in the script handling code
When resetting the segment manager, sometimes the locals block for a script is placed in a segment smaller than the script itself. This shouldn't be happening, but it isn't fatal, however it should be resolved in a proper manner
Diffstat (limited to 'engines')
-rw-r--r--engines/sci/engine/seg_manager.cpp41
-rw-r--r--engines/sci/engine/seg_manager.h8
2 files changed, 22 insertions, 27 deletions
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index a6c145979f..d425e170ce 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -143,16 +143,34 @@ Script *SegManager::allocateScript(int script_nr, SegmentId *segid) {
}
void SegManager::deallocate(SegmentId seg) {
- if (!check(seg))
- error("SegManager::deallocate(): invalid segment ID");
+ if (seg < 1 || (uint)seg >= _heap.size())
+ error("Attempt to deallocate an invalid segment ID");
SegmentObj *mobj = _heap[seg];
+ if (!mobj)
+ error("Attempt to deallocate an already freed segment");
if (mobj->getType() == SEG_TYPE_SCRIPT) {
Script *scr = (Script *)mobj;
_scriptSegMap.erase(scr->getScriptNumber());
- if (scr->getLocalsSegment())
- deallocate(scr->getLocalsSegment());
+ if (scr->getLocalsSegment()) {
+ // HACK: Check if the locals segment has already been deallocated.
+ // This happens sometimes in SQ4CD when resetting the segment
+ // manager: the locals for script 808 are somehow stored in a
+ // smaller segment than the script itself, so by the time the script
+ // is about to be freed, the locals block has already been freed.
+ // This isn't fatal, but it shouldn't be happening at all.
+ // FIXME: Check why this happens. Perhaps there's a bug in the
+ // script handling code?
+ if (!_heap[scr->getLocalsSegment()]) {
+ warning("SegManager::deallocate(): The locals block of script "
+ "%d has already been deallocated. Script segment: %d, "
+ "locals segment: %d", scr->getScriptNumber(), seg,
+ scr->getLocalsSegment());
+ } else {
+ deallocate(scr->getLocalsSegment());
+ }
+ }
}
delete mobj;
@@ -307,21 +325,6 @@ reg_t SegManager::findObjectByName(const Common::String &name, int index) {
return result[index];
}
-// validate the seg
-// return:
-// false - invalid seg
-// true - valid seg
-bool SegManager::check(SegmentId seg) {
- if (seg < 1 || (uint)seg >= _heap.size()) {
- return false;
- }
- if (!_heap[seg]) {
- warning("SegManager: seg %x is removed from memory, but not removed from hash_map", seg);
- return false;
- }
- return true;
-}
-
// return the seg if script_id is valid and in the map, else 0
SegmentId SegManager::getScriptSegment(int script_id) const {
return _scriptSegMap.getVal(script_id, 0);
diff --git a/engines/sci/engine/seg_manager.h b/engines/sci/engine/seg_manager.h
index 356a1b04a7..074d3f6b0a 100644
--- a/engines/sci/engine/seg_manager.h
+++ b/engines/sci/engine/seg_manager.h
@@ -471,14 +471,6 @@ private:
void createClassTable();
SegmentId findFreeSegment() const;
-
- /**
- * Check segment validity
- * @param[in] seg The segment to validate
- * @return false if 'seg' is an invalid segment, true if
- * 'seg' is a valid segment
- */
- bool check(SegmentId seg);
};
} // End of namespace Sci