SCI: Use Common::List and Common::String to simplify breakpoint handling (untested)

svn-id: r47824

9 files changed, 68 insertions, 133 deletions

diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 08d3692898..34a8562495 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -205,8 +205,8 @@ void Console::preEnter() {
 	if (_vm->_gamestate)
 		_vm->_gamestate->_sound.sfx_suspend(true);
 #endif
-	if (_vm->getEngineState() && _vm->getEngineState()->_soundCmd)
-		_vm->getEngineState()->_soundCmd->pauseAll(true);
+	if (_vm->_gamestate && _vm->_gamestate->_soundCmd)
+		_vm->_gamestate->_soundCmd->pauseAll(true);
 }
 
 void Console::postEnter() {
@@ -214,8 +214,8 @@ void Console::postEnter() {
 	if (_vm->_gamestate)
 		_vm->_gamestate->_sound.sfx_suspend(false);
 #endif
-	if (_vm->getEngineState() && _vm->getEngineState()->_soundCmd)
-		_vm->getEngineState()->_soundCmd->pauseAll(false);
+	if (_vm->_gamestate && _vm->_gamestate->_soundCmd)
+		_vm->_gamestate->_soundCmd->pauseAll(false);
 
 	if (!_videoFile.empty()) {
 		_vm->_gamestate->_gui->hideCursor();
@@ -409,7 +409,7 @@ const char *selector_name(EngineState *s, int selector) {
 bool Console::cmdGetVersion(int argc, const char **argv) {
 	const char *viewTypeDesc[] = { "Unknown", "EGA", "VGA", "VGA SCI1.1", "Amiga" };
 
-	EngineState *s = _vm->getEngineState();
+	EngineState *s = _vm->_gamestate;
 	bool hasVocab997 = s->resMan->testResource(ResourceId(kResourceTypeVocab, VOCAB_RESOURCE_SELECTORS)) ? true : false;
 
 	DebugPrintf("Game ID: %s\n", s->_gameId.c_str());
@@ -693,7 +693,7 @@ bool Console::cmdRoomNumber(int argc, const char **argv) {
 	} else {
 		Common::String roomNumberStr = argv[1];
 		int roomNumber = strtol(roomNumberStr.c_str(), NULL, roomNumberStr.hasSuffix("h") ? 16 : 10);
-		_vm->getEngineState()->setRoomNumber(roomNumber);
+		_vm->_gamestate->setRoomNumber(roomNumber);
 		DebugPrintf("Room number changed to %d (%x in hex)\n", roomNumber, roomNumber);
 	}
 
@@ -1446,7 +1446,7 @@ bool Console::cmdSongLib(int argc, const char **argv) {
 	} while (seeker);
 	DebugPrintf("\n");
 #else
-	_vm->getEngineState()->_soundCmd->printPlayList(this);
+	_vm->_gamestate->_soundCmd->printPlayList(this);
 #endif
 
 	return true;
@@ -1467,7 +1467,7 @@ bool Console::cmdSongInfo(int argc, const char **argv) {
 		return true;
 	}
 
-	_vm->getEngineState()->_soundCmd->printSongInfo(addr, this);
+	_vm->_gamestate->_soundCmd->printSongInfo(addr, this);
 
 	return true;
 }
@@ -1486,7 +1486,7 @@ bool Console::cmdStartSound(int argc, const char **argv) {
 		return true;
 	}
 
-	_vm->getEngineState()->_soundCmd->startNewSound(number);
+	_vm->_gamestate->_soundCmd->startNewSound(number);
 
 	return false;
 }
@@ -1590,7 +1590,7 @@ bool Console::cmdIsSample(int argc, const char **argv) {
 		return true;
 	}
 
-	SoundResource *soundRes = new SoundResource(number, _vm->getResourceManager(), _vm->getEngineState()->detectDoSoundType());
+	SoundResource *soundRes = new SoundResource(number, _vm->getResourceManager(), _vm->_gamestate->detectDoSoundType());
 
 	if (!soundRes) {
 		DebugPrintf("Not a sound resource!\n");
@@ -2367,25 +2367,25 @@ bool Console::cmdGo(int argc, const char **argv) {
 }
 
 bool Console::cmdBreakpointList(int argc, const char **argv) {
-	Breakpoint *bp = _vm->_gamestate->bp_list;
 	int i = 0;
 	int bpdata;
 
 	DebugPrintf("Breakpoint list:\n");
 
-	while (bp) {
+	Common::List<Breakpoint>::const_iterator bp = _vm->_gamestate->_breakpoints.begin();
+	Common::List<Breakpoint>::const_iterator end = _vm->_gamestate->_breakpoints.end();
+	for (; bp != end; ++bp) {
 		DebugPrintf("  #%i: ", i);
 		switch (bp->type) {
 		case BREAK_SELECTOR:
-			DebugPrintf("Execute %s\n", bp->data.name);
+			DebugPrintf("Execute %s\n", bp->name.c_str());
 			break;
 		case BREAK_EXPORT:
-			bpdata = bp->data.address;
+			bpdata = bp->address;
 			DebugPrintf("Execute script %d, export %d\n", bpdata >> 16, bpdata & 0xFFFF);
 			break;
 		}
 
-		bp = bp->next;
 		i++;
 	}
 
@@ -2399,45 +2399,30 @@ bool Console::cmdBreakpointDelete(int argc, const char **argv) {
 		return true;
 	}
 
-	Breakpoint *bp, *bp_next, *bp_prev;
-	int i = 0, found = 0;
-	int type;
-	int idx = atoi(argv[1]);
+	const int idx = atoi(argv[1]);
 
-	// Find breakpoint with given index
-	bp_prev = NULL;
-	bp = _vm->_gamestate->bp_list;
-	while (bp && i < idx) {
-		bp_prev = bp;
-		bp = bp->next;
-		i++;
+	// Find the breakpoint at index idx.
+	Common::List<Breakpoint>::iterator bp = _vm->_gamestate->_breakpoints.begin();
+	const Common::List<Breakpoint>::iterator end = _vm->_gamestate->_breakpoints.end();
+	for (int i = 0; bp != end && i < idx; ++bp, ++i) {
+		// do nothing
 	}
-	if (!bp) {
+
+	if (bp == end) {
 		DebugPrintf("Invalid breakpoint index %i\n", idx);
 		return true;
 	}
 
 	// Delete it
-	bp_next = bp->next;
-	type = bp->type;
-	if (type == BREAK_SELECTOR) free(bp->data.name);
-	free(bp);
-	if (bp_prev)
-		bp_prev->next = bp_next;
-	else
-		_vm->_gamestate->bp_list = bp_next;
+	_vm->_gamestate->_breakpoints.erase(bp);
 
-	// Check if there are more breakpoints of the same type. If not, clear
-	// the respective bit in s->have_bp.
-	for (bp = _vm->_gamestate->bp_list; bp; bp = bp->next) {
-		if (bp->type == type) {
-			found = 1;
-			break;
-		}
+	// Update EngineState::_activeBreakpointTypes.
+	int type = 0;
+	for (bp = _vm->_gamestate->_breakpoints.begin(); bp != end; ++bp) {
+		type |= bp->type;
 	}
 
-	if (!found)
-		_vm->_gamestate->have_bp &= ~type;
+	_vm->_gamestate->_activeBreakpointTypes = type;
 
 	return true;
 }
@@ -2455,24 +2440,12 @@ bool Console::cmdBreakpointExecMethod(int argc, const char **argv) {
 	/* Note: We can set a breakpoint on a method that has not been loaded yet.
 	   Thus, we can't check whether the command argument is a valid method name.
 	   A breakpoint set on an invalid method name will just never trigger. */
-	Breakpoint *bp;
-	if (_vm->_gamestate->bp_list) {
-		// List exists, append the breakpoint to the end
-		bp = _vm->_gamestate->bp_list;
-		while (bp->next)
-			bp = bp->next;
-		bp->next = (Breakpoint *)malloc(sizeof(Breakpoint));
-		bp = bp->next;
-	} else {
-		// No list, so create the list head
-		_vm->_gamestate->bp_list = (Breakpoint *)malloc(sizeof(Breakpoint));
-		bp = _vm->_gamestate->bp_list;
-	}
-	bp->next = NULL;
-	bp->type = BREAK_SELECTOR;
-	bp->data.name = (char *)malloc(strlen(argv[1]) + 1);
-	strcpy(bp->data.name, argv[1]);
-	_vm->_gamestate->have_bp |= BREAK_SELECTOR;
+	Breakpoint bp;
+	bp.type = BREAK_SELECTOR;
+	bp.name = argv[1];
+
+	_vm->_gamestate->_breakpoints.push_back(bp);
+	_vm->_gamestate->_activeBreakpointTypes |= BREAK_SELECTOR;
 
 	return true;
 }
@@ -2488,23 +2461,12 @@ bool Console::cmdBreakpointExecFunction(int argc, const char **argv) {
 	/* Note: We can set a breakpoint on a method that has not been loaded yet.
 	   Thus, we can't check whether the command argument is a valid method name.
 	   A breakpoint set on an invalid method name will just never trigger. */
-	Breakpoint *bp;
-	if (_vm->_gamestate->bp_list) {
-		// List exists, append the breakpoint to the end
-		bp = _vm->_gamestate->bp_list;
-		while (bp->next)
-			bp = bp->next;
-		bp->next = (Breakpoint *)malloc(sizeof(Breakpoint));
-		bp = bp->next;
-	} else {
-		// No list, so create the list head
-		_vm->_gamestate->bp_list = (Breakpoint *)malloc(sizeof(Breakpoint));
-		bp = _vm->_gamestate->bp_list;
-	}
-	bp->next = NULL;
-	bp->type = BREAK_EXPORT;
-	bp->data.address = (atoi(argv[1]) << 16 | atoi(argv[2]));
-	_vm->_gamestate->have_bp |= BREAK_EXPORT;
+	Breakpoint bp;
+	bp.type = BREAK_EXPORT;
+	bp.address = (atoi(argv[1]) << 16 | atoi(argv[2]));
+
+	_vm->_gamestate->_breakpoints.push_back(bp);
+	_vm->_gamestate->_activeBreakpointTypes |= BREAK_EXPORT;
 
 	return true;
 }
diff --git a/engines/sci/engine/game.cpp b/engines/sci/engine/game.cpp
index db2d776fbf..9572fb89c5 100644
--- a/engines/sci/engine/game.cpp
+++ b/engines/sci/engine/game.cpp
@@ -218,8 +218,8 @@ int script_init_engine(EngineState *s) {
 
 	s->restarting_flags = SCI_GAME_IS_NOT_RESTARTING;
 
-	s->bp_list = NULL; // No breakpoints defined
-	s->have_bp = 0;
+	s->_breakpoints.clear(); // No breakpoints defined
+	s->_activeBreakpointTypes = 0;
 
 	if (s->detectLofsType() == SCI_VERSION_1_MIDDLE)
 		s->_segMan->setExportAreWide(true);
@@ -231,22 +231,6 @@ int script_init_engine(EngineState *s) {
 	return 0;
 }
 
-void script_free_breakpoints(EngineState *s) {
-	Breakpoint *bp, *bp_next;
-
-	// Free breakpoint list
-	bp = s->bp_list;
-	while (bp) {
-		bp_next = bp->next;
-		if (bp->type == BREAK_SELECTOR)
-			free(bp->data.name);
-		free(bp);
-		bp = bp_next;
-	}
-
-	s->bp_list = NULL;
-}
-
 /*************************************************************/
 /* Game instance stuff: Init/Unitialize state-dependant data */
 /*************************************************************/
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 00b3fcf242..e8253bb93b 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -1003,8 +1003,8 @@ void gamestate_restore(EngineState *s, Common::SeekableReadStream *fh) {
 		retval->_voc->parser_base = make_reg(s->sys_strings_segment, SYS_STRING_PARSER_BASE);
 
 	// Copy breakpoint information from current game instance
-	retval->have_bp = s->have_bp;
-	retval->bp_list = s->bp_list;
+	retval->_breakpoints = s->_breakpoints;
+	retval->_activeBreakpointTypes = s->_activeBreakpointTypes;
 
 	retval->successor = NULL;
 	retval->_gameId = s->_gameId;
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index cef90b0e28..e94e9f64e6 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -204,8 +204,6 @@ extern opcode_format g_opcode_formats[128][4];
 
 void script_adjust_opcode_formats(EngineState *s);
 
-void script_free_breakpoints(EngineState *s);
-
 } // End of namespace Sci
 
 #endif // SCI_ENGINE_SCRIPT_H
diff --git a/engines/sci/engine/state.cpp b/engines/sci/engine/state.cpp
index 2936097696..7eff388df1 100644
--- a/engines/sci/engine/state.cpp
+++ b/engines/sci/engine/state.cpp
@@ -59,8 +59,7 @@ EngineState::EngineState(ResourceManager *res, Kernel *kernel, Vocabulary *voc,
 
 	script_000 = 0;
 
-	bp_list = 0;
-	have_bp = 0;
+	_activeBreakpointTypes = 0;
 	sys_strings_segment = 0;
 	sys_strings = 0;
 
diff --git a/engines/sci/engine/state.h b/engines/sci/engine/state.h
index 98f072a645..2833957bbb 100644
--- a/engines/sci/engine/state.h
+++ b/engines/sci/engine/state.h
@@ -280,8 +280,8 @@ public:
 	bool usesCdTrack() { return _usesCdTrack; }
 
 	/* Debugger data: */
-	Breakpoint *bp_list;   /**< List of breakpoints */
-	int have_bp;  /**< Bit mask specifying which types of breakpoints are used in bp_list */
+	Common::List<Breakpoint> _breakpoints;   /**< List of breakpoints */
+	int _activeBreakpointTypes;  /**< Bit mask specifying which types of breakpoints are active */
 
 	/* System strings */
 	SegmentId sys_strings_segment;
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 859ebac0c5..e6c20f8e5a 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -52,7 +52,7 @@ int script_abort_flag = 0; // Set to 1 to abort execution. Set to 2 to force a r
 int script_step_counter = 0; // Counts the number of steps executed	// FIXME: Avoid non-const global vars
 int script_gc_interval = GC_INTERVAL; // Number of steps in between gcs	// FIXME: Avoid non-const global vars
 
-static bool breakpointFlag = false;	// FIXME: Avoid non-const global vars
+static bool breakpointWasHit = false;	// FIXME: Avoid non-const global vars
 
 // validation functionality
 
@@ -241,22 +241,20 @@ ExecStack *execute_method(EngineState *s, uint16 script, uint16 pubfunct, StackP
 	}
 
 	// Check if a breakpoint is set on this method
-	if (s->have_bp & BREAK_EXPORT) {
-		Breakpoint *bp;
+	if (s->_activeBreakpointTypes & BREAK_EXPORT) {
 		uint32 bpaddress;
 
 		bpaddress = (script << 16 | pubfunct);
 
-		bp = s->bp_list;
-		while (bp) {
-			if (bp->type == BREAK_EXPORT && bp->data.address == bpaddress) {
+		Common::List<Breakpoint>::const_iterator bp;
+		for (bp = s->_breakpoints.begin(); bp != s->_breakpoints.end(); ++bp) {
+			if (bp->type == BREAK_EXPORT && bp->address == bpaddress) {
 				Console *con = ((SciEngine *)g_engine)->getSciDebugger();
 				con->DebugPrintf("Break on script %d, export %d\n", script, pubfunct);
 				g_debugState.debugging = true;
-				breakpointFlag = true;
+				breakpointWasHit = true;
 				break;
 			}
-			bp = bp->next;
 		}
 	}
 
@@ -322,32 +320,30 @@ ExecStack *send_selector(EngineState *s, reg_t send_obj, reg_t work_obj, StackPt
 		}
 
 		// Check if a breakpoint is set on this method
-		if (s->have_bp & BREAK_SELECTOR) {
-			Breakpoint *bp;
-			char method_name [256];
+		if (s->_activeBreakpointTypes & BREAK_SELECTOR) {
+			char method_name[256];
 
 			sprintf(method_name, "%s::%s", s->_segMan->getObjectName(send_obj), s->_kernel->getSelectorName(selector).c_str());
 
-			bp = s->bp_list;
-			while (bp) {
-				int cmplen = strlen(bp->data.name);
-				if (bp->data.name[cmplen - 1] != ':')
+			Common::List<Breakpoint>::const_iterator bp;
+			for (bp = s->_breakpoints.begin(); bp != s->_breakpoints.end(); ++bp) {
+				int cmplen = bp->name.size();
+				if (bp->name.lastChar() != ':')
 					cmplen = 256;
 
-				if (bp->type == BREAK_SELECTOR && !strncmp(bp->data.name, method_name, cmplen)) {
+				if (bp->type == BREAK_SELECTOR && !strncmp(bp->name.c_str(), method_name, cmplen)) {
 					Console *con = ((SciEngine *)g_engine)->getSciDebugger();
 					con->DebugPrintf("Break on %s (in [%04x:%04x])\n", method_name, PRINT_REG(send_obj));
 					print_send_action = 1;
-					breakpointFlag = true;
+					breakpointWasHit = true;
 					g_debugState.debugging = true;
 					break;
 				}
-				bp = bp->next;
 			}
 		}
 
 #ifdef VM_DEBUG_SEND
-		printf("Send to %04x:%04x, selector %04x (%s):", PRINT_REG(send_obj), selector, ((SciEngine*)g_engine)->getKernel()->getSelectorName(selector).c_str());
+		printf("Send to %04x:%04x, selector %04x (%s):", PRINT_REG(send_obj), selector, ((SciEngine *)g_engine)->getKernel()->getSelectorName(selector).c_str());
 #endif // VM_DEBUG_SEND
 
 		ObjVarRef varp;
@@ -651,8 +647,8 @@ void run_vm(EngineState *s, int restoring) {
 		// Debug if this has been requested:
 		// TODO: re-implement sci_debug_flags
 		if (g_debugState.debugging /* sci_debug_flags*/) {
-			script_debug(s, breakpointFlag);
-			breakpointFlag = false;
+			script_debug(s, breakpointWasHit);
+			breakpointWasHit = false;
 		}
 		Console *con = ((Sci::SciEngine*)g_engine)->getSciDebugger();
 		if (con->isAttached()) {
diff --git a/engines/sci/engine/vm.h b/engines/sci/engine/vm.h
index 28a43dd9a2..cb9e5782c0 100644
--- a/engines/sci/engine/vm.h
+++ b/engines/sci/engine/vm.h
@@ -287,11 +287,8 @@ enum BreakpointType {
 
 struct Breakpoint {
 	BreakpointType type;
-	union {
-		uint32 address;  ///< Breakpoints on exports
-		char *name; ///< Breakpoints on selector names
-	} data;
-	Breakpoint *next;
+	uint32 address;  ///< Breakpoints on exports
+	Common::String name; ///< Breakpoints on selector names
 };
 
 /**
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index 373e3d7f33..2ebdd98aa9 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -35,7 +35,7 @@
 
 #include "sci/engine/state.h"
 #include "sci/engine/kernel.h"
-#include "sci/engine/script.h"	// for script_adjust_opcode_formats & script_free_breakpoints
+#include "sci/engine/script.h"	// for script_adjust_opcode_formats
 
 #include "sci/sound/audio.h"
 #include "sci/sound/soundcmd.h"
@@ -242,7 +242,6 @@ Common::Error SciEngine::run() {
 	game_run(&_gamestate); // Run the game
 
 	game_exit(_gamestate);
-	script_free_breakpoints(_gamestate);
 
 	ConfMan.flushToDisk();