SCI: Clean up some memory management and loops

7 files changed, 55 insertions, 90 deletions

diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 571d2f834b..395ba4a8c7 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -860,14 +860,14 @@ bool Console::cmdVerifyScripts(int argc, const char **argv) {
 		return true;
 	}
 
-	Common::List<ResourceId> *resources = _engine->getResMan()->listResources(kResourceTypeScript);
-	Common::sort(resources->begin(), resources->end());
-	Common::List<ResourceId>::iterator itr = resources->begin();
+	Common::List<ResourceId> resources = _engine->getResMan()->listResources(kResourceTypeScript);
+	Common::sort(resources.begin(), resources.end());
 
-	DebugPrintf("%d SCI1.1-SCI3 scripts found, performing sanity checks...\n", resources->size());
+	DebugPrintf("%d SCI1.1-SCI3 scripts found, performing sanity checks...\n", resources.size());
 
 	Resource *script, *heap;
-	while (itr != resources->end()) {
+	Common::List<ResourceId>::iterator itr;
+	for (itr = resources.begin(); itr != resources.end(); ++itr) {
 		script = _engine->getResMan()->findResource(*itr, false);
 		if (!script)
 			DebugPrintf("Error: script %d couldn't be loaded\n", itr->getNumber());
@@ -885,12 +885,9 @@ bool Console::cmdVerifyScripts(int argc, const char **argv) {
 				DebugPrintf("Error: script %d is larger than 64KB (%d bytes)\n",
 				itr->getNumber(), script->size);
 		}
-
-		++itr;
 	}
 
 	DebugPrintf("SCI1.1-SCI2.1 script check finished\n");
-	delete resources;
 
 	return true;
 }
