SCI: Implement bounds-checked reads of game resources

6 files changed, 198 insertions, 192 deletions

diff --git a/engines/sci/sound/drivers/adlib.cpp b/engines/sci/sound/drivers/adlib.cpp
index 4f557be95e..b8e90802b2 100644
--- a/engines/sci/sound/drivers/adlib.cpp
+++ b/engines/sci/sound/drivers/adlib.cpp
@@ -31,6 +31,7 @@
 
 #include "sci/resource.h"
 #include "sci/sound/drivers/mididriver.h"
+#include "sci/util.h"
 
 namespace Sci {
 
@@ -50,7 +51,7 @@ public:
 		kRhythmKeys = 62
 	};
 
-	MidiDriver_AdLib(Audio::Mixer *mixer) :_playSwitch(true), _masterVolume(15), _rhythmKeyMap(0), _opl(0), _isOpen(false) { }
+	MidiDriver_AdLib(Audio::Mixer *mixer) :_playSwitch(true), _masterVolume(15), _rhythmKeyMap(), _opl(0), _isOpen(false) { }
 	virtual ~MidiDriver_AdLib() { }
 
 	// MidiDriver
@@ -70,10 +71,10 @@ public:
 
 	void setVolume(byte volume);
 	void playSwitch(bool play);
-	bool loadResource(const byte *data, uint size);
+	bool loadResource(const SciSpan<const byte> &data);
 	virtual uint32 property(int prop, uint32 param);
 
