SCI: Add some FIXMEs, and print warning if accessing a raw segment as non-raw or vice versa

svn-id: r44245

4 files changed, 18 insertions, 5 deletions

diff --git a/engines/sci/engine/kstring.cpp b/engines/sci/engine/kstring.cpp
index d6161ca954..5cd4aba54d 100644
--- a/engines/sci/engine/kstring.cpp
+++ b/engines/sci/engine/kstring.cpp
@@ -789,6 +789,8 @@ reg_t kMessage(EngineState *s, int, int argc, reg_t *argv) {
 		byte *buffer = s->segMan->derefBulkPtr(argv[1], 10);
 
 		if (buffer) {
+			// FIXME: Is this correct? I.e., do we really write into a "raw" segment
+			// here? Or maybe we want to write 4 reg_t instead?
 			WRITE_LE_UINT16(buffer, module);
 			WRITE_LE_UINT16(buffer + 2, msg.noun);
 			WRITE_LE_UINT16(buffer + 4, msg.verb);
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index e7e8d4197b..dbc6c64917 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -876,12 +876,19 @@ SegmentRef SegManager::dereference(reg_t pointer) {
 	return mobj->dereference(pointer);
 }
 
-static void *_kernel_dereference_pointer(SegManager *segMan, reg_t pointer, int entries) {
+static void *_kernel_dereference_pointer(SegManager *segMan, reg_t pointer, int entries, bool wantRaw) {
 	SegmentRef ret = segMan->dereference(pointer);
 
 	if (!ret.raw)
 		return NULL;
 
+	if (ret.isRaw != wantRaw) {
+		warning("Dereferencing pointer %04x:%04x (type %d) which is %s, but expected %s", PRINT_REG(pointer),
+			segMan->_heap[pointer.segment]->getType(),
+			ret.isRaw ? "raw" : "not raw",
+			wantRaw ? "raw" : "not raw");
+	}
+
 	if (entries > ret.maxSize) {
 		warning("Trying to dereference pointer %04x:%04x beyond end of segment", PRINT_REG(pointer));
 		return NULL;
@@ -891,7 +898,7 @@ static void *_kernel_dereference_pointer(SegManager *segMan, reg_t pointer, int
 }
 
 byte *SegManager::derefBulkPtr(reg_t pointer, int entries) {
-	return (byte *)_kernel_dereference_pointer(this, pointer, entries);
+	return (byte *)_kernel_dereference_pointer(this, pointer, entries, true);
 }
 
 reg_t *SegManager::derefRegPtr(reg_t pointer, int entries) {
@@ -901,11 +908,11 @@ reg_t *SegManager::derefRegPtr(reg_t pointer, int entries) {
 		return NULL;
 	}
 
-	return (reg_t *)_kernel_dereference_pointer(this, pointer, entries);
+	return (reg_t *)_kernel_dereference_pointer(this, pointer, entries, false);
 }
 
 char *SegManager::derefString(reg_t pointer, int entries) {
-	return (char *)_kernel_dereference_pointer(this, pointer, entries);
+	return (char *)_kernel_dereference_pointer(this, pointer, entries, true);
 }
 
 
diff --git a/engines/sci/engine/segment.cpp b/engines/sci/engine/segment.cpp
index 7d2889aefc..bf4b8bfea0 100644
--- a/engines/sci/engine/segment.cpp
+++ b/engines/sci/engine/segment.cpp
@@ -233,6 +233,10 @@ SegmentRef Script::dereference(reg_t pointer) {
 
 	SegmentRef ret;
 	ret.isRaw = true;
+	// FIXME: here we subtract pointer.offset from maxSize, elsewhere we don't -- huh?
+	// It's hard to say which is right, because it depends on how the various
+	// SegManager::deref*() methods are used. It's quite possible that this variant
+	// is "correct" and all the others aren't... we'll have to find out.
 	ret.maxSize = _bufSize - pointer.offset;
 	ret.raw = _buf + pointer.offset;
 	return ret;
diff --git a/engines/sci/vocabulary.cpp b/engines/sci/vocabulary.cpp
index 60e3334af0..c949dfca40 100644
--- a/engines/sci/vocabulary.cpp
+++ b/engines/sci/vocabulary.cpp
@@ -345,7 +345,7 @@ ResultWord Vocabulary::lookupWord(const char *word, int word_len) {
 }
 
 void Vocabulary::decipherSaidBlock(byte *addr) {
-	int nextitem;
+	byte nextitem;
 
 	do {
 		nextitem = *addr++;