SCUMM: Replace UB-triggering serialization code with Common::Serializer

Fixes Trac#10342.

6 files changed, 71 insertions, 91 deletions

diff --git a/engines/scumm/players/player_ad.cpp b/engines/scumm/players/player_ad.cpp
index 55bbeeef98..2de8ac96e4 100644
--- a/engines/scumm/players/player_ad.cpp
+++ b/engines/scumm/players/player_ad.cpp
@@ -191,27 +191,27 @@ int Player_AD::getSoundStatus(int sound) const {
 	return false;
 }
 
-void Player_AD::saveLoadWithSerializer(Serializer *ser) {
+void Player_AD::saveLoadWithSerializer(Common::Serializer &s) {
 	Common::StackLock lock(_mutex);
 
-	if (ser->getVersion() < VER(95)) {
+	if (s.getVersion() < VER(95)) {
 		IMuse *dummyImuse = IMuse::create(_vm->_system, NULL, NULL);
-		dummyImuse->save_or_load(ser, _vm, false);
+		dummyImuse->saveLoadWithSerializer(s, _vm, false);
 		delete dummyImuse;
 		return;
 	}
 
-	if (ser->getVersion() >= VER(96)) {
+	if (s.getVersion() >= VER(96)) {
 		int32 res[4] = {
 			_musicResource, _sfx[0].resource, _sfx[1].resource, _sfx[2].resource
 		};
 
 		// The first thing we save is a list of sound resources being played
 		// at the moment.
-		ser->saveLoadArrayOf(res, 4, sizeof(res[0]), sleInt32);
+		s.syncArray(res, 4, Common::Serializer::Sint32LE);
 
 		// If we are loading start the music again at this point.
-		if (ser->isLoading()) {
+		if (s.isLoading()) {
 			if (res[0] != -1) {
 				startSound(res[0]);
 			}
@@ -219,26 +219,21 @@ void Player_AD::saveLoadWithSerializer(Serializer *ser) {
 
 		uint32 musicOffset = _curOffset;
 
-		static const SaveLoadEntry musicData[] = {
-			MKLINE(Player_AD, _engineMusicTimer, sleInt32, VER(96)),
-			MKLINE(Player_AD, _musicTimer, sleUint32, VER(96)),
-			MKLINE(Player_AD, _internalMusicTimer, sleUint32, VER(96)),
-			MKLINE(Player_AD, _curOffset, sleUint32, VER(96)),
-			MKLINE(Player_AD, _nextEventTimer, sleUint32, VER(96)),
-			MKEND()
-		};
-
-		ser->saveLoadEntries(this, musicData);
+		s.syncAsSint32LE(_engineMusicTimer, VER(96));
+		s.syncAsUint32LE(_musicTimer, VER(96));
+		s.syncAsUint32LE(_internalMusicTimer, VER(96));
+		s.syncAsUint32LE(_curOffset, VER(96));
+		s.syncAsUint32LE(_nextEventTimer, VER(96));
 
 		// We seek back to the old music position.
-		if (ser->isLoading()) {
+		if (s.isLoading()) {
 			SWAP(musicOffset, _curOffset);
 			musicSeekTo(musicOffset);
 		}
 
 		// Finally start up the SFX. This makes sure that they are not
 		// accidently stopped while seeking to the old music position.
-		if (ser->isLoading()) {
+		if (s.isLoading()) {
 			for (int i = 1; i < ARRAYSIZE(res); ++i) {
 				if (res[i] != -1) {
 					startSound(res[i]);
diff --git a/engines/scumm/players/player_ad.h b/engines/scumm/players/player_ad.h
index 1e665e82d5..20c8b3d475 100644
--- a/engines/scumm/players/player_ad.h
+++ b/engines/scumm/players/player_ad.h
@@ -26,6 +26,7 @@
 #include "scumm/music.h"
 
 #include "common/mutex.h"
+#include "common/serializer.h"
 
 namespace OPL {
 class OPL;
@@ -34,7 +35,6 @@ class OPL;
 namespace Scumm {
 
 class ScummEngine;
-class Serializer;
 
 /**
  * Sound output for v3/v4 AdLib data.
@@ -52,7 +52,7 @@ public:
 	virtual int  getMusicTimer();
 	virtual int  getSoundStatus(int sound) const;
 
-	virtual void saveLoadWithSerializer(Serializer *ser);
+	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 
 	// Timer callback
 	void onTimer();
diff --git a/engines/scumm/players/player_mac.cpp b/engines/scumm/players/player_mac.cpp
index 87406f4e34..cc65adceab 100644
--- a/engines/scumm/players/player_mac.cpp
+++ b/engines/scumm/players/player_mac.cpp
@@ -97,53 +97,46 @@ Player_Mac::~Player_Mac() {
 	delete[] _channel;
 }
 
-void Player_Mac::saveLoadWithSerializer(Serializer *ser) {
+void syncWithSerializer(Common::Serializer &s, Player_Mac::Channel &c) {
+	s.syncAsUint16LE(c._pos, VER(94));
+	s.syncAsSint32LE(c._pitchModifier, VER(94));
+	s.syncAsByte(c._velocity, VER(94));
+	s.syncAsUint32LE(c._remaining, VER(94));
+	s.syncAsByte(c._notesLeft, VER(94));
+}
+
+void syncWithSerializer(Common::Serializer &s, Player_Mac::Instrument &i) {
+	s.syncAsUint32LE(i._pos, VER(94));
+	s.syncAsUint32LE(i._subPos, VER(94));
+}
+
+void Player_Mac::saveLoadWithSerializer(Common::Serializer &s) {
 	Common::StackLock lock(_mutex);
-	if (ser->getVersion() < VER(94)) {
-		if (_vm->_game.id == GID_MONKEY && ser->isLoading()) {
+	if (s.getVersion() < VER(94)) {
+		if (_vm->_game.id == GID_MONKEY && s.isLoading()) {
 			IMuse *dummyImuse = IMuse::create(_vm->_system, NULL, NULL);
-			dummyImuse->save_or_load(ser, _vm, false);
+			dummyImuse->saveLoadWithSerializer(s, _vm, false);
 			delete dummyImuse;
 		}
 	} else {
-		static const SaveLoadEntry musicEntries[] = {
-			MKLINE(Player_Mac, _sampleRate, sleUint32, VER(94)),
-			MKLINE(Player_Mac, _soundPlaying, sleInt16, VER(94)),
-			MKEND()
-		};
-
-		static const SaveLoadEntry channelEntries[] = {
-			MKLINE(Channel, _pos, sleUint16, VER(94)),
-			MKLINE(Channel, _pitchModifier, sleInt32, VER(94)),
-			MKLINE(Channel, _velocity, sleUint8, VER(94)),
-			MKLINE(Channel, _remaining, sleUint32, VER(94)),
-			MKLINE(Channel, _notesLeft, sleUint8, VER(94)),
-			MKEND()
-		};
-
-		static const SaveLoadEntry instrumentEntries[] = {
-			MKLINE(Instrument, _pos, sleUint32, VER(94)),
-			MKLINE(Instrument, _subPos, sleUint32, VER(94)),
-			MKEND()
-		};
-
 		uint32 mixerSampleRate = _sampleRate;
 		int i;
 
-		ser->saveLoadEntries(this, musicEntries);
+		s.syncAsUint32LE(_sampleRate, VER(94));
+		s.syncAsSint16LE(_soundPlaying, VER(94));
 
-		if (ser->isLoading() && _soundPlaying != -1) {
+		if (s.isLoading() && _soundPlaying != -1) {
 			const byte *ptr = _vm->getResourceAddress(rtSound, _soundPlaying);
 			assert(ptr);
 			loadMusic(ptr);
 		}
 
-		ser->saveLoadArrayOf(_channel, _numberOfChannels, sizeof(Channel), channelEntries);
+		s.syncArray(_channel, _numberOfChannels, syncWithSerializer);
 		for (i = 0; i < _numberOfChannels; i++) {
-			ser->saveLoadEntries(&_channel[i], instrumentEntries);
+			syncWithSerializer(s, _channel[i]);
 		}
 
-		if (ser->isLoading()) {
+		if (s.isLoading()) {
 			// If necessary, adjust the channel data to fit the
 			// current sample rate.
 			if (_soundPlaying != -1 && _sampleRate != mixerSampleRate) {
diff --git a/engines/scumm/players/player_mac.h b/engines/scumm/players/player_mac.h
index d247ce9883..897960d775 100644
--- a/engines/scumm/players/player_mac.h
+++ b/engines/scumm/players/player_mac.h
@@ -63,7 +63,7 @@ public:
 	virtual bool endOfData() const { return false; }
 	virtual int getRate() const { return _sampleRate; }
 
-	virtual void saveLoadWithSerializer(Serializer *ser);
+	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 
 private:
 	Common::Mutex _mutex;
@@ -92,6 +92,7 @@ private:
 
 		void generateSamples(int16 *data, int pitchModifier, int volume, int numSamples, int remainingSamplesOnNote, bool fadeNoteEnds);
 	};
+	friend void syncWithSerializer(Common::Serializer &, Instrument &);
 
 	int _pitchTable[128];
 	int _numberOfChannels;
@@ -120,6 +121,7 @@ protected:
 
 		bool loadInstrument(Common::SeekableReadStream *stream);
  	};
+	friend void syncWithSerializer(Common::Serializer &, Channel &);
 
 	ScummEngine *const _vm;
 	Channel *_channel;
diff --git a/engines/scumm/players/player_towns.cpp b/engines/scumm/players/player_towns.cpp
index d540fc4f6b..16080205c0 100644
--- a/engines/scumm/players/player_towns.cpp
+++ b/engines/scumm/players/player_towns.cpp
@@ -46,19 +46,18 @@ int Player_Towns::getSoundStatus(int sound) const {
 	return 0;
 }
 
-void Player_Towns::saveLoadWithSerializer(Serializer *ser) {
-	static const SaveLoadEntry pcmEntries[] = {
-		MKLINE(PcmCurrentSound, index, sleInt16, VER(81)),
-		MKLINE(PcmCurrentSound, chan, sleInt16, VER(81)),
-		MKLINE(PcmCurrentSound, note, sleUint8, VER(81)),
-		MKLINE(PcmCurrentSound, velo, sleUint8, VER(81)),
-		MKLINE(PcmCurrentSound, pan, sleUint8, VER(81)),
-		MKLINE(PcmCurrentSound, paused, sleUint8, VER(81)),
-		MKLINE(PcmCurrentSound, looping, sleUint8, VER(81)),
-		MKLINE(PcmCurrentSound, priority, sleUint32, VER(81)),
-		MKEND()
-	};
-
+void syncWithSerializer(Common::Serializer &s, Player_Towns::PcmCurrentSound &pcs) {
+	s.syncAsSint16LE(pcs.index, VER(81));
+	s.syncAsSint16LE(pcs.chan, VER(81));
+	s.syncAsByte(pcs.note, VER(81));
+	s.syncAsByte(pcs.velo, VER(81));
+	s.syncAsByte(pcs.pan, VER(81));
+	s.syncAsByte(pcs.paused, VER(81));
+	s.syncAsByte(pcs.looping, VER(81));
+	s.syncAsUint32LE(pcs.priority, VER(81));
+}
+
+void Player_Towns::saveLoadWithSerializer(Common::Serializer &s) {
 	for (int i = 1; i < 9; i++) {
 		if (!_pcmCurrentSound[i].index)
 			continue;
@@ -71,7 +70,7 @@ void Player_Towns::saveLoadWithSerializer(Serializer *ser) {
 		_pcmCurrentSound[i].index = 0;
 	}
 
-	ser->saveLoadArrayOf(_pcmCurrentSound, 9, sizeof(PcmCurrentSound), pcmEntries);
+	s.syncArray(_pcmCurrentSound, 9, syncWithSerializer);
 }
 
 void Player_Towns::restoreAfterLoad() {
@@ -362,34 +361,24 @@ void Player_Towns_v1::setSoundNote(int sound, int note) {
 		_soundOverride[sound].note = note;
 }
 
-void Player_Towns_v1::saveLoadWithSerializer(Serializer *ser) {
+void Player_Towns_v1::saveLoadWithSerializer(Common::Serializer &s) {
 	_cdaCurrentSoundTemp = (_vm->_sound->pollCD() && _cdaNumLoops > 1) ? _cdaCurrentSound & 0xff : 0;
 	_cdaNumLoopsTemp = _cdaNumLoops & 0xff;
 
-	static const SaveLoadEntry cdEntries[] = {
-		MKLINE(Player_Towns_v1, _cdaCurrentSoundTemp, sleUint8, VER(81)),
-		MKLINE(Player_Towns_v1, _cdaNumLoopsTemp, sleUint8, VER(81)),
-		MKLINE(Player_Towns_v1, _cdaVolLeft, sleUint8, VER(81)),
-		MKLINE(Player_Towns_v1, _cdaVolRight, sleUint8, VER(81)),
-		MKEND()
-	};
-
-	ser->saveLoadEntries(this, cdEntries);
+	s.syncAsByte(_cdaCurrentSoundTemp, VER(81));
+	s.syncAsByte(_cdaNumLoopsTemp, VER(81));
+	s.syncAsByte(_cdaVolLeft, VER(81));
+	s.syncAsByte(_cdaVolRight, VER(81));
 
 	if (!_eupLooping && !_player->isPlaying())
 		_eupCurrentSound = 0;
 
-	static const SaveLoadEntry eupEntries[] = {
-		MKLINE(Player_Towns_v1, _eupCurrentSound, sleUint8, VER(81)),
-		MKLINE(Player_Towns_v1, _eupLooping, sleUint8, VER(81)),
-		MKLINE(Player_Towns_v1, _eupVolLeft, sleUint8, VER(81)),
-		MKLINE(Player_Towns_v1, _eupVolRight, sleUint8, VER(81)),
-		MKEND()
-	};
-
-	ser->saveLoadEntries(this, eupEntries);
+	s.syncAsByte(_eupCurrentSound, VER(81));
+	s.syncAsByte(_eupLooping, VER(81));
+	s.syncAsByte(_eupVolLeft, VER(81));
+	s.syncAsByte(_eupVolRight, VER(81));
 
-	Player_Towns::saveLoadWithSerializer(ser);
+	Player_Towns::saveLoadWithSerializer(s);
 }
 
 void Player_Towns_v1::restoreAfterLoad() {
@@ -721,9 +710,9 @@ int32 Player_Towns_v2::doCommand(int numargs, int args[]) {
 	return res;
 }
 
-void Player_Towns_v2::saveLoadWithSerializer(Serializer *ser) {
-	if (ser->getVersion() >= 83)
-		Player_Towns::saveLoadWithSerializer(ser);
+void Player_Towns_v2::saveLoadWithSerializer(Common::Serializer &s) {
+	if (s.getVersion() >= VER(83))
+		Player_Towns::saveLoadWithSerializer(s);
 }
 
 void Player_Towns_v2::playVocTrack(const uint8 *data) {
diff --git a/engines/scumm/players/player_towns.h b/engines/scumm/players/player_towns.h
index 576d17e392..ad51c3e561 100644
--- a/engines/scumm/players/player_towns.h
+++ b/engines/scumm/players/player_towns.h
@@ -43,7 +43,7 @@ public:
 
 	virtual int32 doCommand(int numargs, int args[]) = 0;
 
-	virtual void saveLoadWithSerializer(Serializer *ser);
+	virtual void saveLoadWithSerializer(Common::Serializer &ser);
 	virtual void restoreAfterLoad();
 
 	// version 1 specific
@@ -69,6 +69,7 @@ protected:
 		uint8 looping;
 		uint32 priority;
 	} _pcmCurrentSound[9];
+	friend void syncWithSerializer(Common::Serializer &, PcmCurrentSound &);
 
 	uint8 _unkFlags;
 
@@ -101,7 +102,7 @@ public:
 	void setSoundVolume(int sound, int left, int right);
 	void setSoundNote(int sound, int note);
 
-	void saveLoadWithSerializer(Serializer *ser);
+	void saveLoadWithSerializer(Common::Serializer &ser);
 	void restoreAfterLoad();
 
 private:
@@ -154,7 +155,7 @@ public:
 
 	int32 doCommand(int numargs, int args[]);
 
-	void saveLoadWithSerializer(Serializer *ser);
+	void saveLoadWithSerializer(Common::Serializer &ser);
 
 private:
 	void playVocTrack(const uint8 *data);