From 0c40d60d24e2b7419999c8c05bf00458dae3489e Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Thu, 5 Jan 2017 14:34:55 -0600
Subject: SCI32: Fix off-by-one error in array resizing

This bug existed in SSCI and was pulled in carelessly during
initial implementation of SciArray. Closer examination of SCI3
reveals that this only happened to work in SSCI because it would
always allocate on the first resize, and would always allocate 25
extra elements per allocation.
---
 engines/sci/engine/segment.h | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

(limited to 'engines/sci')

diff --git a/engines/sci/engine/segment.h b/engines/sci/engine/segment.h
index c5295c5523..538f31170d 100644
--- a/engines/sci/engine/segment.h
+++ b/engines/sci/engine/segment.h
@@ -542,7 +542,14 @@ public:
 	 */
 	reg_t getAsID(const uint16 index) {
 		if (getSciVersion() >= SCI_VERSION_3) {
-			resize(index);
+			// SCI3 resizes arrays automatically when out-of-bounds indices are
+			// passed, but it has an off-by-one error, always passing the index
+			// instead of `index + 1` on a read. This happens to work in SSCI
+			// only because the resize method there actually allocates memory
+			// for `index + 25` elements when growing the array, and it always
+			// grows the array on its first resize because it decides whether to
+			// grow based on byte size including an extra array header.
+			resize(index + 1);
 		} else {
 			assert(index < _size);
 		}
@@ -573,7 +580,8 @@ public:
 	 */
 	void setFromID(const uint16 index, const reg_t value) {
 		if (getSciVersion() >= SCI_VERSION_3) {
-			resize(index);
+			// This code is different from SSCI; see getAsID for an explanation
+			resize(index + 1);
 		} else {
 			assert(index < _size);
 		}
@@ -599,7 +607,8 @@ public:
 		assert(_type == kArrayTypeInt16);
 
 		if (getSciVersion() >= SCI_VERSION_3) {
-			resize(index);
+			// This code is different from SSCI; see getAsID for an explanation
+			resize(index + 1);
 		} else {
 			assert(index < _size);
 		}
@@ -616,6 +625,7 @@ public:
 		assert(_type == kArrayTypeInt16);
 
 		if (getSciVersion() >= SCI_VERSION_3) {
+			// This code is different from SSCI; see getAsID for an explanation
 			resize(index + 1);
 		} else {
 			assert(index < _size);
@@ -632,7 +642,8 @@ public:
 		assert(_type == kArrayTypeString || _type == kArrayTypeByte);
 
 		if (getSciVersion() >= SCI_VERSION_3) {
-			resize(index);
+			// This code is different from SSCI; see getAsID for an explanation
+			resize(index + 1);
 		} else {
 			assert(index < _size);
 		}
@@ -648,7 +659,8 @@ public:
 		assert(_type == kArrayTypeString || _type == kArrayTypeByte);
 
 		if (getSciVersion() >= SCI_VERSION_3) {
-			resize(index);
+			// This code is different from SSCI; see getAsID for an explanation
+			resize(index + 1);
 		} else {
 			assert(index < _size);
 		}
@@ -664,7 +676,8 @@ public:
 		assert(_type == kArrayTypeID || _type == kArrayTypeInt16);
 
 		if (getSciVersion() >= SCI_VERSION_3) {
-			resize(index);
+			// This code is different from SSCI; see getAsID for an explanation
+			resize(index + 1);
 		} else {
 			assert(index < _size);
 		}
-- 
cgit v1.2.3