[Scummvm-git-logs] scummvm master -> 5924b3810b5c859d1df1c469cb95a6690d7be14d

[AndywinXp]

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

Summary:
5924b3810b SCUMM: HE: Fix file handling opcodes issues


Commit: 5924b3810b5c859d1df1c469cb95a6690d7be14d
    https://github.com/scummvm/scummvm/commit/5924b3810b5c859d1df1c469cb95a6690d7be14d
Author: AndywinXp (andywinxp at gmail.com)
Date: 2024-07-07T17:51:51+02:00

Commit Message:
SCUMM: HE: Fix file handling opcodes issues

Unsurprisingly SPUTM handles files via integer file
handles, instead of SeekableStream objects :-P
This means that scripts can (and will!) occasionally
send out a -1 file handle which means that:
- Write operations will result in NOP
- Read operations will result in reading the value 0

Closes #13863:

"SCUMM/HE: Blue's Treasure Hunt - Breakout
Minigame Editor crashes the program"

This game was trying to open the -1 file handle,
and therefore it crashed while trying to access
position -1 in our array of file streams :'(

Changed paths:
    engines/scumm/he/script_v100he.cpp
    engines/scumm/he/script_v72he.cpp


diff --git a/engines/scumm/he/script_v100he.cpp b/engines/scumm/he/script_v100he.cpp
index 29e28bb40ed..b054f313f7d 100644
--- a/engines/scumm/he/script_v100he.cpp
+++ b/engines/scumm/he/script_v100he.cpp
@@ -2400,7 +2400,16 @@ void ScummEngine_v100he::o100_writeFile() {
 
 	byte subOp = fetchScriptByte();
 
-	assert(_hOutFileTable[slot]);
+	// The original doesn't make assumptions of any
+	// kind when the slot is -1 (which is a possible
+	// value from the scripts) and does NOPs instead...
+	if (slot != -1)
+		assert(_hOutFileTable[slot]);
+
+	// Arrays will handle the -1 value by themselves...
+	if (slot == -1 && subOp != SO_ARRAY)
+		return;
+
 	switch (subOp) {
 	case SO_ARRAY:
 		fetchScriptByte();
@@ -2777,20 +2786,35 @@ void ScummEngine_v100he::o100_readFile() {
 		break;
 	case SO_INT:
 		slot = pop();
-		assert(_hInFileTable[slot]);
-		val = _hInFileTable[slot]->readUint16LE();
+		if (slot == -1) {
+			val = 0;
+		} else {
+			assert(_hInFileTable[slot]);
+			val = _hInFileTable[slot]->readUint16LE();
+		}
+
 		push(val);
 		break;
 	case SO_DWORD:
 		slot = pop();
-		assert(_hInFileTable[slot]);
-		val = _hInFileTable[slot]->readUint32LE();
+		if (slot == -1) {
+			val = 0;
+		} else {
+			assert(_hInFileTable[slot]);
+			val = _hInFileTable[slot]->readUint32LE();
+		}
+
 		push(val);
 		break;
 	case SO_BYTE:
 		slot = pop();
-		assert(_hInFileTable[slot]);
-		val = _hInFileTable[slot]->readByte();
+		if (slot == -1) {
+			val = 0;
+		} else {
+			assert(_hInFileTable[slot]);
+			val = _hInFileTable[slot]->readByte();
+		}
+
 		push(val);
 		break;
 	default:
diff --git a/engines/scumm/he/script_v72he.cpp b/engines/scumm/he/script_v72he.cpp
index a443d568a27..d43ffa78039 100644
--- a/engines/scumm/he/script_v72he.cpp
+++ b/engines/scumm/he/script_v72he.cpp
@@ -1695,10 +1695,16 @@ int ScummEngine_v72he::readFileToArray(int slot, int32 size) {
 	byte *data = defineArray(0, kByteArray, 0, 0, 0, size);
 
 	if (slot != -1) {
+		assert(_hInFileTable[slot]);
 		_hInFileTable[slot]->read(data, size + 1);
 	}
 
-	return readVar(0);
+	int returnValue = readVar(0);
+
+	if (_game.heversion >= 80)
+		returnValue |= MAGIC_ARRAY_NUMBER;
+
+	return returnValue;
 }
 
 void ScummEngine_v72he::o72_readFile() {
@@ -1709,28 +1715,43 @@ void ScummEngine_v72he::o72_readFile() {
 	switch (subOp) {
 	case SO_BYTE:
 		slot = pop();
-		assert(_hInFileTable[slot]);
-		val = _hInFileTable[slot]->readByte();
+		if (slot == -1) {
+			val = 0;
+		} else {
+			assert(_hInFileTable[slot]);
+			val = _hInFileTable[slot]->readByte();
+		}
+
 		push(val);
 		break;
 	case SO_INT:
 		slot = pop();
-		assert(_hInFileTable[slot]);
-		val = _hInFileTable[slot]->readUint16LE();
+		if (slot == -1) {
+			val = 0;
+		} else {
+			assert(_hInFileTable[slot]);
+			val = _hInFileTable[slot]->readUint16LE();
+		}
+
 		push(val);
 		break;
 	case SO_DWORD:
 		slot = pop();
-		assert(_hInFileTable[slot]);
-		val = _hInFileTable[slot]->readUint32LE();
+		if (slot == -1) {
+			val = 0;
+		} else {
+			assert(_hInFileTable[slot]);
+			val = _hInFileTable[slot]->readUint32LE();
+		}
+
 		push(val);
 		break;
 	case SO_ARRAY:
 		fetchScriptByte();
 		size = pop();
 		slot = pop();
-		assert(_hInFileTable[slot]);
 		val = readFileToArray(slot, size);
+
 		push(val);
 		break;
 	default:
@@ -1744,6 +1765,7 @@ void ScummEngine_v72he::writeFileFromArray(int slot, int32 resID) {
 		(FROM_LE_32(ah->downMax) - FROM_LE_32(ah->downMin) + 1);
 
 	if (slot != -1) {
+		assert(_hInFileTable[slot]);
 		_hOutFileTable[slot]->write(ah->data, size);
 	}
 }
@@ -1773,7 +1795,16 @@ void ScummEngine_v72he::o72_writeFile() {
 	int slot = pop();
 	byte subOp = fetchScriptByte();
 
-	assert(_hOutFileTable[slot]);
+	// The original doesn't make assumptions of any
+	// kind when the slot is -1 (which is a possible
+	// value from the scripts) and does NOPs instead...
+	if (slot != -1)
+		assert(_hOutFileTable[slot]);
+
+	// Arrays will handle the -1 value by themselves...
+	if (slot == -1 && subOp != SO_ARRAY)
+		return;
+
 	switch (subOp) {
 	case SO_BYTE:
 		_hOutFileTable[slot]->writeByte(resID);