GLK: ADRIFT: Refactoring load serializer to not use longjmp

7 files changed, 125 insertions, 141 deletions

diff --git a/engines/glk/adrift/serialization.cpp b/engines/glk/adrift/serialization.cpp
index 404619a223..5a855e797c 100644
--- a/engines/glk/adrift/serialization.cpp
+++ b/engines/glk/adrift/serialization.cpp
@@ -124,7 +124,7 @@ void SaveSerializer::save() {
 		if (task > 0)
 			writeBool(gs_task_done(_game, task - 1));
 		else
-			writeBool(FALSE);
+			writeBool(false);
 	}
 
 	/* Save NPCs information. */
@@ -177,8 +177,8 @@ void SaveSerializer::save() {
 	 * references.
 	 */
 	flush(TRUE);
-	_callback = NULL;
-	_opaque = NULL;
+	_callback = nullptr;
+	_opaque = nullptr;
 }
 
 void SaveSerializer::flush(sc_bool is_final) {
@@ -225,20 +225,13 @@ void SaveSerializer::writeUint(sc_uint value) {
 }
 
 void SaveSerializer::writeBool(sc_bool boolean) {
-	// Write a 1 for TRUE, 0 for FALSE
+	// Write a 1 for TRUE, 0 for false
 	writeString(boolean ? "1" : "0");
 }
 
 /*--------------------------------------------------------------------------*/
 
-/* TAS input file line counter. */
-static sc_tafref_t ser_tas = NULL;
-static sc_int ser_tasline = 0;
-
-/* Restore error jump buffer. */
-static jmp_buf ser_tas_error;
-static sc_var_setref_t new_vars;  /* For setjmp safety */
-static sc_gameref_t new_game;     /* For setjmp safety */
+#define CHECK if (context._break) goto ser_tas_error
 
 bool LoadSerializer::load() {
 	const sc_filterref_t filter = gs_get_filter(_game);
@@ -246,144 +239,128 @@ bool LoadSerializer::load() {
 	sc_vartype_t vt_key[3];
 	sc_int index_, var_count;
 	const sc_char *gamename;
+	sc_var_setref_t new_vars = nullptr;
+	sc_gameref_t new_game = nullptr;
+	Context context;
 
-	/* Create a TAF (TAS) reference from callbacks, for reader functions. */
+	// Create a TAF (TAS) reference from callbacks, for reader functions
 	ser_tas = taf_create_tas(_callback, _opaque);
 	if (!ser_tas)
-		return FALSE;
+		return false;
 
-	/* Reset line counter for error messages. */
+	// Reset line counter for error messages.
 	ser_tasline = 1;
 
-	new_game = NULL;
-	new_vars = NULL;
-
-	/* Set up error handling jump buffer, and handle errors. */
-	if (setjmp(ser_tas_error) != 0) {
-		/* Destroy any temporary _game and variables. */
-		if (new_game)
-			gs_destroy(new_game);
-		if (new_vars)
-			var_destroy(new_vars);
-
-		/* Destroy the TAF (TAS) file and return fail status. */
-		taf_destroy(ser_tas);
-		ser_tas = NULL;
-		return FALSE;
-	}
-
-	/*
-	 * Read the _game name, and compare with the one in the _game.  Fail if
-	 * they don't match exactly.  A tighter check than this would perhaps be
-	 * preferable, say, something based on the TAF file header, but this isn't
-	 * in the save file format.
-	 */
+	// Read the _game name, and compare with the one in the _game.  Fail if they don't match exactly.
+	// A tighter check than this would perhaps be preferable, say, something based on the TAF file
+	// header, but this isn't in the save file format.
+	
 	vt_key[0].string = "Globals";
 	vt_key[1].string = "GameName";
 	gamename = prop_get_string(bundle, "S<-ss", vt_key);
-	if (strcmp(readString(), gamename) != 0)
-		longjmp(ser_tas_error, 1);
-
-	/* Read and verify the counts in the saved _game. */
-	if (readInt() != gs_room_count(_game)
-		|| readInt() != gs_object_count(_game)
-		|| readInt() != gs_task_count(_game)
-		|| readInt() != gs_event_count(_game)
-		|| readInt() != gs_npc_count(_game))
-		longjmp(ser_tas_error, 1);
-
-	/* Create a variables set and _game to restore into. */
+	if (strcmp(readString(context), gamename) != 0 || context._break)
+		goto ser_tas_error;
+
+	// Read and verify the counts in the saved _game.
+	if ((readInt(context) != gs_room_count(_game) || context._break)
+		|| (readInt(context) != gs_object_count(_game) || context._break)
+		|| (readInt(context) != gs_task_count(_game) || context._break)
+		|| (readInt(context) != gs_event_count(_game) || context._break)
+		|| (readInt(context) != gs_npc_count(_game) || context._break))
+		goto ser_tas_error;
+
+	// Create a variables set and _game to restore into.
 	new_vars = var_create(bundle);
 	new_game = gs_create(new_vars, bundle, filter);
 	var_register_game(new_vars, new_game);
 
-	/* All set to load TAF (TAS) data into the new _game. */
+	// All set to load TAF (TAS) data into the new _game.
 
-	/* Restore the score and player information. */
-	new_game->score = readInt();
-	gs_set_playerroom(new_game, readInt() - 1);
-	gs_set_playerparent(new_game, readInt());
-	gs_set_playerposition(new_game, readInt());
+	// Restore the score and player information.
+	new_game->score = readInt(context); CHECK;
+	gs_set_playerroom(new_game, readInt(context) - 1); CHECK;
+	gs_set_playerparent(new_game, readInt(context)); CHECK;
+	gs_set_playerposition(new_game, readInt(context)); CHECK;
 
-	/* Skip player gender. */
-	(void)readInt();
+	// Skip player gender.
+	(void)readInt(context); CHECK;
 
-	/* Skip encumbrance details, not currently maintained by the _game. */
-	(void)readInt();
-	(void)readInt();
-	(void)readInt();
-	(void)readInt();
+	// Skip encumbrance details, not currently maintained by the _game.
+	(void)readInt(context); CHECK;
+	(void)readInt(context); CHECK;
+	(void)readInt(context); CHECK;
+	(void)readInt(context); CHECK;
 
-	/* Restore rooms information. */
-	for (index_ = 0; index_ < gs_room_count(new_game); index_++)
-		gs_set_room_seen(new_game, index_, readBool());
+	// Restore rooms information.
+	for (index_ = 0; index_ < gs_room_count(new_game); index_++) {
+		gs_set_room_seen(new_game, index_, readBool(context)); CHECK;
+	}
 
-	/* Restore objects information. */
+	// Restore objects information.
 	for (index_ = 0; index_ < gs_object_count(new_game); index_++) {
 		sc_int openable, currentstate;
 
-		/* Bypass mutators for position and parent.  Fix later? */
-		new_game->objects[index_].position = readInt();
-		gs_set_object_seen(new_game, index_, readBool());
-		new_game->objects[index_].parent = readInt();
+		// Bypass mutators for position and parent.  Fix later?
+		new_game->objects[index_].position = readInt(context); CHECK;
+		gs_set_object_seen(new_game, index_, readBool(context)); CHECK;
+		new_game->objects[index_].parent = readInt(context); CHECK;
 
 		vt_key[0].string = "Objects";
 		vt_key[1].integer = index_;
 		vt_key[2].string = "Openable";
 		openable = prop_get_integer(bundle, "I<-sis", vt_key);
-		gs_set_object_openness(new_game, index_,
-			openable != 0 ? readInt() : 0);
+		gs_set_object_openness(new_game, index_, openable != 0 ? readInt(context) : 0); CHECK;
 
 		vt_key[2].string = "CurrentState";
 		currentstate = prop_get_integer(bundle, "I<-sis", vt_key);
 		gs_set_object_state(new_game, index_,
-			currentstate != 0 ? readInt() : 0);
+			currentstate != 0 ? readInt(context) : 0); CHECK;
 
-		gs_set_object_unmoved(new_game, index_, readBool());
+		gs_set_object_unmoved(new_game, index_, readBool(context)); CHECK;
 	}
 
-	/* Restore tasks information. */
+	// Restore tasks information.
 	for (index_ = 0; index_ < gs_task_count(new_game); index_++) {
-		gs_set_task_done(new_game, index_, readBool());
-		gs_set_task_scored(new_game, index_, readBool());
+		gs_set_task_done(new_game, index_, readBool(context)); CHECK;
+		gs_set_task_scored(new_game, index_, readBool(context)); CHECK;
 	}
 
-	/* Restore events information. */
+	// Restore events information.
 	for (index_ = 0; index_ < gs_event_count(new_game); index_++) {
 		sc_int startertype, task;
 
-		/* Restore first event details. */
-		gs_set_event_time(new_game, index_, readInt());
-		task = readInt();
-		gs_set_event_state(new_game, index_, readInt() + 1);
+		// Restore first event details.
+		gs_set_event_time(new_game, index_, readInt(context)); CHECK;
+		task = readInt(context); CHECK;
+		gs_set_event_state(new_game, index_, readInt(context) + 1); CHECK;
 
-		/* Verify and restore the starter task, if any. */
+		// Verify and restore the starter task, if any.
 		if (task > 0) {
 			vt_key[0].string = "Events";
 			vt_key[1].integer = index_;
 			vt_key[2].string = "StarterType";
 			startertype = prop_get_integer(bundle, "I<-sis", vt_key);
 			if (startertype != 3)
-				longjmp(ser_tas_error, 1);
+				goto ser_tas_error;
 
-			/* Restore task state. */
-			gs_set_task_done(new_game, task - 1, readBool());
+			// Restore task state.
+			gs_set_task_done(new_game, task - 1, readBool(context)); CHECK;
+		} else {
+			(void)readBool(context); CHECK;
 		}
-		else
-			(void)readBool();
 	}
 
-	/* Restore NPCs information. */
+	// Restore NPCs information.
 	for (index_ = 0; index_ < gs_npc_count(new_game); index_++) {
 		sc_int walk;
 
-		gs_set_npc_location(new_game, index_, readInt());
-		gs_set_npc_seen(new_game, index_, readBool());
+		gs_set_npc_location(new_game, index_, readInt(context)); CHECK;
+		gs_set_npc_seen(new_game, index_, readBool(context)); CHECK;
 		for (walk = 0; walk < gs_npc_walkstep_count(new_game, index_); walk++)
-			gs_set_npc_walkstep(new_game, index_, walk, readInt());
+			gs_set_npc_walkstep(new_game, index_, walk, readInt(context)); CHECK;
 	}
 
-	/* Restore each variable. */
+	// Restore each variable.
 	vt_key[0].string = "Variables";
 	var_count = prop_get_child_count(bundle, "I<-s", vt_key);
 
@@ -400,11 +377,11 @@ bool LoadSerializer::load() {
 
 		switch (var_type) {
 		case TAFVAR_NUMERIC:
-			var_put_integer(new_vars, name, readInt());
+			var_put_integer(new_vars, name, readInt(context)); CHECK;
 			break;
 
 		case TAFVAR_STRING:
-			var_put_string(new_vars, name, readString());
+			var_put_string(new_vars, name, readString(context)); CHECK;
 			break;
 
 		default:
@@ -412,102 +389,108 @@ bool LoadSerializer::load() {
 		}
 	}
 
-	/* Restore timing information. */
-	var_set_elapsed_seconds(new_vars, readUint());
+	// Restore timing information.
+	var_set_elapsed_seconds(new_vars, readUint(context)); CHECK;
 
-	/* Restore turns count. */
-	new_game->turns = (sc_int)readUint();
+	// Restore turns count.
+	new_game->turns = (sc_int)readUint(context); CHECK;
 
-	/*
-	 * Resources tweak -- set requested to match those in the current _game
-	 * so that they remain unchanged by the gs_copy() of new_game onto
-	 * _game.  This way, both the requested and the active resources in the
-	 * _game are unchanged by restore.
+	/* Resources tweak -- set requested to match those in the current _game so that they remain
+	 * unchanged by the gs_copy() of new_game onto game.  This way, both the requested and the
+	 * active resources in the game are unchanged by restore.
 	 */
 	new_game->requested_sound = _game->requested_sound;
 	new_game->requested_graphic = _game->requested_graphic;
 
-	/*
-	 * Quitter tweak -- set the quit jump buffer in the new _game to be the
+	/* Quitter tweak -- set the quit jump buffer in the new _game to be the
 	 * same as the current one, so that it remains unchanged by gs_copy().  The
 	 * one in the new _game is still the unset one from gs_create().
 	 */
 	memcpy(&new_game->quitter, &_game->quitter, sizeof(_game->quitter));
 
-	/*
-	 * If we got this far, we successfully restored the _game from the file.
+	/* If we got this far, we successfully restored the _game from the file.
 	 * As our final act, copy the new _game onto the old one.
 	 */
 	new_game->temporary = _game->temporary;
 	new_game->undo = _game->undo;
 	gs_copy(_game, new_game);
 
-	/* Done with the temporary _game and variables. */
+	// Done with the temporary _game and variables.
 	gs_destroy(new_game);
 	var_destroy(new_vars);
 
-	/* Done with TAF (TAS) file; destroy it and return successfully. */
+	// Done with TAF (TAS) file; destroy it and return successfully
+	taf_destroy(ser_tas);
+	return true;
+
+ser_tas_error:
+	// Destroy any temporary _game and variables
+	if (new_game)
+		gs_destroy(new_game);
+	if (new_vars)
+		var_destroy(new_vars);
+
+	// Destroy the TAF (TAS) file and return fail status
 	taf_destroy(ser_tas);
-	ser_tas = NULL;
-	return TRUE;
+	return false;
 }
 
-const sc_char *LoadSerializer::readString() {
+const sc_char *LoadSerializer::readString(CONTEXT) {
 	const sc_char *string;
 
 	/* Get the next line, and complain if absent. */
 	string = taf_next_line(ser_tas);
 	if (!string) {
 		sc_error("readString: out of TAS data at line %ld\n", ser_tasline);
-		longjmp(ser_tas_error, 1);
+		LONG_JUMP0
 	}
 
 	ser_tasline++;
 	return string;
 }
 
-sc_int LoadSerializer::readInt() {
+sc_int LoadSerializer::readInt(CONTEXT) {
 	const sc_char *string;
 	sc_int value;
 
 	// Get line, and scan for a single integer; return it
-	string = readString();
+	R0FUNC0(readString, string)
 	if (sscanf(string, "%ld", &value) != 1) {
 		sc_error("readInt: invalid integer at line %ld\n", ser_tasline - 1);
-		longjmp(ser_tas_error, 1);
+		LONG_JUMP0
 	}
 
 	return value;
 }
 
-sc_uint LoadSerializer::readUint() {
+sc_uint LoadSerializer::readUint(CONTEXT) {
 	const sc_char *string;
 	sc_uint value;
 
 	// Get line, and scan for a single integer; return it
-	string = readString();
+	R0FUNC0(readString, string)
 	if (sscanf(string, "%lu", &value) != 1) {
 		sc_error("readUint: invalid integer at line %ld\n", ser_tasline - 1);
-		longjmp(ser_tas_error, 1);
+		LONG_JUMP0
 	}
 
 	return value;
 }
 
-sc_bool LoadSerializer::readBool(void) {
+sc_bool LoadSerializer::readBool(CONTEXT) {
 	const sc_char *string;
 	sc_uint value;
 
 	// Get line, and scan for a single integer; check it's a valid-looking flag, and return it.
-	string = readString();
+	R0FUNC0(readString, string)
+	string = readString(context);
 	if (sscanf(string, "%lu", &value) != 1) {
-		sc_error("readBool:"
-		         " invalid boolean at line %ld\n", ser_tasline - 1);
-		longjmp(ser_tas_error, 1);
+		sc_error("readBool: invalid boolean at line %ld\n", ser_tasline - 1);
+		LONG_JUMP0
 	}
 	if (value != 0 && value != 1) {
-		sc_error("readBool:"
-		         " warning: suspect boolean at line %ld\n", ser_tasline - 1);
+		sc_error("readBool: warning: suspect boolean at line %ld\n", ser_tasline - 1);
+		LONG_JUMP0
 	}
 
 	return value != 0;
diff --git a/engines/glk/adrift/serialization.h b/engines/glk/adrift/serialization.h
index 47559a2d28..c6960442fd 100644
--- a/engines/glk/adrift/serialization.h
+++ b/engines/glk/adrift/serialization.h
@@ -26,6 +26,7 @@
 #include "common/memstream.h"
 #include "common/str.h"
 #include "glk/adrift/scprotos.h"
+#include "glk/jumps.h"
 
 namespace Glk {
 namespace Adrift {
@@ -102,32 +103,34 @@ private:
 	sc_gameref_t _game;
 	sc_read_callbackref_t _callback;
 	void *_opaque;
+	sc_tafref_t ser_tas;
+	sc_int ser_tasline;
 private:
 	/**
 	 * Reads a string
 	 */
-	const sc_char *readString();
+	const sc_char *readString(CONTEXT);
 
 	/**
 	 * Read a signed integer
 	 */
-	sc_int readInt();
+	sc_int readInt(CONTEXT);
 
 	/**
 	 * Read an unsigned integer
 	 */
-	sc_uint readUint();
+	sc_uint readUint(CONTEXT);
 
 	/**
 	 * Read a boolean
 	 */
-	sc_bool readBool();
+	sc_bool readBool(CONTEXT);
 public:
 	/**
 	 * Constructor
 	 */
 	LoadSerializer(sc_gameref_t game, sc_read_callbackref_t callback, void *opaque) :
-		_game(game), _callback(callback), _opaque(opaque) {
+			_game(game), _callback(callback), _opaque(opaque), ser_tas(nullptr), ser_tasline(0) {
 		assert(callback);
 	}
 
diff --git a/engines/glk/alan2/exe.h b/engines/glk/alan2/exe.h
index 89a34b5379..b46f5b8f41 100644
--- a/engines/glk/alan2/exe.h
+++ b/engines/glk/alan2/exe.h
@@ -27,7 +27,7 @@
  */
 
 #include "glk/alan2/types.h"
-#include "glk/alan2/jumps.h"
+#include "glk/jumps.h"
 
 namespace Glk {
 namespace Alan2 {
diff --git a/engines/glk/alan2/inter.h b/engines/glk/alan2/inter.h
index bb26ce1f3e..ee2c323b3f 100644
--- a/engines/glk/alan2/inter.h
+++ b/engines/glk/alan2/inter.h
@@ -23,7 +23,7 @@
 #ifndef GLK_ALAN2_INTER
 #define GLK_ALAN2_INTER
 
-#include "glk/alan2/jumps.h"
+#include "glk/jumps.h"
 
 namespace Glk {
 namespace Alan2 {
diff --git a/engines/glk/alan2/main.h b/engines/glk/alan2/main.h
index 3473ebf0f2..a43d3f729b 100644
--- a/engines/glk/alan2/main.h
+++ b/engines/glk/alan2/main.h
@@ -27,7 +27,7 @@
 
 #include "common/file.h"
 #include "glk/alan2/types.h"
-#include "glk/alan2/jumps.h"
+#include "glk/jumps.h"
 
 namespace Glk {
 namespace Alan2 {
diff --git a/engines/glk/alan2/parse.h b/engines/glk/alan2/parse.h
index 100d739aa7..1cfdcf1364 100644
--- a/engines/glk/alan2/parse.h
+++ b/engines/glk/alan2/parse.h
@@ -25,7 +25,7 @@
 
 /* Parse data for ALAN interpreter module. */
 
-#include "engines/glk/alan2/jumps.h"
+#include "engines/glk/jumps.h"
 
 namespace Glk {
 namespace Alan2 {
diff --git a/engines/glk/alan2/jumps.h b/engines/glk/jumps.h
index 2aa4561dd6..f3438128f4 100644
--- a/engines/glk/alan2/jumps.h
+++ b/engines/glk/jumps.h
@@ -20,15 +20,14 @@
  *
  */
 
-#ifndef GLK_ALAN2_JUMPS
-#define GLK_ALAN2_JUMPS
+#ifndef GLK_JUMPS
+#define GLK_JUMPS
 
 /* This provides a simplified version of the ScummVM coroutines to allow for automated
  * breakouts to the main game loop from subroutinese rather than using unportable setjmps
  */
 
 namespace Glk {
-namespace Alan2 {
 
 /**
  * Context used for flagging when a break to the outer game loop
@@ -64,7 +63,6 @@ struct Context {
 #define LONG_JUMP { context._break = true; return; }
 #define LONG_JUMP0 { context._break = true; return 0; }
 
-} // End of namespace Alan2
 } // End of namespace Glk
 
 #endif