- Removed the toggle for "weak" validations, as there are cases where they fail (e.g. in Longbow), so there is no point in having strict validations

- Removed the invalid toggle from kernel signatures - we now never error out on invalid object references, but throw a warning instead
- Simplified determine_reg_type()

svn-id: r44511

5 files changed, 31 insertions, 90 deletions

diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 770885f3d3..ee300d251d 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -49,7 +49,6 @@ namespace Sci {
 int g_debug_sleeptime_factor = 1;
 int g_debug_simulated_key = 0;
 bool g_debug_track_mouse_clicks = false;
-bool g_debug_weak_validations = true;
 
 Console::Console(SciEngine *vm) : GUI::Debugger() {
 	_vm = vm;
@@ -59,7 +58,6 @@ Console::Console(SciEngine *vm) : GUI::Debugger() {
 	DVar_Register("gc_interval",		&script_gc_interval, DVAR_INT, 0);
 	DVar_Register("simulated_key",		&g_debug_simulated_key, DVAR_INT, 0);
 	DVar_Register("track_mouse_clicks",	&g_debug_track_mouse_clicks, DVAR_BOOL, 0);
-	DVar_Register("weak_validations",	&g_debug_weak_validations, DVAR_BOOL, 0);
 	DVar_Register("script_abort_flag",	&script_abort_flag, DVAR_INT, 0);
 
 	// General
@@ -1759,13 +1757,9 @@ bool Console::cmdValueType(int argc, const char **argv) {
 		return true;
 	}
 
-	int t = determine_reg_type(_vm->_gamestate->segMan, val, true);
-	int invalid = t & KSIG_INVALID;
+	int t = determine_reg_type(_vm->_gamestate->segMan, val);
 
-	switch (t & ~KSIG_INVALID) {
-	case 0:
-		DebugPrintf("Invalid");
-		break;
+	switch (t) {
 	case KSIG_LIST:
 		DebugPrintf("List");
 		break;
@@ -1782,8 +1776,6 @@ bool Console::cmdValueType(int argc, const char **argv) {
 		DebugPrintf("Erroneous unknown type %02x(%d decimal)\n", t, t);
 	}
 
-	DebugPrintf("%s\n", invalid ? " (invalid)" : "");
-
 	return true;
 }
 
@@ -1834,13 +1826,11 @@ bool Console::cmdViewReference(int argc, const char **argv) {
 		}
 	}
 
-	int type_mask = determine_reg_type(_vm->_gamestate->segMan, reg, 1);
+	int type_mask = determine_reg_type(_vm->_gamestate->segMan, reg);
 	int filter;
 	int found = 0;
 
-	DebugPrintf("%04x:%04x is of type 0x%x%s: ", PRINT_REG(reg), type_mask & ~KSIG_INVALID, type_mask & KSIG_INVALID ? " (invalid)" : "");
-
-	type_mask &= ~KSIG_INVALID;
+	DebugPrintf("%04x:%04x is of type 0x%x: ", PRINT_REG(reg), type_mask);
 
 	if (reg.segment == 0 && reg.offset == 0) {
 		DebugPrintf("Null.\n");
diff --git a/engines/sci/debug.h b/engines/sci/debug.h
index f331c1e7ed..dbc929e591 100644
--- a/engines/sci/debug.h
+++ b/engines/sci/debug.h
@@ -55,7 +55,6 @@ struct DebugState {
 extern int g_debug_sleeptime_factor;
 extern int g_debug_simulated_key;
 extern bool g_debug_track_mouse_clicks;
-extern bool g_debug_weak_validations;
 extern DebugState g_debugState;
 
 } // End of namespace Sci
diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index d55f7d0da7..3e4506e6e9 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -246,9 +246,9 @@ SciKernelFunction kfunct_mappers[] = {
 	/*35*/	DEFUN("FirstNode", kFirstNode, "Zl"),
 	/*36*/	DEFUN("LastNode", kLastNode, "l"),
 	/*37*/	DEFUN("EmptyList", kEmptyList, "l"),
-	/*38*/	DEFUN("NextNode", kNextNode, "n!"),
+	/*38*/	DEFUN("NextNode", kNextNode, "n"),
 	/*39*/	DEFUN("PrevNode", kPrevNode, "n"),
-	/*3a*/	DEFUN("NodeValue", kNodeValue, "Zn!"),
+	/*3a*/	DEFUN("NodeValue", kNodeValue, "Zn"),
 	/*3b*/	DEFUN("AddAfter", kAddAfter, "lnn"),
 	/*3c*/	DEFUN("AddToFront", kAddToFront, "ln"),
 	/*3d*/	DEFUN("AddToEnd", kAddToEnd, "ln"),
@@ -550,10 +550,6 @@ static void kernel_compile_signature(const char **s) {
 				v |= KSIG_ANY;
 				break;
 
-			case KSIG_SPEC_ALLOW_INV:
-				v |= KSIG_ALLOW_INV;
-				break;
-
 			case KSIG_SPEC_ELLIPSIS:
 				v |= KSIG_ELLIPSIS;
 				ellipsis = 1;
@@ -564,7 +560,7 @@ static void kernel_compile_signature(const char **s) {
 				          " '%c')\n", *s, c, c);
 			}
 			}
-		} while (*src && (*src == KSIG_SPEC_ALLOW_INV || *src == KSIG_SPEC_ELLIPSIS || (c < 'a' && c != KSIG_SPEC_ANY)));
+		} while (*src && (*src == KSIG_SPEC_ELLIPSIS || (c < 'a' && c != KSIG_SPEC_ANY)));
 
 		// To handle sum types
 		result[index++] = v;
@@ -636,66 +632,41 @@ void Kernel::mapFunctions() {
 	return;
 }
 
-int determine_reg_type(SegManager *segMan, reg_t reg, bool allow_invalid) {
-	SegmentObj *mobj;
-	int type = 0;
-
-	if (!reg.segment) {
-		type = KSIG_ARITHMETIC;
-		if (!reg.offset)
-			type |= KSIG_NULL;
+int determine_reg_type(SegManager *segMan, reg_t reg) {
+	// No segment? Must be arithmetic
+	if (!reg.segment)
+		return reg.offset ? KSIG_ARITHMETIC : KSIG_ARITHMETIC | KSIG_NULL;
 
-		return type;
-	}
-
-	mobj = segMan->getSegmentObj(reg.segment);
+	// Otherwise it's an object
+	SegmentObj *mobj = segMan->getSegmentObj(reg.segment);
 	if (!mobj)
 		return 0; // Invalid
 
+	if (!mobj->isValidOffset(reg.offset))
+		warning("[KERN] ref %04x:%04x is invalid", PRINT_REG(reg));
+
 	switch (mobj->getType()) {
 	case SEG_TYPE_SCRIPT:
-		if (reg.offset <= (*(Script *)mobj)._bufSize && reg.offset >= -SCRIPT_OBJECT_MAGIC_OFFSET
-		        && RAW_IS_OBJECT((*(Script *)mobj)._buf + reg.offset)) {
-			Object *obj = ((Script *)mobj)->getObject(reg.offset);
-			if (obj)
-				return KSIG_OBJECT;
-			else
-				return KSIG_REF;
+		if (reg.offset <= (*(Script *)mobj)._bufSize && 
+			reg.offset >= -SCRIPT_OBJECT_MAGIC_OFFSET &&
+		    RAW_IS_OBJECT((*(Script *)mobj)._buf + reg.offset)) {
+			return ((Script *)mobj)->getObject(reg.offset) ? KSIG_OBJECT : KSIG_REF;
 		} else
 			return KSIG_REF;
-
 	case SEG_TYPE_CLONES:
-		type = KSIG_OBJECT;
-		break;
-
+		return KSIG_OBJECT;
 	case SEG_TYPE_LOCALS:
 	case SEG_TYPE_STACK:
 	case SEG_TYPE_SYS_STRINGS:
 	case SEG_TYPE_DYNMEM:
-		type = KSIG_REF;
-		break;
-
+		return KSIG_REF;
 	case SEG_TYPE_LISTS:
-		type = KSIG_LIST;
-		break;
-
+		return KSIG_LIST;
 	case SEG_TYPE_NODES:
-		type = KSIG_NODE;
-		break;
-
+		return KSIG_NODE;
 	default:
 		return 0;
 	}
-
-	if (!allow_invalid && !mobj->isValidOffset(reg.offset))
-		type |= KSIG_INVALID;
-	return type;
-}
-
-const char *kernel_argtype_description(int type) {
-	type &= ~KSIG_INVALID;
-
-	return argtype_description[sci_ffs(type)];
 }
 
 bool kernel_matches_signature(SegManager *segMan, const char *sig, int argc, const reg_t *argv) {
@@ -705,19 +676,13 @@ bool kernel_matches_signature(SegManager *segMan, const char *sig, int argc, con
 
 	while (*sig && argc) {
 		if ((*sig & KSIG_ANY) != KSIG_ANY) {
-			int type = determine_reg_type(segMan, *argv, *sig & KSIG_ALLOW_INV);
+			int type = determine_reg_type(segMan, *argv);
 
 			if (!type) {
 				warning("[KERN] Could not determine type of ref %04x:%04x; failing signature check", PRINT_REG(*argv));
 				return false;
 			}
 
-			if (type & KSIG_INVALID) {
-				warning("[KERN] ref %04x:%04x was determined to be a %s, but the reference itself is invalid",
-				          PRINT_REG(*argv), kernel_argtype_description(type));
-				return false;
-			}
-
 			if (!(type & *sig))
 				return false;
 
diff --git a/engines/sci/engine/kernel_types.h b/engines/sci/engine/kernel_types.h
index 609ded11ca..a8da4fead1 100644
--- a/engines/sci/engine/kernel_types.h
+++ b/engines/sci/engine/kernel_types.h
@@ -41,7 +41,6 @@ namespace Sci {
 #define KSIG_SPEC_ARITHMETIC 'i'
 #define KSIG_SPEC_NULL 'z'
 #define KSIG_SPEC_ANY '.'
-#define KSIG_SPEC_ALLOW_INV '!' // Allow invalid pointers
 #define KSIG_SPEC_ELLIPSIS '*' // Arbitrarily more TYPED arguments
 
 #define KSIG_SPEC_SUM_DONE ('a' - 'A') // Use small letters to indicate end of sum type
@@ -61,8 +60,6 @@ namespace Sci {
 #define KSIG_NULL	0x40
 #define KSIG_ANY	0x5f
 #define KSIG_ELLIPSIS	0x80
-#define KSIG_ALLOW_INV  0x20
-#define KSIG_INVALID	KSIG_ALLOW_INV
 
 /**
  * Determines whether a list of registers matches a given signature.
@@ -81,12 +78,11 @@ bool kernel_matches_signature(SegManager *segMan, const char *sig, int argc, con
  * Determines the type of the object indicated by reg.
  * @param segMan			the Segment manager
  * @param reg				register to check
- * @param allow_invalid		determines whether invalid pointer (=offset) values are allowed
- * @return one of KSIG_* below KSIG_NULL.
+  * @return one of KSIG_* below KSIG_NULL.
  *	       KSIG_INVALID set if the type of reg can be determined, but is invalid.
  *	       0 on error.
  */
-int determine_reg_type(SegManager *segMan, reg_t reg, bool allow_invalid);
+int determine_reg_type(SegManager *segMan, reg_t reg);
 
 /**
  * Returns a textual description of the type of an object.
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index b6a474cb8f..a4ca058bbd 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -28,7 +28,7 @@
 
 #include "sci/sci.h"
 #include "sci/console.h"
-#include "sci/debug.h"	// for g_debug_weak_validations
+#include "sci/debug.h"	// for g_debugState
 #include "sci/resource.h"
 #include "sci/engine/state.h"
 #include "sci/engine/kernel.h"
@@ -88,10 +88,7 @@ static StackPtr validate_stack_addr(EngineState *s, StackPtr sp) {
 
 static int validate_arithmetic(reg_t reg) {
 	if (reg.segment) {
-		if (g_debug_weak_validations)
-			warning("[VM] Attempt to read arithmetic value from non-zero segment [%04x]", reg.segment);
-		else
-			error("[VM] Attempt to read arithmetic value from non-zero segment [%04x]", reg.segment);
+		warning("[VM] Attempt to read arithmetic value from non-zero segment [%04x]", reg.segment);
 		return 0;
 	}
 
@@ -100,10 +97,7 @@ static int validate_arithmetic(reg_t reg) {
 
 static int signed_validate_arithmetic(reg_t reg) {
 	if (reg.segment) {
-		if (g_debug_weak_validations)
-			warning("[VM] Attempt to read arithmetic value from non-zero segment [%04x]", reg.segment);
-		else
-			error("[VM] Attempt to read arithmetic value from non-zero segment [%04x]", reg.segment);
+		warning("[VM] Attempt to read arithmetic value from non-zero segment [%04x]", reg.segment);
 		return 0;
 	}
 
@@ -127,10 +121,7 @@ static int validate_variable(reg_t *r, reg_t *stack_base, int type, int max, int
 			strcat(txt, tmp);
 		}
 
-		if (g_debug_weak_validations)
-			warning("%s", txt);
-		else
-			error("%s", txt);
+		warning("%s", txt);
 
 #ifdef STRICT_READ
 		return 1;