@@ -914,9 +911,8 @@ bool Console::cmdShowInstruments(int argc, const char **argv) {
 	MidiParser_SCI *parser = new MidiParser_SCI(doSoundVersion, 0);
 	parser->setMidiDriver(player);
 
-	Common::List<ResourceId> *resources = _engine->getResMan()->listResources(kResourceTypeSound);
-	Common::sort(resources->begin(), resources->end());
-	Common::List<ResourceId>::iterator itr = resources->begin();
+	Common::List<ResourceId> resources = _engine->getResMan()->listResources(kResourceTypeSound);
+	Common::sort(resources.begin(), resources.end());
 	int instruments[128];
 	bool instrumentsSongs[128][1000];
 
@@ -928,26 +924,21 @@ bool Console::cmdShowInstruments(int argc, const char **argv) {
 			instrumentsSongs[i][j] = false;
 
 	if (songNumber == -1) {
-		DebugPrintf("%d sounds found, checking their instrument mappings...\n", resources->size());
+		DebugPrintf("%d sounds found, checking their instrument mappings...\n", resources.size());
 		DebugPrintf("Instruments:\n");
 		DebugPrintf("============\n");
 	}
 
-	SoundResource *sound;
-
-	while (itr != resources->end()) {
-		if (songNumber >= 0 && itr->getNumber() != songNumber) {
-			++itr;
+	Common::List<ResourceId>::iterator itr;
+	for (itr = resources.begin(); itr != resources.end(); ++itr) {
+		if (songNumber >= 0 && itr->getNumber() != songNumber)
 			continue;
-		}
 
-		sound = new SoundResource(itr->getNumber(), _engine->getResMan(), doSoundVersion);
-		int channelFilterMask = sound->getChannelFilterMask(player->getPlayId(), player->hasRhythmChannel());
-		SoundResource::Track *track = sound->getTrackByType(player->getPlayId());
+		SoundResource sound(itr->getNumber(), _engine->getResMan(), doSoundVersion);
+		int channelFilterMask = sound.getChannelFilterMask(player->getPlayId(), player->hasRhythmChannel());
+		SoundResource::Track *track = sound.getTrackByType(player->getPlayId());
 		if (track->digitalChannelNr != -1) {
 			// Skip digitized sound effects
-			delete sound;
-			++itr;
 			continue;
 		}
 
@@ -1027,9 +1018,6 @@ bool Console::cmdShowInstruments(int argc, const char **argv) {
 		} while (!endOfTrack);
 
 		DebugPrintf("\n");
-
-		delete sound;
-		++itr;
 	}
 
 	delete parser;
@@ -1069,7 +1057,6 @@ bool Console::cmdShowInstruments(int argc, const char **argv) {
 		DebugPrintf("\n\n");
 	}
 
-	delete resources;
 	return true;
 }
 
@@ -1132,12 +1119,12 @@ bool Console::cmdList(int argc, const char **argv) {
 			number = atoi(argv[2]);
 		}
 
-		Common::List<ResourceId> *resources = _engine->getResMan()->listResources(res, number);
-		Common::sort(resources->begin(), resources->end());
-		Common::List<ResourceId>::iterator itr = resources->begin();
+		Common::List<ResourceId> resources = _engine->getResMan()->listResources(res, number);
+		Common::sort(resources.begin(), resources.end());
 
 		int cnt = 0;
-		while (itr != resources->end()) {
+		Common::List<ResourceId>::iterator itr;
+		for (itr = resources.begin(); itr != resources.end(); ++itr) {
 			if (number == -1) {
 				DebugPrintf("%8i", itr->getNumber());
 				if (++cnt % 10 == 0)
@@ -1149,10 +1136,8 @@ bool Console::cmdList(int argc, const char **argv) {
 				if (++cnt % 4 == 0)
 					DebugPrintf("\n");
 			}
-			++itr;
 		}
 		DebugPrintf("\n");
-		delete resources;
 	}
 
 	return true;
@@ -2915,12 +2900,11 @@ bool Console::cmdDisassembleAddress(int argc, const char **argv) {
 }
 
 void Console::printKernelCallsFound(int kernelFuncNum, bool showFoundScripts) {
-	Common::List<ResourceId> *resources = _engine->getResMan()->listResources(kResourceTypeScript);
-	Common::sort(resources->begin(), resources->end());
-	Common::List<ResourceId>::iterator itr = resources->begin();
+	Common::List<ResourceId> resources = _engine->getResMan()->listResources(kResourceTypeScript);
+	Common::sort(resources.begin(), resources.end());
 
 	if (showFoundScripts)
-		DebugPrintf("%d scripts found, dissassembling...\n", resources->size());
+		DebugPrintf("%d scripts found, dissassembling...\n", resources.size());
 
 	int scriptSegment;
 	Script *script;
@@ -2928,13 +2912,13 @@ void Console::printKernelCallsFound(int kernelFuncNum, bool showFoundScripts) {
 	// manager won't be affected by loading and unloading scripts here.
 	SegManager *customSegMan = new SegManager(_engine->getResMan());
 
-	while (itr != resources->end()) {
+	Common::List<ResourceId>::iterator itr;
+	for (itr = resources.begin(); itr != resources.end(); ++itr) {
 		// Ignore specific leftover scripts, which require other non-existing scripts
 		if ((_engine->getGameId() == GID_HOYLE3         && itr->getNumber() == 995) ||
 		    (_engine->getGameId() == GID_KQ5            && itr->getNumber() == 980) ||
 		    (_engine->getGameId() == GID_SLATER         && itr->getNumber() == 947) ||
 			(_engine->getGameId() == GID_MOTHERGOOSE256 && itr->getNumber() == 980)) {
-			itr++;
 			continue;
 		}
 
@@ -2993,12 +2977,9 @@ void Console::printKernelCallsFound(int kernelFuncNum, bool showFoundScripts) {
 		}	// for (it = script->_objects.begin(); it != end; ++it)
 
 		customSegMan->uninstantiateScript(itr->getNumber());
-		++itr;
 	}
 
 	delete customSegMan;
-
-	delete resources;
 }
 
 bool Console::cmdFindKernelFunctionCall(int argc, const char **argv) {
diff --git a/engines/sci/detection.cpp b/engines/sci/detection.cpp
index 80f45b4325..c43849056b 100644
--- a/engines/sci/detection.cpp
+++ b/engines/sci/detection.cpp
@@ -303,29 +303,29 @@ Common::String convertSierraGameId(Common::String sierraId, uint32 *gameFlags, R
 	if (sierraId == "fp" || sierraId == "gk" || sierraId == "pq4")
 		demoThreshold = 150;
 
-	Common::ScopedPtr<Common::List<ResourceId> > resources(resMan.listResources(kResourceTypeScript, -1));
-	if (resources->size() < demoThreshold) {
+	Common::List<ResourceId> resources = resMan.listResources(kResourceTypeScript, -1);
+	if (resources.size() < demoThreshold) {
 		*gameFlags |= ADGF_DEMO;
 
 		// Crazy Nick's Picks
-		if (sierraId == "lsl1" && resources->size() == 34)
+		if (sierraId == "lsl1" && resources.size() == 34)
 			return "cnick-lsl";
-		if (sierraId == "sq4" && resources->size() == 34)
+		if (sierraId == "sq4" && resources.size() == 34)
 			return "cnick-sq";
-		if (sierraId == "hoyle3" && resources->size() == 42)
+		if (sierraId == "hoyle3" && resources.size() == 42)
 			return "cnick-kq";
-		if (sierraId == "rh budget" && resources->size() == 39)
+		if (sierraId == "rh budget" && resources.size() == 39)
 			return "cnick-longbow";
 		// TODO: cnick-laurabow (the name of the game object contains junk)
 
 		// Handle Astrochicken 1 (SQ3) and 2 (SQ4)
-		if (sierraId == "sq3" && resources->size() == 20)
+		if (sierraId == "sq3" && resources.size() == 20)
 			return "astrochicken";
 		if (sierraId == "sq4")
 			return "msastrochicken";
 	}
 
-	if (sierraId == "torin" && resources->size() == 226)	// Torin's Passage demo
+	if (sierraId == "torin" && resources.size() == 226)	// Torin's Passage demo
 		*gameFlags |= ADGF_DEMO;
 
 	for (const OldNewIdTableEntry *cur = s_oldNewTable; cur->oldId[0]; ++cur) {
@@ -350,7 +350,7 @@ Common::String convertSierraGameId(Common::String sierraId, uint32 *gameFlags, R
 			return "qfg4";
 
 		// qfg4 demo has less than 50 scripts
-		if (resources->size() < 50)
+		if (resources.size() < 50)
 			return "qfg4";
 
 		// Otherwise it's qfg3
diff --git a/engines/sci/engine/features.cpp b/engines/sci/engine/features.cpp
index a5c1b970f1..b3cfee873c 100644
--- a/engines/sci/engine/features.cpp
+++ b/engines/sci/engine/features.cpp
@@ -430,19 +430,16 @@ SciVersion GameFeatures::detectMessageFunctionType() {
 		return _messageFunctionType;
 	}
 
-	Common::List<ResourceId> *resources = g_sci->getResMan()->listResources(kResourceTypeMessage, -1);
-
-	if (resources->empty()) {
-		delete resources;
+	Common::List<ResourceId> resources = g_sci->getResMan()->listResources(kResourceTypeMessage, -1);
 
+	if (resources.empty()) {
 		// No messages found, so this doesn't really matter anyway...
 		_messageFunctionType = SCI_VERSION_1_1;
 		return _messageFunctionType;
 	}
 
-	Resource *res = g_sci->getResMan()->findResource(*resources->begin(), false);
+	Resource *res = g_sci->getResMan()->findResource(*resources.begin(), false);
 	assert(res);
-	delete resources;
 
 	// Only v2 Message resources use the kGetMessage kernel function.
 	// v3-v5 use the kMessage kernel function.
diff --git a/engines/sci/engine/kscripts.cpp b/engines/sci/engine/kscripts.cpp
index 93c1fffe3c..9b0e490d7e 100644
--- a/engines/sci/engine/kscripts.cpp
+++ b/engines/sci/engine/kscripts.cpp
@@ -74,17 +74,13 @@ reg_t kLock(EngineState *s, int argc, reg_t *argv) {
 	case 0 :
 		if (id.getNumber() == 0xFFFF) {
 			// Unlock all resources of the requested type
-			Common::List<ResourceId> *resources = g_sci->getResMan()->listResources(type);
-			Common::List<ResourceId>::iterator itr = resources->begin();
-
-			while (itr != resources->end()) {
+			Common::List<ResourceId> resources = g_sci->getResMan()->listResources(type);
+			Common::List<ResourceId>::iterator itr;
+			for (itr = resources.begin(); itr != resources.end(); ++itr) {
 				Resource *res = g_sci->getResMan()->testResource(*itr);
 				if (res->isLocked())
 					g_sci->getResMan()->unlockResource(res);
-				++itr;
 			}
-
-			delete resources;
 		} else {
 			which = g_sci->getResMan()->findResource(id, 0);
 
diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index c3bd03bac3..41062eb19b 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -753,12 +753,10 @@ void ResourceManager::addScriptChunkSources() {
 		// to try to get to any scripts in there. The Lighthouse SCI2.1 demo
 		// does exactly this.
 
-		Common::List<ResourceId> *resources = listResources(kResourceTypeScript);
+		Common::List<ResourceId> resources = listResources(kResourceTypeScript);
 
-		if (resources->empty() && testResource(ResourceId(kResourceTypeChunk, 0)))
+		if (resources.empty() && testResource(ResourceId(kResourceTypeChunk, 0)))
 			addResourcesFromChunk(0);
-
-		delete resources;
 	}
 #endif
 }
@@ -1045,13 +1043,13 @@ void ResourceManager::freeOldResources() {
 	}
 }
 
-Common::List<ResourceId> *ResourceManager::listResources(ResourceType type, int mapNumber) {
-	Common::List<ResourceId> *resources = new Common::List<ResourceId>;
+Common::List<ResourceId> ResourceManager::listResources(ResourceType type, int mapNumber) {
+	Common::List<ResourceId> resources;
 
 	ResourceMap::iterator itr = _resMap.begin();
 	while (itr != _resMap.end()) {
 		if ((itr->_value->getType() == type) && ((mapNumber == -1) || (itr->_value->getNumber() == mapNumber)))
-			resources->push_back(itr->_value->_id);
+			resources.push_back(itr->_value->_id);
 		++itr;
 	}
 
@@ -2199,9 +2197,8 @@ void ResourceManager::detectSciVersion() {
 
 	// Handle SCI32 versions here
 	if (_volVersion >= kResVersionSci2) {
-		Common::List<ResourceId> *heaps = listResources(kResourceTypeHeap);
-		bool hasHeapResources = !heaps->empty();
-		delete heaps;
+		Common::List<ResourceId> heaps = listResources(kResourceTypeHeap);
+		bool hasHeapResources = !heaps.empty();
 
 		// SCI2.1/3 and SCI1 Late resource maps are the same, except that
 		// SCI1 Late resource maps have the resource types or'd with
diff --git a/engines/sci/resource.h b/engines/sci/resource.h
index 47602de017..294a4672e2 100644
--- a/engines/sci/resource.h
+++ b/engines/sci/resource.h
@@ -322,7 +322,7 @@ public:
 	 * @param mapNumber	For audio36 and sync36, limit search to this map
 	 * @return			The resource list
 	 */
-	Common::List<ResourceId> *listResources(ResourceType type, int mapNumber = -1);
+	Common::List<ResourceId> listResources(ResourceType type, int mapNumber = -1);
 
 	void setAudioLanguage(int language);
 	int getAudioLanguage() const;
diff --git a/engines/sci/resource_audio.cpp b/engines/sci/resource_audio.cpp
index 8730580302..764f4785b7 100644
--- a/engines/sci/resource_audio.cpp
+++ b/engines/sci/resource_audio.cpp
@@ -539,11 +539,10 @@ bool ResourceManager::isGMTrackIncluded() {
 
 	// Read the first song and check if it has a GM track
 	bool result = false;
-	Common::List<ResourceId> *resources = listResources(kResourceTypeSound, -1);
-	Common::sort(resources->begin(), resources->end());
-	Common::List<ResourceId>::iterator itr = resources->begin();
+	Common::List<ResourceId> resources = listResources(kResourceTypeSound, -1);
+	Common::sort(resources.begin(), resources.end());
+	Common::List<ResourceId>::iterator itr = resources.begin();
 	int firstSongId = itr->getNumber();
-	delete resources;
 
 	SoundResource *song1 = new SoundResource(firstSongId, this, soundVersion);
 	if (!song1) {
@@ -893,10 +892,10 @@ void AudioVolumeResourceSource::loadResource(ResourceManager *resMan, Resource *
 }
 
 bool ResourceManager::addAudioSources() {
-	Common::List<ResourceId> *resources = listResources(kResourceTypeMap);
-	Common::List<ResourceId>::iterator itr = resources->begin();
+	Common::List<ResourceId> resources = listResources(kResourceTypeMap);
+	Common::List<ResourceId>::iterator itr;
 
-	while (itr != resources->end()) {
+	for (itr = resources.begin(); itr != resources.end(); ++itr) {
 		ResourceSource *src = addSource(new IntMapResourceSource("MAP", itr->getNumber()));
 
 		if ((itr->getNumber() == 65535) && Common::File::exists("RESOURCE.SFX"))
@@ -905,12 +904,8 @@ bool ResourceManager::addAudioSources() {
 			addSource(new AudioVolumeResourceSource(this, "RESOURCE.AUD", src, 0));
 		else
 			return false;
-
-		++itr;
 	}
 
-	delete resources;
-
 	return true;
 }
 
@@ -944,8 +939,9 @@ void ResourceManager::changeAudioDirectory(Common::String path) {
 		audioResourceName = Common::String::format("%s/RESOURCE.AUD", path.c_str());
 	}
 
-	Common::List<ResourceId> *resources = listResources(kResourceTypeMap);
-	for (Common::List<ResourceId>::iterator it = resources->begin(); it != resources->end(); ++it) {
+	Common::List<ResourceId> resources = listResources(kResourceTypeMap);
+	Common::List<ResourceId>::iterator it;
+	for (it = resources.begin(); it != resources.end(); ++it) {
 		// Don't readd 65535.map or resource.sfx
 		if ((it->getNumber() == 65535))
 			continue;
@@ -954,8 +950,6 @@ void ResourceManager::changeAudioDirectory(Common::String path) {
 		addSource(new AudioVolumeResourceSource(this, audioResourceName, src, 0));
 	}
 
-	delete resources;
-
 	// Rescan the newly added resources
 	scanNewSources();
 }