[Scummvm-cvs-logs] scummvm master -> f6e4312665614d7b5b626f997194fbbcb00ddbb0

[bluegr]

This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
44935117f4 SCI: Fix a workaround for an uninitialized variable in SQ4CD
f6e4312665 SCI: Add a hack for a bug in the script handling code


Commit: 44935117f493b2e256b5a2c701b63cef57d872af
    https://github.com/scummvm/scummvm/commit/44935117f493b2e256b5a2c701b63cef57d872af
Author: Filippos Karapetis (md5 at scummvm.org)
Date: 2012-07-02T02:49:08-07:00

Commit Message:
SCI: Fix a workaround for an uninitialized variable in SQ4CD

This makes sure that the workaround works for subclassed objects as well,
such as "theProfessor" talker. Fixes bug #3539350 - "SCI: SQ4 CD -
Crash in sewer when text and speech enabled"

Changed paths:
    engines/sci/engine/workarounds.cpp



diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index ecb1e4c..15fca03 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -162,7 +162,7 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_SQ1,            -1,   703,  0,                   "", "export 1",       -1,    0, { WORKAROUND_FAKE,   0 } }, // sub that's called from several objects while on sarien battle cruiser
 	{ GID_SQ1,            -1,   703,  0,         "firePulsar", "changeState", 0x18a,    0, { WORKAROUND_FAKE,   0 } }, // export 1, but called locally (when shooting at aliens)
 	{ GID_SQ4,            -1,   398,  0,            "showBox", "changeState",    -1,    0, { WORKAROUND_FAKE,   0 } }, // CD: called when rummaging in Software Excess bargain bin
-	{ GID_SQ4,            -1,   928,  0,           "Narrator", "startText",      -1, 1000, { WORKAROUND_FAKE,   1 } }, // CD: method returns this to the caller
+	{ GID_SQ4,            -1,   928, -1,           "Narrator", "startText",      -1, 1000, { WORKAROUND_FAKE,   1 } }, // CD: happens in the options dialog and in-game when speech and subtitles are used simultaneously
 	{ GID_SQ5,           201,   201,  0,        "buttonPanel", "doVerb",         -1,    0, { WORKAROUND_FAKE,   1 } }, // when looking at the orange or red button - bug #3038563
 	{ GID_SQ6,            -1,     0,  0,                "SQ6", "init",           -1,    2, { WORKAROUND_FAKE,   0 } }, // Demo and full version: called when the game starts (demo: room 0, full: room 100)
 	{ GID_SQ6,           100, 64950,  0,               "View", "handleEvent",    -1,    0, { WORKAROUND_FAKE,   0 } }, // called when pressing "Start game" in the main menu


Commit: f6e4312665614d7b5b626f997194fbbcb00ddbb0
    https://github.com/scummvm/scummvm/commit/f6e4312665614d7b5b626f997194fbbcb00ddbb0
Author: Filippos Karapetis (md5 at scummvm.org)
Date: 2012-07-02T02:49:10-07:00

Commit Message:
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

Changed paths:
    engines/sci/engine/seg_manager.cpp
    engines/sci/engine/seg_manager.h



diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index a6c1459..d425e17 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 356a1b0..074d3f6 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