SCI: Revise GC interface: use Common::Array<reg_t> instead of callbacks

This means a little bit more overhead but makes the code much more readable
and understandable.

svn-id: r50429

5 files changed, 109 insertions, 71 deletions

diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index f682e0e524..8ea7daa7c6 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -1847,11 +1847,6 @@ bool Console::cmdGCObjects(int argc, const char **argv) {
 	return true;
 }
 
-void _print_address(void * _, reg_t addr) {
-	if (addr.segment)
-		g_sci->getSciDebugger()->DebugPrintf("  %04x:%04x\n", PRINT_REG(addr));
-}
-
 bool Console::cmdGCShowReachable(int argc, const char **argv) {
 	if (argc != 2) {
 		DebugPrintf("Prints all addresses directly reachable from the memory object specified as parameter.\n");
@@ -1875,7 +1870,10 @@ bool Console::cmdGCShowReachable(int argc, const char **argv) {
 	}
 
 	DebugPrintf("Reachable from %04x:%04x:\n", PRINT_REG(addr));
-	mobj->listAllOutgoingReferences(addr, NULL, _print_address);
+	const Common::Array<reg_t> tmp = mobj->listAllOutgoingReferences(addr);
+	for (Common::Array<reg_t>::const_iterator it = tmp.begin(); it != tmp.end(); ++it)
+		if (it->segment)
+			g_sci->getSciDebugger()->DebugPrintf("  %04x:%04x\n", PRINT_REG(*it));
 
 	return true;
 }
@@ -1904,7 +1902,10 @@ bool Console::cmdGCShowFreeable(int argc, const char **argv) {
 	}
 
 	DebugPrintf("Freeable in segment %04x:\n", addr.segment);
-	mobj->listAllDeallocatable(addr.segment, NULL, _print_address);
+	const Common::Array<reg_t> tmp = mobj->listAllDeallocatable(addr.segment);
+	for (Common::Array<reg_t>::const_iterator it = tmp.begin(); it != tmp.end(); ++it)
+		if (it->segment)
+			g_sci->getSciDebugger()->DebugPrintf("  %04x:%04x\n", PRINT_REG(*it));
 
 	return true;
 }
diff --git a/engines/sci/engine/gc.cpp b/engines/sci/engine/gc.cpp
index c2f1c15776..59f62c8996 100644
--- a/engines/sci/engine/gc.cpp
+++ b/engines/sci/engine/gc.cpp
@@ -46,6 +46,11 @@ struct WorklistManager {
 		_map.setVal(reg, true);
 		_worklist.push_back(reg);
 	}
