SCI: Added MemObject::isValidOffset method; use it to simplify determine_reg_type

svn-id: r41175

3 files changed, 90 insertions, 79 deletions

diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index 40c7be207d..d3bcae508c 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -729,12 +729,14 @@ void Kernel::mapFunctions() {
 
 int determine_reg_type(EngineState *s, reg_t reg, bool allow_invalid) {
 	MemObject *mobj;
+	int type = 0;
 
 	if (!reg.segment) {
+		type = KSIG_ARITHMETIC;
 		if (!reg.offset)
-			return KSIG_ARITHMETIC | KSIG_NULL;
+			type |= KSIG_NULL;
 
-		return KSIG_ARITHMETIC;
+		return type;
 	}
 
 	if ((reg.segment >= s->seg_manager->_heap.size()) || !s->seg_manager->_heap[reg.segment])
@@ -755,52 +757,31 @@ int determine_reg_type(EngineState *s, reg_t reg, bool allow_invalid) {
 			return KSIG_REF;
 
 	case MEM_OBJ_CLONES:
-		if (allow_invalid || ((CloneTable *)mobj)->isValidEntry(reg.offset))
-			return KSIG_OBJECT;
-		else
-			return KSIG_OBJECT | KSIG_INVALID;
+		type = KSIG_OBJECT;
+		break;
 
 	case MEM_OBJ_LOCALS:
-		if (allow_invalid || reg.offset < (*(LocalVariables *)mobj)._locals.size() * sizeof(reg_t))
-			return KSIG_REF;
-		else
-			return KSIG_REF | KSIG_INVALID;
-
 	case MEM_OBJ_STACK:
-		if (allow_invalid || reg.offset < (*(DataStack *)mobj).nr * sizeof(reg_t))
-			return KSIG_REF;
-		else
-			return KSIG_REF | KSIG_INVALID;
-
 	case MEM_OBJ_SYS_STRINGS:
-		if (allow_invalid || (reg.offset < SYS_STRINGS_MAX
-		                      && (*(SystemStrings *)mobj).strings[reg.offset].name))
-			return KSIG_REF;
-		else
-			return KSIG_REF | KSIG_INVALID;
+	case MEM_OBJ_DYNMEM:
+		type = KSIG_REF;
+		break;
 
 	case MEM_OBJ_LISTS:
-		if (allow_invalid || ((ListTable *)mobj)->isValidEntry(reg.offset))
-			return KSIG_LIST;
-		else
-			return KSIG_LIST | KSIG_INVALID;
+		type = KSIG_LIST;
+		break;
 
 	case MEM_OBJ_NODES:
-		if (allow_invalid || ((NodeTable *)mobj)->isValidEntry(reg.offset))
-			return KSIG_NODE;
-		else
-			return KSIG_NODE | KSIG_INVALID;
-
-	case MEM_OBJ_DYNMEM:
-		if (allow_invalid || reg.offset < (*(DynMem *)mobj)._size)
-			return KSIG_REF;
-		else
-			return KSIG_REF | KSIG_INVALID;
+		type = KSIG_NODE;
+		break;
 
 	default:
 		return 0;
-
 	}
+
+	if (!allow_invalid && !mobj->isValidOffset(reg.offset))
+		type |= KSIG_INVALID;
+	return type;
 }
 
 const char *kernel_argtype_description(int type) {
diff --git a/engines/sci/engine/memobj.cpp b/engines/sci/engine/memobj.cpp
index ef48270b41..c0775ae51e 100644
--- a/engines/sci/engine/memobj.cpp
+++ b/engines/sci/engine/memobj.cpp
@@ -89,21 +89,6 @@ void Script::freeScript() {
 	_codeBlocks.clear();
 }
 
-// memory operations
-
-void Script::mcpyInOut(int dst, const void *src, size_t n) {
-	if (buf) {
-		assert(dst + n <= buf_size);
-		memcpy(buf + dst, src, n);
-	}
-}
-
-int16 Script::getHeap(uint16 offset) const {
-	assert(offset + 1 < (int)buf_size);
-	return READ_LE_UINT16(buf + offset);
-//	return (buf[offset] | (buf[offset+1]) << 8);
-}
-
 void Script::incrementLockers() {
 	lockers++;
 }
@@ -147,12 +132,31 @@ int Script::getSynonymsNr() const {
 	return synonyms_nr;
 }
 
+// memory operations
+
+void Script::mcpyInOut(int dst, const void *src, size_t n) {
+	if (buf) {
+		assert(dst + n <= buf_size);
+		memcpy(buf + dst, src, n);
+	}
+}
+
+int16 Script::getHeap(uint16 offset) const {
+	assert(offset + 1 < (int)buf_size);
+	return READ_LE_UINT16(buf + offset);
+//	return (buf[offset] | (buf[offset+1]) << 8);
+}
+
 byte *MemObject::dereference(reg_t pointer, int *size) {
 	error("Error: Trying to dereference pointer %04x:%04x to inappropriate segment",
 		          PRINT_REG(pointer));
 	return NULL;
 }
 
+bool Script::isValidOffset(uint16 offset) const {
+	return offset < buf_size;
+}
+
 byte *Script::dereference(reg_t pointer, int *size) {
 	if (pointer.offset > buf_size) {
 		sciprintf("Error: Attempt to dereference invalid pointer %04x:%04x into script segment (script size=%d)\n",
@@ -161,42 +165,52 @@ byte *Script::dereference(reg_t pointer, int *size) {
 	}
 	if (size)
 		*size = buf_size - pointer.offset;
-	return (byte *)(buf + pointer.offset);
+	return buf + pointer.offset;
+}
+
+bool LocalVariables::isValidOffset(uint16 offset) const {
+	return offset < _locals.size() * sizeof(reg_t);
 }
 
 byte *LocalVariables::dereference(reg_t pointer, int *size) {
+	if (size)
+		*size = _locals.size() * sizeof(reg_t);
+
 	// FIXME: The following doesn't seem to be endian safe.
 	// To fix this, we'd have to always treat the reg_t
 	// values stored here as in the little endian format.
-	int count = _locals.size() * sizeof(reg_t);
 	byte *base = (byte *)&_locals[0];
-
-	if (size)
-		*size = count;
-
 	return base + pointer.offset;
 }
 
-byte *DataStack::dereference(reg_t pointer, int *size) {
-	int count = nr * sizeof(reg_t);
-	byte *base = (byte *)entries;
+bool DataStack::isValidOffset(uint16 offset) const {
+	return offset < nr * sizeof(reg_t);
+}
 
+byte *DataStack::dereference(reg_t pointer, int *size) {
 	if (size)
-		*size = count;
+		*size = nr * sizeof(reg_t);
 
+	byte *base = (byte *)entries;
 	return base + pointer.offset;
 }
 
-byte *DynMem::dereference(reg_t pointer, int *size) {
-	int count = _size;
-	byte *base = (byte *)_buf;
+bool DynMem::isValidOffset(uint16 offset) const {
+	return offset < _size;
+}
 
+byte *DynMem::dereference(reg_t pointer, int *size) {
 	if (size)
-		*size = count;
+		*size = _size;
 
+	byte *base = (byte *)_buf;
 	return base + pointer.offset;
 }
 
+bool SystemStrings::isValidOffset(uint16 offset) const {
+	return offset < SYS_STRINGS_MAX && strings[offset].name;
+}
+
 byte *SystemStrings::dereference(reg_t pointer, int *size) {
 	if (size)
 		*size = strings[pointer.offset].max_size;
diff --git a/engines/sci/engine/memobj.h b/engines/sci/engine/memobj.h
index a871d2dcb6..7b9c2e507a 100644
--- a/engines/sci/engine/memobj.h
+++ b/engines/sci/engine/memobj.h
@@ -65,10 +65,16 @@ public:
 	inline int getSegMgrId() const { return _segmgrId; }
 
 	/**
+	 * Check whether the given offset into this memory object is valid,
+	 * i.e., suitable for passing to dereference.
+	 */
+	virtual bool isValidOffset(uint16 offset) const = 0;
+
+	/**
 	 * Dereferences a raw memory pointer.
-	 * @param reg   reference to dereference
-	 * @param size  if not NULL, set to the theoretical maximum size of the referenced data block
-	 * @return              the data block referenced
+	 * @param reg	reference to dereference
+	 * @param size	if not NULL, set to the theoretical maximum size of the referenced data block
+	 * @return		the data block referenced
 	 */
 	virtual byte *dereference(reg_t pointer, int *size);
 
@@ -108,6 +114,8 @@ public:
 
 // TODO: Implement the following class
 struct StringFrag : public MemObject {
+	virtual bool isValidOffset(uint16 offset) const { return false; }
+
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 };
 
@@ -150,6 +158,7 @@ public:
 		}
 	}
 
+	virtual bool isValidOffset(uint16 offset) const;
 	virtual byte *dereference(reg_t pointer, int *size);
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
@@ -177,6 +186,7 @@ public:
 		script_id = 0;
 	}
 
+	virtual bool isValidOffset(uint16 offset) const;
 	virtual byte *dereference(reg_t pointer, int *size);
 	virtual reg_t findCanonicAddress(SegManager *segmgr, reg_t sub_addr);
 	virtual void listAllOutgoingReferences(EngineState *s, reg_t object, void *param, NoteCallback note);
@@ -286,6 +296,7 @@ public:
 
 	void freeScript();
 
+	virtual bool isValidOffset(uint16 offset) const;
 	virtual byte *dereference(reg_t pointer, int *size);
 	virtual reg_t findCanonicAddress(SegManager *segmgr, reg_t sub_addr);
 	virtual void freeAtAddress(SegManager *segmgr, reg_t sub_addr);
@@ -344,15 +355,6 @@ public:
 
 
 	/**
-	 * Copies a byte string into a script's heap representation.
-	 * @param dst	script-relative offset of the destination area
-	 * @param src	pointer to the data source location
-	 * @param n		number of bytes to copy
-	 */
-	void mcpyInOut(int dst, const void *src, size_t n);
-
-
-	/**
 	 * Marks the script as deleted.
 	 * This will not actually delete the script.  If references remain present on the
 	 * heap or the stack, the script will stay in memory in a quasi-deleted state until
@@ -378,12 +380,20 @@ public:
 	}
 
 	/**
+	 * Copies a byte string into a script's heap representation.
+	 * @param dst	script-relative offset of the destination area
+	 * @param src	pointer to the data source location
+	 * @param n		number of bytes to copy
+	 */
+	void mcpyInOut(int dst, const void *src, size_t n);
+
+
+	/**
 	 * Retrieves a 16 bit value from within a script's heap representation.
 	 * @param offset	offset to read from
 	 * @return the value read from the specified location
 	 */
 	int16 getHeap(uint16 offset) const;
-
 };
 
 /** Data stack */
@@ -401,6 +411,7 @@ public:
 		entries = NULL;
 	}
 
+	virtual bool isValidOffset(uint16 offset) const;
 	virtual byte *dereference(reg_t pointer, int *size);
 	virtual reg_t findCanonicAddress(SegManager *segmgr, reg_t sub_addr);
 	virtual void listAllOutgoingReferences(EngineState *s, reg_t object, void *param, NoteCallback note);
@@ -472,7 +483,11 @@ public:
 		}
 	}
 
-	bool isValidEntry(int idx) {
+	virtual bool isValidOffset(uint16 offset) const {
+		return isValidEntry(offset);
+	}
+
+	bool isValidEntry(int idx) const {
 		return idx >= 0 && (uint)idx < _table.size() && _table[idx].next_free == idx;
 	}
 
@@ -543,6 +558,7 @@ public:
 		_buf = NULL;
 	}
 
+	virtual bool isValidOffset(uint16 offset) const;
 	virtual byte *dereference(reg_t pointer, int *size);
 	virtual reg_t findCanonicAddress(SegManager *segmgr, reg_t sub_addr);
 	virtual void listAllDeallocatable(SegmentId segId, void *param, NoteCallback note);