-	bool useRhythmChannel() const { return _rhythmKeyMap != NULL; }
+	bool useRhythmChannel() const { return _rhythmKeyMap; }
 
 private:
 	enum ChannelID {
@@ -139,13 +140,13 @@ private:
 	int _masterVolume;
 	Channel _channels[MIDI_CHANNELS];
 	AdLibVoice _voices[kVoices];
-	byte *_rhythmKeyMap;
+	Common::SpanOwner<SciSpan<const byte> > _rhythmKeyMap;
 	Common::Array<AdLibPatch> _patches;
 
 	Common::TimerManager::TimerProc _adlibTimerProc;
 	void *_adlibTimerParam;
 
-	void loadInstrument(const byte *ins);
+	void loadInstrument(const SciSpan<const byte> &ins);
 	void voiceOn(int voice, int note, int velocity);
 	void voiceOff(int voice);
 	void setPatch(int voice, int patch);
@@ -255,7 +256,7 @@ int MidiDriver_AdLib::openAdLib(bool isSCI0) {
 
 void MidiDriver_AdLib::close() {
 	delete _opl;
-	delete[] _rhythmKeyMap;
+	_rhythmKeyMap.clear();
 }
 
 void MidiDriver_AdLib::setVolume(byte volume) {
@@ -346,12 +347,12 @@ void MidiDriver_AdLib::onTimer() {
 	}
 }
 
-void MidiDriver_AdLib::loadInstrument(const byte *ins) {
+void MidiDriver_AdLib::loadInstrument(const SciSpan<const byte> &ins) {
 	AdLibPatch patch;
 
 	// Set data for the operators
 	for (int i = 0; i < 2; i++) {
-		const byte *op = ins + i * 13;
+		const byte *op = ins.getUnsafeDataAt(i * 13, 13);
 		patch.op[i].kbScaleLevel = op[0] & 0x3;
 		patch.op[i].frequencyMult = op[1] & 0xf;
 		patch.op[i].attackRate = op[3] & 0xf;
@@ -589,7 +590,7 @@ void MidiDriver_AdLib::voiceOn(int voice, int note, int velocity) {
 
 	_voices[voice].age = 0;
 
-	if ((channel == 9) && _rhythmKeyMap) {
+	if (channel == 9 && _rhythmKeyMap) {
 		patch = CLIP(note, 27, 88) + 101;
 	} else {
 		patch = _channels[channel].patch;
@@ -616,7 +617,7 @@ void MidiDriver_AdLib::setNote(int voice, int note, bool key) {
 	float delta;
 	int bend = _channels[channel].pitchWheel;
 
-	if ((channel == 9) && _rhythmKeyMap) {
+	if (channel == 9 && _rhythmKeyMap) {
 		note = _rhythmKeyMap[CLIP(note, 27, 88) - 27];
 	}
 
@@ -756,30 +757,32 @@ void MidiDriver_AdLib::playSwitch(bool play) {
 	renewNotes(-1, play);
 }
 
-bool MidiDriver_AdLib::loadResource(const byte *data, uint size) {
-	if ((size != 1344) && (size != 2690) && (size != 5382)) {
-		error("ADLIB: Unsupported patch format (%i bytes)", size);
+bool MidiDriver_AdLib::loadResource(const SciSpan<const byte> &data) {
+	const uint32 size = data.size();
+	if (size != 1344 && size != 2690 && size != 5382) {
+		error("ADLIB: Unsupported patch format (%u bytes)", size);
 		return false;
 	}
 
 	for (int i = 0; i < 48; i++)
-		loadInstrument(data + (28 * i));
+		loadInstrument(data.subspan(28 * i));
 
 	if (size == 1344) {
 		byte dummy[28] = {0};
 
 		// Only 48 instruments, add dummies
 		for (int i = 0; i < 48; i++)
-			loadInstrument(dummy);
+			loadInstrument(SciSpan<const byte>(dummy, sizeof(dummy)));
 	} else if (size == 2690) {
 		for (int i = 48; i < 96; i++)
-			loadInstrument(data + 2 + (28 * i));
+			loadInstrument(data.subspan(2 + (28 * i)));
 	} else {
 		// SCI1.1 and later
-		for (int i = 48; i < 190; i++)
-			loadInstrument(data + (28 * i));
-		_rhythmKeyMap = new byte[kRhythmKeys];
-		memcpy(_rhythmKeyMap, data + 5320, kRhythmKeys);
+		for (int i = 48; i < 190; i++) {
+			loadInstrument(data.subspan(28 * i));
+		}
+
+		_rhythmKeyMap->allocateFromSpan(data.subspan(5320, kRhythmKeys));
 	}
 
 	return true;
@@ -802,11 +805,11 @@ int MidiPlayer_AdLib::open(ResourceManager *resMan) {
 	assert(resMan != NULL);
 
 	// Load up the patch.003 file, parse out the instruments
-	Resource *res = resMan->findResource(ResourceId(kResourceTypePatch, 3), 0);
+	Resource *res = resMan->findResource(ResourceId(kResourceTypePatch, 3), false);
 	bool ok = false;
 
 	if (res) {
-		ok = static_cast<MidiDriver_AdLib *>(_driver)->loadResource(res->data, res->size);
+		ok = static_cast<MidiDriver_AdLib *>(_driver)->loadResource(*res);
 	} else {
 		// Early SCI0 games have the sound bank embedded in the AdLib driver
 
@@ -819,13 +822,13 @@ int MidiPlayer_AdLib::open(ResourceManager *resMan) {
 			// Note: Funseeker's Guide also has another version of adl.drv, 8803 bytes.
 			// This isn't supported, but it's not really used anywhere, as that demo
 			// doesn't have sound anyway.
-			if ((size == 5684) || (size == 5720) || (size == 5727)) {
-				byte *buf = new byte[patchSize];
-
-				if (f.seek(0x45a) && (f.read(buf, patchSize) == patchSize))
-					ok = static_cast<MidiDriver_AdLib *>(_driver)->loadResource(buf, patchSize);
-
-				delete[] buf;
+			if (size == 5684 || size == 5720 || size == 5727) {
+				ok = f.seek(0x45a);
+				if (ok) {
+					Common::SpanOwner<SciSpan<const byte> > patchData;
+					patchData->allocateFromStream(f, patchSize);
+					ok = static_cast<MidiDriver_AdLib *>(_driver)->loadResource(*patchData);
+				}
 			}
 		}
 	}
diff --git a/engines/sci/sound/drivers/amigamac.cpp b/engines/sci/sound/drivers/amigamac.cpp
index f5daab726e..b8b6a32214 100644
--- a/engines/sci/sound/drivers/amigamac.cpp
+++ b/engines/sci/sound/drivers/amigamac.cpp
@@ -610,7 +610,7 @@ int MidiDriver_AmigaMac::open() {
 			return Common::kUnknownError;
 		}
 
-		Common::MemoryReadStream stream(resource->data, resource->size);
+		Common::MemoryReadStream stream(resource->toStream());
 
 		if (_isSci1) {
 			if (!loadInstrumentsSCI1(stream))
diff --git a/engines/sci/sound/drivers/cms.cpp b/engines/sci/sound/drivers/cms.cpp
index a222090fc8..8b92432cb9 100644
--- a/engines/sci/sound/drivers/cms.cpp
+++ b/engines/sci/sound/drivers/cms.cpp
@@ -29,6 +29,7 @@
 #include "common/system.h"
 
 #include "sci/resource.h"
+#include "sci/util.h"
 
 namespace Sci {
 
@@ -72,7 +73,7 @@ private:
 	bool _playSwitch;
 	uint16 _masterVolume;
 
-	uint8 *_patchData;
+	Common::SpanOwner<SciSpan<uint8> > _patchData;
 
 	struct Channel {
 		Channel()
@@ -96,7 +97,7 @@ private:
 
 	struct Voice {
 		Voice() : channel(0xFF), note(0xFF), sustained(0xFF), ticks(0),
-		    turnOffTicks(0), patchDataPtr(0), patchDataIndex(0),
+		    turnOffTicks(0), patchDataPtr(), patchDataIndex(0),
 		    amplitudeTimer(0), amplitudeModifier(0), turnOff(false),
 		    velocity(0) {
 		}
@@ -106,7 +107,7 @@ private:
 		uint8 sustained;
 		uint16 ticks;
 		uint16 turnOffTicks;
-		const uint8 *patchDataPtr;
+		SciSpan<uint8> patchDataPtr;
 		uint8 patchDataIndex;
 		uint8 amplitudeTimer;
 		uint8 amplitudeModifier;
@@ -172,12 +173,11 @@ int MidiDriver_CMS::open() {
 		return MERR_ALREADY_OPEN;
 
 	assert(_resMan);
-	Resource *res = _resMan->findResource(ResourceId(kResourceTypePatch, 101), 0);
+	Resource *res = _resMan->findResource(ResourceId(kResourceTypePatch, 101), false);
 	if (!res)
 		return -1;
 
-	_patchData = new uint8[res->size];
-	memcpy(_patchData, res->data, res->size);
+	_patchData->allocateFromSpan(*res);
 
 	for (uint i = 0; i < ARRAYSIZE(_channel); ++i)
 		_channel[i] = Channel();
@@ -218,9 +218,9 @@ int MidiDriver_CMS::open() {
 void MidiDriver_CMS::close() {
 	_mixer->stopHandle(_mixerSoundHandle);
 
-	delete[] _patchData;
+	_patchData.clear();
 	delete _cms;
-	_cms = 0;
+	_cms = nullptr;
 }
 
 void MidiDriver_CMS::send(uint32 b) {
@@ -295,7 +295,7 @@ void MidiDriver_CMS::voiceOn(int voiceNr, int note, int velocity) {
 	voice.amplitudeTimer = 0;
 	voice.ticks = 0;
 	voice.turnOffTicks = 0;
-	voice.patchDataPtr = _patchData + READ_LE_UINT16(&_patchData[_channel[voice.channel].patch * 2]);
+	voice.patchDataPtr = _patchData->subspan(_patchData->getUint16LEAt(_channel[voice.channel].patch * 2));
 	if (velocity)
 		velocity = _velocityTable[(velocity >> 3)];
 	voice.velocity = velocity;
@@ -798,7 +798,7 @@ public:
 		_driver->setTimerCallback(0, 0);
 		_driver->close();
 		delete _driver;
-		_driver = 0;
+		_driver = nullptr;
 	}
 
 	bool hasRhythmChannel() const { return false; }
diff --git a/engines/sci/sound/drivers/fb01.cpp b/engines/sci/sound/drivers/fb01.cpp
index db9f7558e2..3f3f581ee2 100644
--- a/engines/sci/sound/drivers/fb01.cpp
+++ b/engines/sci/sound/drivers/fb01.cpp
@@ -24,6 +24,7 @@
 
 #include "sci/resource.h"
 #include "sci/sound/drivers/mididriver.h"
+#include "sci/util.h"
 
 #include "common/file.h"
 #include "common/system.h"
@@ -71,8 +72,8 @@ private:
 
 	void setVoiceParam(byte voice, byte param, byte value);
 	void setSystemParam(byte sysChan, byte param, byte value);
-	void sendVoiceData(byte instrument, const byte *data);
-	void sendBanks(const byte *data, int size);
+	void sendVoiceData(byte instrument, const SciSpan<const byte> &data);
+	void sendBanks(const SciSpan<const byte> &data);
 	void storeVoiceData(byte instrument, byte bank, byte index);
 	void initVoices();
 
@@ -442,22 +443,22 @@ void MidiPlayer_Fb01::setTimerCallback(void *timer_param, Common::TimerManager::
 	_driver->setTimerCallback(this, midiTimerCallback);
 }
 
-void MidiPlayer_Fb01::sendBanks(const byte *data, int size) {
-	if (size < 3072)
+void MidiPlayer_Fb01::sendBanks(const SciSpan<const byte> &data) {
+	if (data.size() < 3072)
 		error("Failed to read FB-01 patch");
 
 	// SSCI sends bank dumps containing 48 instruments at once. We cannot do that
 	// due to the limited maximum SysEx length. Instead we send the instruments
 	// one by one and store them in the banks.
 	for (int i = 0; i < 48; i++) {
-		sendVoiceData(0, data + i * 64);
+		sendVoiceData(0, data.subspan(i * 64));
 		storeVoiceData(0, 0, i);
 	}
 
 	// Send second bank if available
-	if ((size >= 6146) && (READ_BE_UINT16(data + 3072) == 0xabcd)) {
+	if (data.size() >= 6146 && data.getUint16BEAt(3072) == 0xabcd) {
 		for (int i = 0; i < 48; i++) {
-			sendVoiceData(0, data + 3074 + i * 64);
+			sendVoiceData(0, data.subspan(3074 + i * 64));
 			storeVoiceData(0, 1, i);
 		}
 	}
@@ -479,10 +480,10 @@ int MidiPlayer_Fb01::open(ResourceManager *resMan) {
 	// Turn off memory protection
 	setSystemParam(0, 0x21, 0);
 
-	Resource *res = resMan->findResource(ResourceId(kResourceTypePatch, 2), 0);
+	Resource *res = resMan->findResource(ResourceId(kResourceTypePatch, 2), false);
 
 	if (res) {
-		sendBanks(res->data, res->size);
+		sendBanks(*res);
 	} else {
 		// Early SCI0 games have the sound bank embedded in the IMF driver.
 		// Note that these games didn't actually support the FB-01 as a device,
@@ -494,27 +495,23 @@ int MidiPlayer_Fb01::open(ResourceManager *resMan) {
 		Common::File f;
 
 		if (f.open("IMF.DRV")) {
-			int size = f.size();
-			byte *buf = new byte[size];
-
-			f.read(buf, size);
+			Common::SpanOwner<SciSpan<const byte> > buf;
+			buf->allocateFromStream(f);
 
 			// Search for start of sound bank
-			int offset;
-			for (offset = 0; offset < size; ++offset) {
-				if (!strncmp((char *)buf + offset, "SIERRA ", 7))
+			uint offset;
+			for (offset = 0; offset < buf->size() - 7; ++offset) {
+				if (!strncmp((const char *)buf->getUnsafeDataAt(offset, 7), "SIERRA ", 7))
 					break;
 			}
 
 			// Skip to voice data
 			offset += 0x20;
 
-			if (offset >= size)
+			if (offset >= buf->size())
 				error("Failed to locate start of FB-01 sound bank");
 
-			sendBanks(buf + offset, size - offset);
-
-			delete[] buf;
+			sendBanks(buf->subspan(offset));
 		} else
 			error("Failed to open IMF.DRV");
 	}
@@ -553,7 +550,7 @@ void MidiPlayer_Fb01::setSystemParam(byte sysChan, byte param, byte value) {
 	sysEx(_sysExBuf, 6);
 }
 
-void MidiPlayer_Fb01::sendVoiceData(byte instrument, const byte *data) {
+void MidiPlayer_Fb01::sendVoiceData(byte instrument, const SciSpan<const byte> &data) {
 	_sysExBuf[2] = 0x00;
 	_sysExBuf[3] = 0x08 | instrument;
 	_sysExBuf[4] = 0x00;
diff --git a/engines/sci/sound/drivers/fmtowns.cpp b/engines/sci/sound/drivers/fmtowns.cpp
index f6dbac2a67..270843c396 100644
--- a/engines/sci/sound/drivers/fmtowns.cpp
+++ b/engines/sci/sound/drivers/fmtowns.cpp
@@ -101,7 +101,7 @@ public:
 	~MidiDriver_FMTowns();
 
 	int open();
-	void loadInstruments(const uint8 *data);
+	void loadInstruments(const SciSpan<const uint8> &data);
 	bool isOpen() const { return _isOpen; }
 	void close();
 
@@ -461,14 +461,18 @@ int MidiDriver_FMTowns::open() {
 	return 0;
 }
 
-void MidiDriver_FMTowns::loadInstruments(const uint8 *data) {
-	if (data) {
-		data += 6;
-		for (int i = 0; i < 128; i++) {
-			_intf->callback(5, 0, i, data);
-			data += 48;
+void MidiDriver_FMTowns::loadInstruments(const SciSpan<const uint8> &data) {
+	enum {
+		fmDataSize = 48
+	};
+
+	if (data.size()) {
+		SciSpan<const uint8> instrumentData = data.subspan(6);
+		for (int i = 0; i < 128; i++, instrumentData += fmDataSize) {
+			_intf->callback(5, 0, i, instrumentData.getUnsafeDataAt(0, fmDataSize));
 		}
 	}
+
 	_intf->callback(70, 3);
 	property(MIDI_PROP_MASTER_VOLUME, _masterVolume);
 }
@@ -622,7 +626,7 @@ int MidiPlayer_FMTowns::open(ResourceManager *resMan) {
 	if (_townsDriver) {
 		result = _townsDriver->open();
 		if (!result && _version == SCI_VERSION_1_LATE)
-			_townsDriver->loadInstruments((resMan->findResource(ResourceId(kResourceTypePatch, 8), true))->data);
+			_townsDriver->loadInstruments(*resMan->findResource(ResourceId(kResourceTypePatch, 8), false));
 	}
 	return result;
 }
diff --git a/engines/sci/sound/drivers/midi.cpp b/engines/sci/sound/drivers/midi.cpp
index 7b2b102284..b9035b07ea 100644
--- a/engines/sci/sound/drivers/midi.cpp
+++ b/engines/sci/sound/drivers/midi.cpp
@@ -70,18 +70,18 @@ public:
 	void playSwitch(bool play);
 
 private:
-	bool isMt32GmPatch(const byte *data, int size);
-	void readMt32GmPatch(const byte *data, int size);
-	void readMt32Patch(const byte *data, int size);
+	bool isMt32GmPatch(const SciSpan<const byte> &data);
+	void readMt32GmPatch(const SciSpan<const byte> &data);
+	void readMt32Patch(const SciSpan<const byte> &data);
 	void readMt32DrvData();
 
-	void mapMt32ToGm(byte *data, size_t size);
+	void mapMt32ToGm(const SciSpan<const byte> &data);
 	uint8 lookupGmInstrument(const char *iname);
 	uint8 lookupGmRhythmKey(const char *iname);
 	uint8 getGmInstrument(const Mt32ToGmMap &Mt32Ins);
 
-	void sendMt32SysEx(const uint32 addr, Common::SeekableReadStream *str, int len, bool noDelay);
-	void sendMt32SysEx(const uint32 addr, const byte *buf, int len, bool noDelay);
+	void sendMt32SysEx(const uint32 addr, Common::SeekableReadStream &data, const int len, bool noDelay);
+	void sendMt32SysEx(const uint32 addr, const SciSpan<const byte> &data, bool noDelay);
 	void setMt32Volume(byte volume);
 	void resetMt32();
 
@@ -382,8 +382,9 @@ int MidiPlayer_Midi::getVolume() {
 void MidiPlayer_Midi::setReverb(int8 reverb) {
 	assert(reverb < kReverbConfigNr);
 
-	if (_hasReverb && (_reverb != reverb))
-		sendMt32SysEx(0x100001, _reverbConfig[reverb], 3, true);
+	if (_hasReverb && _reverb != reverb) {
+		sendMt32SysEx(0x100001, SciSpan<const byte>(_reverbConfig[reverb], 3), true);
+	}
 
 	_reverb = reverb;
 }
@@ -398,7 +399,9 @@ void MidiPlayer_Midi::playSwitch(bool play) {
 	}
 }
 
-bool MidiPlayer_Midi::isMt32GmPatch(const byte *data, int size) {
+bool MidiPlayer_Midi::isMt32GmPatch(const SciSpan<const byte> &data) {
+	uint32 size = data.size();
+
 	// WORKAROUND: Some Mac games (e.g. LSL5) may have an extra byte at the
 	// end, so compensate for that here - bug #6725.
 	if (size == 16890)
@@ -419,21 +422,21 @@ bool MidiPlayer_Midi::isMt32GmPatch(const byte *data, int size) {
 
 	// First, check for a GM patch. The presence of MIDI data after the
 	// initial 1153 + 2 bytes indicates a GM patch
-	if (READ_LE_UINT16(data + 1153) + 1155 == size)
+	if (data.getUint16LEAt(1153) + 1155U == size)
 		isMt32Gm = true;
 
 	// Now check for a regular MT-32 patch. Check readMt32Patch() below for
 	// more info.
 	// 491 = 20 + 20 + 20 + 2 + 1 + 11 + 3 * 11 + 256 + 128
 	byte timbresNr = data[491];
-	int pos = 492 + 246 * timbresNr;
+	uint pos = 492 + 246 * timbresNr;
 
 	// Patches 49-96
-	if ((size >= (pos + 386)) && (READ_BE_UINT16(data + pos) == 0xabcd))
+	if (size >= pos + 386 && data.getUint16BEAt(pos) == 0xabcd)
 		pos += 386;	// 256 + 128 + 2
 
 	// Rhythm key map + partial reserve
-	if ((size >= (pos + 267)) && (READ_BE_UINT16(data + pos) == 0xdcba))
+	if (size >= pos + 267 && data.getUint16BEAt(pos) == 0xdcba)
 		pos += 267;	// 256 + 9 + 2
 
 	if (size == pos)
@@ -445,7 +448,7 @@ bool MidiPlayer_Midi::isMt32GmPatch(const byte *data, int size) {
 	return isMt32Gm;
 }
 
-void MidiPlayer_Midi::sendMt32SysEx(const uint32 addr, Common::SeekableReadStream *str, int len, bool noDelay = false) {
+void MidiPlayer_Midi::sendMt32SysEx(const uint32 addr, Common::SeekableReadStream &stream, int len, bool noDelay = false) {
 	if (len + 8 > kMaxSysExSize) {
 		warning("SysEx message exceed maximum size; ignoring");
 		return;
@@ -457,8 +460,7 @@ void MidiPlayer_Midi::sendMt32SysEx(const uint32 addr, Common::SeekableReadStrea
 	_sysExBuf[5] = (addr >> 8) & 0xff;
 	_sysExBuf[6] = addr & 0xff;
 
-	for (int i = 0; i < len; i++)
-		_sysExBuf[7 + i] = str->readByte();
+	stream.read(_sysExBuf + 7, len);
 
 	for (int i = 4; i < 7 + len; i++)
 		chk -= _sysExBuf[i];
@@ -471,81 +473,82 @@ void MidiPlayer_Midi::sendMt32SysEx(const uint32 addr, Common::SeekableReadStrea
 		sysEx(_sysExBuf, len + 8);
 }
 
-void MidiPlayer_Midi::sendMt32SysEx(const uint32 addr, const byte *buf, int len, bool noDelay = false) {
-	Common::MemoryReadStream *str = new Common::MemoryReadStream(buf, len);
-	sendMt32SysEx(addr, str, len, noDelay);
-	delete str;
+void MidiPlayer_Midi::sendMt32SysEx(const uint32 addr, const SciSpan<const byte> &buf, bool noDelay = false) {
+	Common::MemoryReadStream stream(buf.toStream());
+	sendMt32SysEx(addr, stream, buf.size(), noDelay);
 }
 
-void MidiPlayer_Midi::readMt32Patch(const byte *data, int size) {
+
+void MidiPlayer_Midi::readMt32Patch(const SciSpan<const byte> &data) {
 	// MT-32 patch contents:
-	// - 20 bytes unkown
-	// - 20 bytes before-SysEx message
-	// - 20 bytes goodbye SysEx message
-	// - 2 bytes volume
-	// - 1 byte reverb
-	// - 11 bytes reverb Sysex message
-	// - 3 * 11 reverb data
-	// - 256 + 128 bytes patches 1-48
+	// - 0-19        after-SysEx message
+	// - 20-39       before-SysEx message
+	// - 40-59       goodbye SysEx message
+	// - 60-61       volume
+	// - 62          reverb
+	// - 63-73       reverb Sysex message
+	// - 74-106      [3 * 11] reverb data
+	// - 107-490     [256 + 128] patches 1-48
 	// --> total: 491 bytes
-	// - 1 byte number of timbres (64 max)
-	// - 246 * timbres timbre data
-	// - 2 bytes flag (0xabcd)
-	// - 256 + 128 bytes patches 49-96
-	// - 2 bytes flag (0xdcba)
-	// - 256 bytes rhythm key map
-	// - 9 bytes partial reserve
+	// - 491         number of timbres (64 max)
+	// - 492..n      [246 * number of timbres] timbre data
+	// - n-n+1       flag (0xabcd)
+	// - n+2-n+385   [256 + 128] patches 49-96
+	// - n+386-n+387 flag (0xdcba)
+	// - n+388-n+643 rhythm key map
+	// - n+644-n+652 partial reserve
 
-	Common::MemoryReadStream *str = new Common::MemoryReadStream(data, size);
+	Common::MemoryReadStream stream(data.toStream());
 
 	// Send before-SysEx text
-	str->seek(20);
-	sendMt32SysEx(0x200000, str, 20);
+	stream.seek(20);
+	sendMt32SysEx(0x200000, stream, 20);
 
 	// Save goodbye message
-	str->read(_goodbyeMsg, 20);
+	assert(sizeof(_goodbyeMsg) == 20);
+	stream.read(_goodbyeMsg, 20);
 
-	byte volume = CLIP<uint16>(str->readUint16LE(), 0, 100);
+	const uint8 volume = MIN<uint16>(stream.readUint16LE(), 100);
 	setMt32Volume(volume);
 
 	// Reverb default only used in (roughly) SCI0/SCI01
-	byte reverb = str->readByte();
+	byte reverb = stream.readByte();
 
 	_hasReverb = true;
 
 	// Skip reverb SysEx message
-	str->seek(11, SEEK_CUR);
+	stream.seek(11, SEEK_CUR);
 
 	// Read reverb data (stored vertically - patch #3117434)
 	for (int j = 0; j < 3; ++j) {
 		for (int i = 0; i < kReverbConfigNr; i++) {
-			_reverbConfig[i][j] = str->readByte();
+			_reverbConfig[i][j] = stream.readByte();
 		}
 	}
 
 	// Patches 1-48
-	sendMt32SysEx(0x50000, str, 256);
-	sendMt32SysEx(0x50200, str, 128);
+	sendMt32SysEx(0x50000, stream, 256);
+	sendMt32SysEx(0x50200, stream, 128);
 
 	// Timbres
-	byte timbresNr = str->readByte();
+	const uint8 timbresNr = stream.readByte();
 	for (int i = 0; i < timbresNr; i++)
-		sendMt32SysEx(0x80000 + (i << 9), str, 246);
+		sendMt32SysEx(0x80000 + (i << 9), stream, 246);
 
-	uint16 flag = str->readUint16BE();
+	uint16 flag = stream.readUint16BE();
 
-	if (!str->eos() && (flag == 0xabcd)) {
+	if (!stream.eos() && flag == 0xabcd) {
 		// Patches 49-96
-		sendMt32SysEx(0x50300, str, 256);
-		sendMt32SysEx(0x50500, str, 128);
-		flag = str->readUint16BE();
+		sendMt32SysEx(0x50300, stream, 256);
+		sendMt32SysEx(0x50500, stream, 128);
+		flag = stream.readUint16BE();
 	}
 
-	if (!str->eos() && (flag == 0xdcba)) {
+	if (!stream.eos() && flag == 0xdcba) {
 		// Rhythm key map
-		sendMt32SysEx(0x30110, str, 256);
+		sendMt32SysEx(0x30110, stream, 256);
 		// Partial reserve
-		sendMt32SysEx(0x100004, str, 9);
+		sendMt32SysEx(0x100004, stream, 9);
 	}
 
 	// Reverb for SCI0
@@ -553,16 +556,15 @@ void MidiPlayer_Midi::readMt32Patch(const byte *data, int size) {
 		setReverb(reverb);
 
 	// Send after-SysEx text
-	str->seek(0);
-	sendMt32SysEx(0x200000, str, 20);
+	stream.seek(0);
+	sendMt32SysEx(0x200000, stream, 20);
 
 	// Send the mystery SysEx
-	sendMt32SysEx(0x52000a, (const byte *)"\x16\x16\x16\x16\x16\x16", 6);
-
-	delete str;
+	Common::MemoryReadStream mystery((const byte *)"\x16\x16\x16\x16\x16\x16", 6);
+	sendMt32SysEx(0x52000a, mystery, 6);
 }
 
-void MidiPlayer_Midi::readMt32GmPatch(const byte *data, int size) {
+void MidiPlayer_Midi::readMt32GmPatch(const SciSpan<const byte> &data) {
 	// GM patch contents:
 	// - 128 bytes patch map
 	// - 128 bytes key shift
@@ -573,21 +575,21 @@ void MidiPlayer_Midi::readMt32GmPatch(const byte *data, int size) {
 	// - 512 bytes velocity map
 	// --> total: 1153 bytes
 
-	memcpy(_patchMap, data, 128);
-	memcpy(_keyShift, data + 128, 128);
-	memcpy(_volAdjust, data + 256, 128);
-	memcpy(_percussionMap, data + 384, 128);
+	data.subspan(0, sizeof(_patchMap)).unsafeCopyDataTo(_patchMap);
+	data.subspan(128, sizeof(_keyShift)).unsafeCopyDataTo(_keyShift);
+	data.subspan(256, sizeof(_volAdjust)).unsafeCopyDataTo(_volAdjust);
+	data.subspan(384, sizeof(_percussionMap)).unsafeCopyDataTo(_percussionMap);
 	_channels[MIDI_RHYTHM_CHANNEL].volAdjust = data[512];
-	memcpy(_velocityMapIdx, data + 513, 128);
-	memcpy(_velocityMap, data + 641, 512);
+	data.subspan(513, sizeof(_velocityMapIdx)).unsafeCopyDataTo(_velocityMapIdx);
+	data.subspan(641, sizeof(_velocityMap)).unsafeCopyDataTo(_velocityMap);
 
-	uint16 midiSize = READ_LE_UINT16(data + 1153);
+	uint16 midiSize = data.getUint16LEAt(1153);
 
 	if (midiSize > 0) {
-		if (size < midiSize + 1155)
+		if (data.size() < midiSize + 1155U)
 			error("Failed to read MIDI data");
 
-		const byte *midi = data + 1155;
+		const SciSpan<const byte> midi = data.subspan(1155, midiSize);
 		byte command = 0;
 		uint i = 0;
 
@@ -599,15 +601,16 @@ void MidiPlayer_Midi::readMt32GmPatch(const byte *data, int size) {
 
 			switch (command & 0xf0) {
 			case 0xf0: {
-				const byte *sysExEnd = (const byte *)memchr(midi + i, 0xf7, midiSize - i);
+				const byte *sysExStart = midi.getUnsafeDataAt(i, midiSize - i);
+				const byte *sysExEnd = (const byte *)memchr(sysExStart, 0xf7, midiSize - i);
 
 				if (!sysExEnd)
 					error("Failed to find end of sysEx");
 
-				int len = sysExEnd - (midi + i);
-				sysEx(midi + i, len);
+				int len = sysExEnd - sysExStart;
+				sysEx(sysExStart, len);
 
-				i += len + 1; // One more for the 0x7f
+				i += len + 1; // One more for the 0xf7
 				break;
 			}
 			case 0x80:
@@ -656,13 +659,13 @@ void MidiPlayer_Midi::readMt32DrvData() {
 			f.seek(-2, SEEK_CUR);
 
 		// Send before-SysEx text
-		sendMt32SysEx(0x200000, &f, 20);
+		sendMt32SysEx(0x200000, f, 20);
 
 		if (size != 2271) {
 			// Send after-SysEx text (SSCI sends this before every song).
 			// There aren't any SysEx calls in old drivers, so this can
 			// be sent right after the before-SysEx text.
-			sendMt32SysEx(0x200000, &f, 20);
+			sendMt32SysEx(0x200000, f, 20);
 		} else {
 			// Skip the after-SysEx text in the newer patch version, we'll send
 			// it after the SysEx messages are sent.
@@ -696,14 +699,14 @@ void MidiPlayer_Midi::readMt32DrvData() {
 			f.skip(2235);	// skip driver code
 
 			// Patches 1-48
-			sendMt32SysEx(0x50000, &f, 256);
-			sendMt32SysEx(0x50200, &f, 128);
+			sendMt32SysEx(0x50000, f, 256);
+			sendMt32SysEx(0x50200, f, 128);
 
 			setReverb(reverb);
 
 			// Send the after-SysEx text
 			f.seek(0x3d);
-			sendMt32SysEx(0x200000, &f, 20);
+			sendMt32SysEx(0x200000, f, 20);
 		} else {
 			byte reverbSysEx[13];
 			// This old driver should have a full reverb SysEx
@@ -775,11 +778,11 @@ uint8 MidiPlayer_Midi::getGmInstrument(const Mt32ToGmMap &Mt32Ins) {
 		return Mt32Ins.gmInstr;
 }
 
-void MidiPlayer_Midi::mapMt32ToGm(byte *data, size_t size) {
+void MidiPlayer_Midi::mapMt32ToGm(const SciSpan<const byte> &data) {
 	// FIXME: Clean this up
 	int memtimbres, patches;
 	uint8 group, number, keyshift, /*finetune,*/ bender_range;
-	uint8 *patchpointer;
+	SciSpan<const byte> patchpointer;
 	uint32 pos;
 	int i;
 
@@ -791,10 +794,10 @@ void MidiPlayer_Midi::mapMt32ToGm(byte *data, size_t size) {
 	for (i = 0; i < 128; i++)
 		_percussionMap[i] = Mt32PresetRhythmKeymap[i];
 
-	memtimbres = *(data + 0x1eb);
+	memtimbres = data[0x1eb];
 	pos = 0x1ec + memtimbres * 0xf6;
 
-	if (size > pos && ((0x100 * *(data + pos) + *(data + pos + 1)) == 0xabcd)) {
+	if (data.size() > pos && data.getUint16BEAt(pos) == 0xabcd) {
 		patches = 96;
 		pos += 2 + 8 * 48;
 	} else {
@@ -807,18 +810,18 @@ void MidiPlayer_Midi::mapMt32ToGm(byte *data, size_t size) {
 	debugC(kDebugLevelSound, "\n[MT32-to-GM] Mapping patches..");
 
 	for (i = 0; i < patches; i++) {
-		char name[11];
+		Common::String name;
 
 		if (i < 48)
-			patchpointer = data + 0x6b + 8 * i;
+			patchpointer = data.subspan(0x6b + 8 * i);
 		else
-			patchpointer = data + 0x1ec + 8 * (i - 48) + memtimbres * 0xf6 + 2;
+			patchpointer = data.subspan(0x1ec + 8 * (i - 48) + memtimbres * 0xf6 + 2);
 
-		group = *patchpointer;
-		number = *(patchpointer + 1);
-		keyshift = *(patchpointer + 2);
-		//finetune = *(patchpointer + 3);
-		bender_range = *(patchpointer + 4);
+		group = patchpointer[0];
+		number = patchpointer[1];
+		keyshift = patchpointer[2];
+		//finetune = patchpointer[3];
+		bender_range = patchpointer[4];
 
 		debugCN(kDebugLevelSound, "  [%03d] ", i);
 
@@ -832,10 +835,9 @@ void MidiPlayer_Midi::mapMt32ToGm(byte *data, size_t size) {
 			break;
 		case 2:
 			if (number < memtimbres) {
-				strncpy(name, (const char *)data + 0x1ec + number * 0xf6, 10);
-				name[10] = 0;
-				_patchMap[i] = lookupGmInstrument(name);
-				debugCN(kDebugLevelSound, "%s -> ", name);
+				name = data.getStringAt(0x1ec + number * 0xf6, 10);
+				_patchMap[i] = lookupGmInstrument(name.c_str());
+				debugCN(kDebugLevelSound, "%s -> ", name.c_str());
 			} else {
 				_patchMap[i] = 0xff;
 				debugCN(kDebugLevelSound, "[Invalid]  -> ");
@@ -865,21 +867,19 @@ void MidiPlayer_Midi::mapMt32ToGm(byte *data, size_t size) {
 		_pitchBendRange[i] = CLIP<uint8>(bender_range, 0, 24);
 	}
 
-	if (size > pos && ((0x100 * *(data + pos) + *(data + pos + 1)) == 0xdcba)) {
+	if (data.size() > pos && data.getUint16BEAt(pos) == 0xdcba) {
 		debugC(kDebugLevelSound, "\n[MT32-to-GM] Mapping percussion..");
 
 		for (i = 0; i < 64; i++) {
-			number = *(data + pos + 4 * i + 2);
+			number = data[pos + 4 * i + 2];
 			byte ins = i + 24;
 
 			debugCN(kDebugLevelSound, "  [%03d] ", ins);
 
 			if (number < 64) {
-				char name[11];
-				strncpy(name, (const char *)data + 0x1ec + number * 0xf6, 10);
-				name[10] = 0;
-				debugCN(kDebugLevelSound, "%s -> ", name);
-				_percussionMap[ins] = lookupGmRhythmKey(name);
+				Common::String name = data.getStringAt(0x1ec + number * 0xf6, 10);
+				debugCN(kDebugLevelSound, "%s -> ", name.c_str());
+				_percussionMap[ins] = lookupGmRhythmKey(name.c_str());
 			} else {
 				if (number < 94) {
 					debugCN(kDebugLevelSound, "%s -> ", Mt32RhythmTimbreMaps[number - 64].name);
@@ -897,17 +897,19 @@ void MidiPlayer_Midi::mapMt32ToGm(byte *data, size_t size) {
 				debugC(kDebugLevelSound, "%s", GmPercussionNames[_percussionMap[ins]]);
 #endif
 
-			_percussionVelocityScale[ins] = *(data + pos + 4 * i + 3) * 127 / 100;
+			_percussionVelocityScale[ins] = data[pos + 4 * i + 3] * 127 / 100;
 		}
 	}
 }
 
 void MidiPlayer_Midi::setMt32Volume(byte volume) {
-	sendMt32SysEx(0x100016, &volume, 1);
+	Common::MemoryReadStream s(&volume, 1);
+	sendMt32SysEx(0x100016, s, 1);
 }
 
 void MidiPlayer_Midi::resetMt32() {
-	sendMt32SysEx(0x7f0000, (const byte *)"\x01\x00", 2, true);
+	Common::MemoryReadStream s((const byte *)"\x01\x00", 2);
+	sendMt32SysEx(0x7f0000, s, 2, true);
 
 	// This seems to require a longer delay than usual
 	g_system->delayMillis(150);
@@ -937,11 +939,11 @@ int MidiPlayer_Midi::open(ResourceManager *resMan) {
 		_percussionVelocityScale[i] = 127;
 	}
 
-	Resource *res = NULL;
+	Resource *res = nullptr;
 
 	if (g_sci && g_sci->_features->useAltWinGMSound()) {
-		res = resMan->findResource(ResourceId(kResourceTypePatch, 4), 0);
-		if (!(res && isMt32GmPatch(res->data, res->size))) {
+		res = resMan->findResource(ResourceId(kResourceTypePatch, 4), false);
+		if (!res || !isMt32GmPatch(*res)) {
 			// Don't do any mapping when a Windows alternative track is selected
 			// and no MIDI patch is available
 			_useMT32Track = false;
@@ -953,15 +955,15 @@ int MidiPlayer_Midi::open(ResourceManager *resMan) {
 		// MT-32
 		resetMt32();
 
-		res = resMan->findResource(ResourceId(kResourceTypePatch, 1), 0);
+		res = resMan->findResource(ResourceId(kResourceTypePatch, 1), false);
 
 		if (res) {
-			if (isMt32GmPatch(res->data, res->size)) {
-				readMt32GmPatch(res->data, res->size);
+			if (isMt32GmPatch(*res)) {
+				readMt32GmPatch(*res);
 				// Note that _goodbyeMsg is not zero-terminated
 				memcpy(_goodbyeMsg, "      ScummVM       ", 20);
 			} else {
-				readMt32Patch(res->data, res->size);
+				readMt32Patch(*res);
 			}
 		} else {
 			// Early SCI0 games have the sound bank embedded in the MT-32 driver
@@ -969,22 +971,22 @@ int MidiPlayer_Midi::open(ResourceManager *resMan) {
 		}
 	} else {
 		// General MIDI
-		res = resMan->findResource(ResourceId(kResourceTypePatch, 4), 0);
+		res = resMan->findResource(ResourceId(kResourceTypePatch, 4), false);
 
-		if (res && isMt32GmPatch(res->data, res->size)) {
+		if (res && isMt32GmPatch(*res)) {
 			// There is a GM patch
-			readMt32GmPatch(res->data, res->size);
+			readMt32GmPatch(*res);
 
 			if (g_sci && g_sci->_features->useAltWinGMSound()) {
 				// Always use the GM track if an alternative GM Windows soundtrack is selected
 				_useMT32Track = false;
 			} else {
 				// Detect the format of patch 1, so that we know what play mask to use
-				res = resMan->findResource(ResourceId(kResourceTypePatch, 1), 0);
+				res = resMan->findResource(ResourceId(kResourceTypePatch, 1), false);
 				if (!res)
 					_useMT32Track = false;
 				else
-					_useMT32Track = !isMt32GmPatch(res->data, res->size);
+					_useMT32Track = !isMt32GmPatch(*res);
 
 				// Check if the songs themselves have a GM track
 				if (!_useMT32Track) {
@@ -1013,17 +1015,17 @@ int MidiPlayer_Midi::open(ResourceManager *resMan) {
 				_velocityMap[3][i] = 0x20 + (i - 1) / 2;
 			}
 
-			res = resMan->findResource(ResourceId(kResourceTypePatch, 1), 0);
+			res = resMan->findResource(ResourceId(kResourceTypePatch, 1), false);
 
 			if (res) {
-				if (!isMt32GmPatch(res->data, res->size)) {
-					mapMt32ToGm(res->data, res->size);
+				if (!isMt32GmPatch(*res)) {
+					mapMt32ToGm(*res);
 				} else {
 					if (getSciVersion() < SCI_VERSION_3) {
 						error("MT-32 patch has wrong type");
 					} else {
 						// Happens in the SCI3 interactive demo of Lighthouse
-						warning("TODO: Ignoring new SCI3 type of MT-32 patch for now (size = %d)", res->size);
+						warning("TODO: Ignoring new SCI3 type of MT-32 patch for now (size = %lu)", res->size());
 					}
 				}
 			} else {
@@ -1053,7 +1055,7 @@ int MidiPlayer_Midi::open(ResourceManager *resMan) {
 void MidiPlayer_Midi::close() {
 	if (_isMt32) {
 		// Send goodbye message
-		sendMt32SysEx(0x200000, _goodbyeMsg, 20, true);
+		sendMt32SysEx(0x200000, SciSpan<const byte>(_goodbyeMsg, 20), true);
 	}
 
 	_driver->close();