+
+	void push(const Common::Array<reg_t> &tmp) {
+		for (Common::Array<reg_t>::const_iterator it = tmp.begin(); it != tmp.end(); ++it)
+			push(*it);
+	}
 };
 
 static reg_t_hash_map *normalise_hashmap_ptrs(SegManager *segMan, reg_t_hash_map &nonnormal_map) {
@@ -65,11 +70,6 @@ static reg_t_hash_map *normalise_hashmap_ptrs(SegManager *segMan, reg_t_hash_map
 }
 
 
-void add_outgoing_refs(void *refcon, reg_t addr) {
-	WorklistManager *wm = (WorklistManager *)refcon;
-	wm->push(addr);
-}
-
 reg_t_hash_map *find_all_used_references(EngineState *s) {
 	SegManager *segMan = s->_segMan;
 	reg_t_hash_map *normal_map = NULL;
@@ -125,15 +125,7 @@ reg_t_hash_map *find_all_used_references(EngineState *s) {
 			Script *script = (Script *)segMan->_heap[i];
 
 			if (script->getLockers()) { // Explicitly loaded?
-				// Locals, if present
-				wm.push(make_reg(script->_localsSegment, 0));
-
-				// All objects (may be classes, may be indirectly reachable)
-				ObjMap::iterator it;
-				const ObjMap::iterator end = script->_objects.end();
-				for (it = script->_objects.begin(); it != end; ++it) {
-					wm.push(it->_value.getPos());
-				}
+				wm.push(script->listObjectReferences());
 			}
 		}
 
@@ -146,8 +138,10 @@ reg_t_hash_map *find_all_used_references(EngineState *s) {
 		wm._worklist.pop_back();
 		if (reg.segment != stack_seg) { // No need to repeat this one
 			debugC(2, kDebugLevelGC, "[GC] Checking %04x:%04x", PRINT_REG(reg));
-			if (reg.segment < segMan->_heap.size() && segMan->_heap[reg.segment])
-				segMan->_heap[reg.segment]->listAllOutgoingReferences(reg, &wm, add_outgoing_refs);
+			if (reg.segment < segMan->_heap.size() && segMan->_heap[reg.segment]) {
+				// Valid heap object? Find its outgoing references!
+				wm.push(segMan->_heap[reg.segment]->listAllOutgoingReferences(reg));
+			}
 		}
 	}
 
@@ -167,8 +161,7 @@ struct deallocator_t {
 	reg_t_hash_map *use_map;
 };
 
-void free_unless_used(void *refcon, reg_t addr) {
-	deallocator_t *deallocator = (deallocator_t *)refcon;
+static void free_unless_used(deallocator_t *deallocator, reg_t addr) {
 	reg_t_hash_map *use_map = deallocator->use_map;
 
 	if (!use_map->contains(addr)) {
@@ -201,7 +194,11 @@ void run_gc(EngineState *s) {
 #ifdef DEBUG_GC
 			deallocator.segnames[deallocator.mobj->getType()] = deallocator.mobj->type;	// FIXME: add a segment "name"
 #endif
-			deallocator.mobj->listAllDeallocatable(seg_nr, &deallocator, free_unless_used);
+			const Common::Array<reg_t> tmp = deallocator.mobj->listAllDeallocatable(seg_nr);
+			for (Common::Array<reg_t>::const_iterator it = tmp.begin(); it != tmp.end(); ++it) {
+				free_unless_used(&deallocator, *it);
+			}
+
 		}
 	}
 
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index aab073f3a1..490a7a81a3 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -104,8 +104,10 @@ public:
 	virtual SegmentRef dereference(reg_t pointer);
 	virtual reg_t findCanonicAddress(SegManager *segMan, reg_t sub_addr) const;
 	virtual void freeAtAddress(SegManager *segMan, reg_t sub_addr);
-	virtual void listAllDeallocatable(SegmentId segId, void *param, NoteCallback note) const;
-	virtual void listAllOutgoingReferences(reg_t object, void *param, NoteCallback note) const;
+	virtual Common::Array<reg_t> listAllDeallocatable(SegmentId segId) const;
+	virtual Common::Array<reg_t> listAllOutgoingReferences(reg_t object) const;
+
+	Common::Array<reg_t> listObjectReferences() const;
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 
diff --git a/engines/sci/engine/segment.cpp b/engines/sci/engine/segment.cpp
index 2bbedf2dcf..f8a941fa7a 100644
--- a/engines/sci/engine/segment.cpp
+++ b/engines/sci/engine/segment.cpp
@@ -218,20 +218,22 @@ void Script::freeAtAddress(SegManager *segMan, reg_t addr) {
 		segMan->deallocateScript(_nr);
 }
 
-void Script::listAllDeallocatable(SegmentId segId, void *param, NoteCallback note) const {
-	(*note)(param, make_reg(segId, 0));
+Common::Array<reg_t> Script::listAllDeallocatable(SegmentId segId) const {
+	const reg_t r = make_reg(segId, 0);
+	return Common::Array<reg_t>(&r, 1);
 }
 
-void Script::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback note) const {
+Common::Array<reg_t> Script::listAllOutgoingReferences(reg_t addr) const {
+	Common::Array<reg_t> tmp;
 	if (addr.offset <= _bufSize && addr.offset >= -SCRIPT_OBJECT_MAGIC_OFFSET && RAW_IS_OBJECT(_buf + addr.offset)) {
 		const Object *obj = getObject(addr.offset);
 		if (obj) {
 			// Note all local variables, if we have a local variable environment
 			if (_localsSegment)
-				(*note)(param, make_reg(_localsSegment, 0));
+				tmp.push_back(make_reg(_localsSegment, 0));
 
 			for (uint i = 0; i < obj->getVarCount(); i++)
-				(*note)(param, obj->getVariable(i));
+				tmp.push_back(obj->getVariable(i));
 		} else {
 			error("Request for outgoing script-object reference at %04x:%04x failed", PRINT_REG(addr));
 		}
@@ -239,12 +241,30 @@ void Script::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback not
 		/*		warning("Unexpected request for outgoing script-object references at %04x:%04x", PRINT_REG(addr));*/
 		/* Happens e.g. when we're looking into strings */
 	}
+	return tmp;
 }
 
+Common::Array<reg_t> Script::listObjectReferences() const {
+	Common::Array<reg_t> tmp;
+
+	// Locals, if present
+	if (_localsSegment)
+		tmp.push_back(make_reg(_localsSegment, 0));
+
+	// All objects (may be classes, may be indirectly reachable)
+	ObjMap::iterator it;
+	const ObjMap::iterator end = _objects.end();
+	for (it = _objects.begin(); it != end; ++it) {
+		tmp.push_back(it->_value.getPos());
+	}
+
+	return tmp;
+}
 
 //-------------------- clones --------------------
 
-void CloneTable::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback note) const {
+Common::Array<reg_t> CloneTable::listAllOutgoingReferences(reg_t addr) const {
+	Common::Array<reg_t> tmp;
 //	assert(addr.segment == _segId);
 
 	if (!isValidEntry(addr.offset)) {
@@ -255,11 +275,13 @@ void CloneTable::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback
 
 	// Emit all member variables (including references to the 'super' delegate)
 	for (uint i = 0; i < clone->getVarCount(); i++)
-		(*note)(param, clone->getVariable(i));
+		tmp.push_back(clone->getVariable(i));
 
 	// Note that this also includes the 'base' object, which is part of the script and therefore also emits the locals.
-	(*note)(param, clone->getPos());
+	tmp.push_back(clone->getPos());
 	//debugC(2, kDebugLevelGC, "[GC] Reporting clone-pos %04x:%04x", PRINT_REG(clone->pos));
+
+	return tmp;
 }
 
 void CloneTable::freeAtAddress(SegManager *segMan, reg_t addr) {
@@ -293,11 +315,14 @@ reg_t LocalVariables::findCanonicAddress(SegManager *segMan, reg_t addr) const {
 	return make_reg(owner_seg, 0);
 }
 
-void LocalVariables::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback note) const {
+Common::Array<reg_t> LocalVariables::listAllOutgoingReferences(reg_t addr) const {
+	Common::Array<reg_t> tmp;
 //	assert(addr.segment == _segId);
 
 	for (uint i = 0; i < _locals.size(); i++)
-		(*note)(param, _locals[i]);
+		tmp.push_back(_locals[i]);
+
+	return tmp;
 }
 
 
@@ -307,11 +332,14 @@ reg_t DataStack::findCanonicAddress(SegManager *segMan, reg_t addr) const {
 	return addr;
 }
 
-void DataStack::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback note) const {
+Common::Array<reg_t> DataStack::listAllOutgoingReferences(reg_t object) const {
+	Common::Array<reg_t> tmp;
 	fprintf(stderr, "Emitting %d stack entries\n", _capacity);
 	for (int i = 0; i < _capacity; i++)
-		(*note)(param, _entries[i]);
+		tmp.push_back(_entries[i]);
 	fprintf(stderr, "DONE");
+
+	return tmp;
 }
 
 
@@ -320,18 +348,20 @@ void ListTable::freeAtAddress(SegManager *segMan, reg_t sub_addr) {
 	freeEntry(sub_addr.offset);
 }
 
-void ListTable::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback note) const {
+Common::Array<reg_t> ListTable::listAllOutgoingReferences(reg_t addr) const {
+	Common::Array<reg_t> tmp;
 	if (!isValidEntry(addr.offset)) {
 		error("Invalid list referenced for outgoing references: %04x:%04x", PRINT_REG(addr));
-		return;
 	}
 
 	const List *list = &(_table[addr.offset]);
 
-	note(param, list->first);
-	note(param, list->last);
+	tmp.push_back(list->first);
+	tmp.push_back(list->last);
 	// We could probably get away with just one of them, but
 	// let's be conservative here.
+
+	return tmp;
 }
 
 
@@ -340,19 +370,21 @@ void NodeTable::freeAtAddress(SegManager *segMan, reg_t sub_addr) {
 	freeEntry(sub_addr.offset);
 }
 
-void NodeTable::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback note) const {
+Common::Array<reg_t> NodeTable::listAllOutgoingReferences(reg_t addr) const {
+	Common::Array<reg_t> tmp;
 	if (!isValidEntry(addr.offset)) {
 		error("Invalid node referenced for outgoing references: %04x:%04x", PRINT_REG(addr));
-		return;
 	}
 	const Node *node = &(_table[addr.offset]);
 
 	// We need all four here. Can't just stick with 'pred' OR 'succ' because node operations allow us
 	// to walk around from any given node
-	note(param, node->pred);
-	note(param, node->succ);
-	note(param, node->key);
-	note(param, node->value);
+	tmp.push_back(node->pred);
+	tmp.push_back(node->succ);
+	tmp.push_back(node->key);
+	tmp.push_back(node->value);
+
+	return tmp;
 }
 
 
@@ -473,8 +505,9 @@ reg_t DynMem::findCanonicAddress(SegManager *segMan, reg_t addr) const {
 	return addr;
 }
 
-void DynMem::listAllDeallocatable(SegmentId segId, void *param, NoteCallback note) const {
-	(*note)(param, make_reg(segId, 0));
+Common::Array<reg_t> DynMem::listAllDeallocatable(SegmentId segId) const {
+	const reg_t r = make_reg(segId, 0);
+	return Common::Array<reg_t>(&r, 1);
 }
 
 #ifdef ENABLE_SCI32
@@ -492,10 +525,10 @@ void ArrayTable::freeAtAddress(SegManager *segMan, reg_t sub_addr) {
 	freeEntry(sub_addr.offset);
 }
 
-void ArrayTable::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback note) const {
+Common::Array<reg_t> ArrayTable::listAllOutgoingReferences(reg_t addr) const {
+	Common::Array<reg_t> tmp;
 	if (!isValidEntry(addr.offset)) {
 		error("Invalid array referenced for outgoing references: %04x:%04x", PRINT_REG(addr));
-		return;
 	}
 
 	const SciArray<reg_t> *array = &(_table[addr.offset]);
@@ -503,8 +536,10 @@ void ArrayTable::listAllOutgoingReferences(reg_t addr, void *param, NoteCallback
 	for (uint32 i = 0; i < array->getSize(); i++) {
 		reg_t value = array->getValue(i);
 		if (value.segment != 0)
-			note(param, value);
+			tmp.push_back(value);
 	}
+
+	return tmp;
 }
 
 Common::String SciString::toString() const {
diff --git a/engines/sci/engine/segment.h b/engines/sci/engine/segment.h
index b621e53412..c12efecf1c 100644
--- a/engines/sci/engine/segment.h
+++ b/engines/sci/engine/segment.h
@@ -80,8 +80,6 @@ enum SegmentType {
 struct SegmentObj : public Common::Serializable {
 	SegmentType _type;
 
-	typedef void (*NoteCallback)(void *param, reg_t addr);	// FIXME: Bad choice of name
-
 public:
 	static SegmentObj *createSegmentObj(SegmentType type);
 
@@ -125,16 +123,19 @@ public:
 	 * @param note		Invoked for each address on which free_at_address() makes sense
 	 * @param param		parameter passed to 'note'
 	 */
-	virtual void listAllDeallocatable(SegmentId segId, void *param, NoteCallback note) const {}
+	virtual Common::Array<reg_t> listAllDeallocatable(SegmentId segId) const {
+		return Common::Array<reg_t>();
+	}
 
 	/**
 	 * Iterates over all references reachable from the specified object.
-	 * @param object	object (within the current segment) to analyse
-	 * @param param		parameter passed to 'note'
-	 * @param note		Invoked for each outgoing reference within the object
+	 * @param  object	object (within the current segment) to analyse
+	 * @return refs		a list of outgoing references within the object
 	 * Note: This function may also choose to report numbers (segment 0) as adresses
 	 */
-	virtual void listAllOutgoingReferences(reg_t object, void *param, NoteCallback note) const {}
+	virtual Common::Array<reg_t> listAllOutgoingReferences(reg_t object) const {
+		return Common::Array<reg_t>();
+	}
 };
 
 
@@ -195,7 +196,7 @@ public:
 	virtual bool isValidOffset(uint16 offset) const;
 	virtual SegmentRef dereference(reg_t pointer);
 	virtual reg_t findCanonicAddress(SegManager *segMan, reg_t sub_addr) const;
-	virtual void listAllOutgoingReferences(reg_t object, void *param, NoteCallback note) const;
+	virtual Common::Array<reg_t> listAllOutgoingReferences(reg_t object) const;
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 };
@@ -349,7 +350,7 @@ public:
 	virtual bool isValidOffset(uint16 offset) const;
 	virtual SegmentRef dereference(reg_t pointer);
 	virtual reg_t findCanonicAddress(SegManager *segMan, reg_t sub_addr) const;
-	virtual void listAllOutgoingReferences(reg_t object, void *param, NoteCallback note) const;
+	virtual Common::Array<reg_t> listAllOutgoingReferences(reg_t object) const;
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 };
@@ -437,10 +438,12 @@ public:
 		entries_used--;
 	}
 
-	virtual void listAllDeallocatable(SegmentId segId, void *param, NoteCallback note) const {
+	virtual Common::Array<reg_t> listAllDeallocatable(SegmentId segId) const {
+		Common::Array<reg_t> tmp;
 		for (uint i = 0; i < _table.size(); i++)
 			if (isValidEntry(i))
-				(*note)(param, make_reg(segId, i));
+				tmp.push_back(make_reg(segId, i));
+		return tmp;
 	}
 };
 
@@ -450,7 +453,7 @@ struct CloneTable : public Table<Clone> {
 	CloneTable() : Table<Clone>(SEG_TYPE_CLONES) {}
 
 	virtual void freeAtAddress(SegManager *segMan, reg_t sub_addr);
-	virtual void listAllOutgoingReferences(reg_t object, void *param, NoteCallback note) const;
+	virtual Common::Array<reg_t> listAllOutgoingReferences(reg_t object) const;
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 };
@@ -461,7 +464,7 @@ struct NodeTable : public Table<Node> {
 	NodeTable() : Table<Node>(SEG_TYPE_NODES) {}
 
 	virtual void freeAtAddress(SegManager *segMan, reg_t sub_addr);
-	virtual void listAllOutgoingReferences(reg_t object, void *param, NoteCallback note) const;
+	virtual Common::Array<reg_t> listAllOutgoingReferences(reg_t object) const;
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 };
@@ -472,7 +475,7 @@ struct ListTable : public Table<List> {
 	ListTable() : Table<List>(SEG_TYPE_LISTS) {}
 
 	virtual void freeAtAddress(SegManager *segMan, reg_t sub_addr);
-	virtual void listAllOutgoingReferences(reg_t object, void *param, NoteCallback note) const;
+	virtual Common::Array<reg_t> listAllOutgoingReferences(reg_t object) const;
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 };
@@ -511,7 +514,7 @@ public:
 	virtual bool isValidOffset(uint16 offset) const;
 	virtual SegmentRef dereference(reg_t pointer);
 	virtual reg_t findCanonicAddress(SegManager *segMan, reg_t sub_addr) const;
-	virtual void listAllDeallocatable(SegmentId segId, void *param, NoteCallback note) const;
+	virtual Common::Array<reg_t> listAllDeallocatable(SegmentId segId) const;
 
 	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 };
@@ -645,7 +648,7 @@ struct ArrayTable : public Table<SciArray<reg_t> > {
 	ArrayTable() : Table<SciArray<reg_t> >(SEG_TYPE_ARRAY) {}
 
 	virtual void freeAtAddress(SegManager *segMan, reg_t sub_addr);
-	virtual void listAllOutgoingReferences(reg_t object, void *param, NoteCallback note) const;
+	virtual Common::Array<reg_t> listAllOutgoingReferences(reg_t object) const;
 
 	void saveLoadWithSerializer(Common::Serializer &ser);
 	SegmentRef dereference(reg_t pointer);