ALL: Load savegame thumbnail only when necessary

This commit introduces the following changes:

1. Graphics::loadThumbnail()

   Now returns a boolean and takes a new argument skipThumbnail which
   defaults to false. In case of true, loadThumbnail() reads past the
   thumbnail data in the input stream instead of actually loading the
   thumbnail. This simplifies savegame handling where, up until now,
   many engines always read the whole savegame metadata (including
   the thumbnail) and then threw away the thumbnail when not needed
   (which is in almost all cases, the most common exception being
   MetaEngine::querySaveMetaInfos() which is responsible for loading
   savegame metadata for displaying it in the GUI launcher.

2. readSavegameHeader()

   Engines which already implement such a method (name varies) now take
   a new argument skipThumbnail (default: true) which is passed
   through to loadThumbnail(). This means that the default case for
   readSavegameHeader() is now _not_ loading the thumbnail from a
   savegame and just reading past it. In those cases, e.g.
   querySaveMetaInfos(), where we actually are interested in loading
   the thumbnail readSavegameHeader() needs to explicitely be called
   with skipThumbnail == false.

   Engines whose readSavegameHeader() (name varies) already takes an
   argument loadThumbnail have been adapted to have a similar
   prototype and semantics.
   I.e. readSaveHeader(in, loadThumbnail, header) now is
   readSaveHeader(in, header, skipThumbnail).

3. Error handling

   Engines which previously did not check the return value of
   readSavegameHeader() (name varies) now do so ensuring that possibly
   broken savegames (be it a broken thumbnail or something else) don't
   make it into the GUI launcher list in the first place.

common/recorderfile.cpp

@@ -608,7 +608,8 @@ Graphics::Surface *PlaybackFile::getScreenShot(int number) {
 		if (screenCount == number) {
 			screenCount++;
 			_readStream->seek(-4, SEEK_CUR);
-			return Graphics::loadThumbnail(*_readStream);
+			Graphics::Surface *thumbnail;
+			return Graphics::loadThumbnail(*_readStream, thumbnail) ? thumbnail : NULL;
 		} else {
 			uint32 size = _readStream->readUint32BE();
 			_readStream->skip(size-8);

engines/access/access.cpp

@@ -488,11 +488,6 @@ Common::Error AccessEngine::loadGameState(int slot) {
 	if (!readSavegameHeader(saveFile, header))
 		error("Invalid savegame");
 
-	if (header._thumbnail) {
-		header._thumbnail->free();
-		delete header._thumbnail;
-	}
-
 	// Load most of the savegame data
 	synchronize(s);
 	delete saveFile;
@@ -537,9 +532,8 @@ void AccessEngine::synchronize(Common::Serializer &s) {
 const char *const SAVEGAME_STR = "ACCESS";
 #define SAVEGAME_STR_SIZE 6
 
-bool AccessEngine::readSavegameHeader(Common::InSaveFile *in, AccessSavegameHeader &header) {
+WARN_UNUSED_RESULT bool AccessEngine::readSavegameHeader(Common::InSaveFile *in, AccessSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[SAVEGAME_STR_SIZE + 1];
-	header._thumbnail = nullptr;
 
 	// Validate the header Id
 	in->read(saveIdentBuffer, SAVEGAME_STR_SIZE + 1);
@@ -557,9 +551,9 @@ bool AccessEngine::readSavegameHeader(Common::InSaveFile *in, AccessSavegameHead
 		header._saveName += ch;
 
 	// Get the thumbnail
-	header._thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header._thumbnail, skipThumbnail)) {
-	if (!header._thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header._year = in->readSint16LE();

engines/access/access.h

@@ -306,7 +306,7 @@ public:
 	/**
 	 * Read in a savegame header
 	 */
-	static bool readSavegameHeader(Common::InSaveFile *in, AccessSavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, AccessSavegameHeader &header, bool skipThumbnail = true);
 
 	/**
 	 * Write out a savegame header

engines/access/detection.cpp

@@ -157,11 +157,9 @@ SaveStateList AccessMetaEngine::listSaves(const char *target) const {
 			Common::InSaveFile *in = g_system->getSavefileManager()->openForLoading(*file);
 
 			if (in) {
-				Access::AccessEngine::readSavegameHeader(in, header);
+				if (Access::AccessEngine::readSavegameHeader(in, header))
-				saveList.push_back(SaveStateDescriptor(slot, header._saveName));
+					saveList.push_back(SaveStateDescriptor(slot, header._saveName));
 
-				header._thumbnail->free();
-				delete header._thumbnail;
 				delete in;
 			}
 		}
@@ -187,7 +185,11 @@ SaveStateDescriptor AccessMetaEngine::querySaveMetaInfos(const char *target, int
 
 	if (f) {
 		Access::AccessSavegameHeader header;
-		Access::AccessEngine::readSavegameHeader(f, header);
+		if (!Access::AccessEngine::readSavegameHeader(f, header, false)) {
+			delete f;
+			return SaveStateDescriptor();
+		}
+
 		delete f;
 
 		// Create the return descriptor

engines/adl/detection.cpp

@@ -401,7 +401,11 @@ SaveStateDescriptor AdlMetaEngine::querySaveMetaInfos(const char *target, int sl
 		return SaveStateDescriptor();
 	}
 
-	Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*inFile);
+	Graphics::Surface *thumbnail;
+	if (!Graphics::loadThumbnail(*inFile, thumbnail)) {
+		delete inFile;
+		return SaveStateDescriptor();
+	}
 	sd.setThumbnail(thumbnail);
 
 	delete inFile;

engines/agi/detection.cpp

@@ -375,7 +375,11 @@ SaveStateDescriptor AgiMetaEngine::querySaveMetaInfos(const char *target, int sl
 
 		char saveVersion = in->readByte();
 		if (saveVersion >= 4) {
-			Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*in);
+			Graphics::Surface *thumbnail;
+			if (!Graphics::loadThumbnail(*in, thumbnail)) {
+				delete in;
+				return SaveStateDescriptor();
+			}
 
 			descriptor.setThumbnail(thumbnail);
 

engines/avalanche/detection.cpp

@@ -193,7 +193,12 @@ SaveStateDescriptor AvalancheMetaEngine::querySaveMetaInfos(const char *target,
 
 		SaveStateDescriptor desc(slot, description);
 
-		Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*f);
+		Graphics::Surface *thumbnail;
+		if (!Graphics::loadThumbnail(*f, thumbnail)) {
+			warning("Cannot read thumbnail data, possibly broken savegame");
+			delete f;
+			return SaveStateDescriptor();
+		}
 		desc.setThumbnail(thumbnail);
 
 		delete f;

engines/bbvs/bbvs.h

@@ -417,7 +417,7 @@ public:
 	const char *getSavegameFilename(int num);
 	bool existsSavegame(int num);
 	static Common::String getSavegameFilename(const Common::String &target, int num);
-	static kReadSaveHeaderError readSaveHeader(Common::SeekableReadStream *in, bool loadThumbnail, SaveHeader &header);
+	WARN_UNUSED_RESULT static kReadSaveHeaderError readSaveHeader(Common::SeekableReadStream *in, SaveHeader &header, bool skipThumbnail = true);
 
 	void allocSnapshot();
 	void freeSnapshot();

engines/bbvs/detection.cpp

@@ -124,7 +124,7 @@ SaveStateList BbvsMetaEngine::listSaves(const char *target) const {
 		if (slotNum >= 0 && slotNum <= 999) {
 			Common::InSaveFile *in = saveFileMan->openForLoading(file->c_str());
 			if (in) {
-				if (Bbvs::BbvsEngine::readSaveHeader(in, false, header) == Bbvs::BbvsEngine::kRSHENoError) {
+				if (Bbvs::BbvsEngine::readSaveHeader(in, header) == Bbvs::BbvsEngine::kRSHENoError) {
 					saveList.push_back(SaveStateDescriptor(slotNum, header.description));
 				}
 				delete in;
@@ -142,7 +142,7 @@ SaveStateDescriptor BbvsMetaEngine::querySaveMetaInfos(const char *target, int s
 	if (in) {
 		Bbvs::BbvsEngine::SaveHeader header;
 		Bbvs::BbvsEngine::kReadSaveHeaderError error;
-		error = Bbvs::BbvsEngine::readSaveHeader(in, true, header);
+		error = Bbvs::BbvsEngine::readSaveHeader(in, header, false);
 		delete in;
 		if (error == Bbvs::BbvsEngine::kRSHENoError) {
 			SaveStateDescriptor desc(slot, header.description);

engines/bbvs/saveload.cpp

@@ -27,7 +27,7 @@
 
 namespace Bbvs {
 
-BbvsEngine::kReadSaveHeaderError BbvsEngine::readSaveHeader(Common::SeekableReadStream *in, bool loadThumbnail, SaveHeader &header) {
+WARN_UNUSED_RESULT BbvsEngine::kReadSaveHeaderError BbvsEngine::readSaveHeader(Common::SeekableReadStream *in, SaveHeader &header, bool skipThumbnail) {
 
 	header.version = in->readUint32LE();
 	if (header.version > BBVS_SAVEGAME_VERSION)
@@ -38,10 +38,8 @@ BbvsEngine::kReadSaveHeaderError BbvsEngine::readSaveHeader(Common::SeekableRead
 	while (descriptionLen--)
 		header.description += (char)in->readByte();
 
-	if (loadThumbnail) {
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-		header.thumbnail = Graphics::loadThumbnail(*in);
+		return kRSHEIoError;
-	} else {
-		Graphics::skipThumbnail(*in);
 	}
 
 	// Not used yet, reserved for future usage
@@ -101,7 +99,7 @@ void BbvsEngine::loadgame(const char *filename) {
 
 	SaveHeader header;
 
-	kReadSaveHeaderError errorCode = readSaveHeader(in, false, header);
+	kReadSaveHeaderError errorCode = readSaveHeader(in, header);
 
 	if (errorCode != kRSHENoError) {
 		warning("Error loading savegame '%s'", filename);

engines/cge/cge.h

@@ -246,7 +246,7 @@ public:
 	void mainLoop();
 	void handleFrame();
 	void saveGame(int slotNumber, const Common::String &desc);
-	static bool readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header, bool skipThumbnail = true);
 	void switchMusic();
 	void selectPocket(int n);
 	void expandSprite(Sprite *spr);

engines/cge/cge_main.cpp

@@ -242,10 +242,6 @@ bool CGEEngine::loadGame(int slotNumber, SavegameHeader *header, bool tiny) {
 			delete readStream;
 			return true;
 		}
-
-		// Delete the thumbnail
-		saveHeader.thumbnail->free();
-		delete saveHeader.thumbnail;
 	}
 
 	// Get in the savegame
@@ -424,9 +420,7 @@ void CGEEngine::syncGame(Common::SeekableReadStream *readStream, Common::WriteSt
 	}
 }
 
-bool CGEEngine::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header) {
+WARN_UNUSED_RESULT bool CGEEngine::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header, bool skipThumbnail) {
-	header.thumbnail = nullptr;
-
 	// Get the savegame version
 	header.version = in->readByte();
 	if (header.version > kSavegameVersion)
@@ -439,9 +433,9 @@ bool CGEEngine::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &heade
 		header.saveName += ch;
 
 	// Get the thumbnail
-	header.thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-	if (!header.thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header.saveYear = in->readSint16LE();

engines/cge/detection.cpp

@@ -221,10 +221,6 @@ SaveStateList CGEMetaEngine::listSaves(const char *target) const {
 					// Valid savegame
 					if (CGE::CGEEngine::readSavegameHeader(file, header)) {
 						saveList.push_back(SaveStateDescriptor(slotNum, header.saveName));
-						if (header.thumbnail) {
-							header.thumbnail->free();
-							delete header.thumbnail;
-						}
 					}
 				} else {
 					// Must be an original format savegame
@@ -253,7 +249,7 @@ SaveStateDescriptor CGEMetaEngine::querySaveMetaInfos(const char *target, int sl
 		f->read(buffer, kSavegameStrSize + 1);
 
 		bool hasHeader = !strncmp(buffer, CGE::savegameStr, kSavegameStrSize + 1) &&
-			CGE::CGEEngine::readSavegameHeader(f, header);
+			CGE::CGEEngine::readSavegameHeader(f, header, false);
 		delete f;
 
 		if (!hasHeader) {

engines/cge2/cge2.h

@@ -161,7 +161,7 @@ public:
 	virtual Common::Error loadGameState(int slot);
 	virtual Common::Error run();
 
-	static bool readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header, bool skipThumbnail = true);
 
 	GUI::Debugger *getDebugger() {
 		return _console;

engines/cge2/detection.cpp

@@ -221,10 +221,6 @@ SaveStateList CGE2MetaEngine::listSaves(const char *target) const {
 					// Valid savegame
 					if (CGE2::CGE2Engine::readSavegameHeader(file, header)) {
 						saveList.push_back(SaveStateDescriptor(slotNum, header.saveName));
-						if (header.thumbnail) {
-							header.thumbnail->free();
-							delete header.thumbnail;
-						}
 					}
 				} else {
 					// Must be an original format savegame
@@ -253,7 +249,7 @@ SaveStateDescriptor CGE2MetaEngine::querySaveMetaInfos(const char *target, int s
 		f->read(buffer, kSavegameStrSize + 1);
 
 		bool hasHeader = !strncmp(buffer, kSavegameStr, kSavegameStrSize + 1) &&
-			CGE2::CGE2Engine::readSavegameHeader(f, header);
+			CGE2::CGE2Engine::readSavegameHeader(f, header, false);
 		delete f;
 
 		if (!hasHeader) {

engines/cge2/saveload.cpp

@@ -117,10 +117,6 @@ bool CGE2Engine::loadGame(int slotNumber) {
 			delete readStream;
 			return false;
 		}
-
-		// Delete the thumbnail
-		saveHeader.thumbnail->free();
-		delete saveHeader.thumbnail;
 	}
 
 	resetGame();
@@ -180,9 +176,7 @@ void CGE2Engine::writeSavegameHeader(Common::OutSaveFile *out, SavegameHeader &h
 	out->writeSint16LE(td.tm_min);
 }
 
-bool CGE2Engine::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header) {
+WARN_UNUSED_RESULT bool CGE2Engine::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header, bool skipThumbnail) {
-	header.thumbnail = nullptr;
-
 	// Get the savegame version
 	header.version = in->readByte();
 	if (header.version > kSavegameVersion)
@@ -195,9 +189,9 @@ bool CGE2Engine::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &head
 		header.saveName += ch;
 
 	// Get the thumbnail
-	header.thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-	if (!header.thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header.saveYear = in->readSint16LE();

engines/cruise/detection.cpp

@@ -241,9 +241,8 @@ SaveStateList CruiseMetaEngine::listSaves(const char *target) const {
 			Common::InSaveFile *in = saveFileMan->openForLoading(*file);
 			if (in) {
 				Cruise::CruiseSavegameHeader header;
-				Cruise::readSavegameHeader(in, header);
+				if (Cruise::readSavegameHeader(in, header))
-				saveList.push_back(SaveStateDescriptor(slotNum, header.saveName));
+					saveList.push_back(SaveStateDescriptor(slotNum, header.saveName));
-				delete header.thumbnail;
 				delete in;
 			}
 		}
@@ -264,7 +263,11 @@ SaveStateDescriptor CruiseMetaEngine::querySaveMetaInfos(const char *target, int
 
 	if (f) {
 		Cruise::CruiseSavegameHeader header;
-		Cruise::readSavegameHeader(f, header);
+		if (!Cruise::readSavegameHeader(f, header, false)) {
+			delete f;
+			return SaveStateDescriptor();
+		}
+
 		delete f;
 
 		// Create the return descriptor

engines/cruise/saveload.cpp

@@ -43,9 +43,8 @@ struct overlayRestoreTemporary {
 
 overlayRestoreTemporary ovlRestoreData[90];
 
-bool readSavegameHeader(Common::InSaveFile *in, CruiseSavegameHeader &header) {
+WARN_UNUSED_RESULT bool readSavegameHeader(Common::InSaveFile *in, CruiseSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[6];
-	header.thumbnail = NULL;
 
 	// Validate the header Id
 	in->read(saveIdentBuffer, 6);
@@ -62,9 +61,9 @@ bool readSavegameHeader(Common::InSaveFile *in, CruiseSavegameHeader &header) {
 	while ((ch = (char)in->readByte()) != '\0') header.saveName += ch;
 
 	// Get the thumbnail
-	header.thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-	if (!header.thumbnail)
 		return false;
+	}
 
 	return true;
 }
@@ -827,8 +826,10 @@ Common::Error loadSavegameData(int saveGameIdx) {
 
 	// Skip over the savegame header
 	CruiseSavegameHeader header;
-	readSavegameHeader(f, header);
+	if (!readSavegameHeader(f, header)) {
-	delete header.thumbnail;
+		delete f;
+		return Common::kReadingFailed;
+	}
 
 	// Synchronise the remaining data of the savegame
 	Common::Serializer s(f, NULL);

engines/cruise/saveload.h

@@ -38,7 +38,7 @@ struct CruiseSavegameHeader {
 
 Common::Error saveSavegameData(int saveGameIdx, const Common::String &saveName);
 Common::Error loadSavegameData(int saveGameIdx);
-bool readSavegameHeader(Common::InSaveFile *in, CruiseSavegameHeader &header);
+WARN_UNUSED_RESULT bool readSavegameHeader(Common::InSaveFile *in, CruiseSavegameHeader &header, bool skipThumbnail = true);
 void initVars();
 
 } // End of namespace Cruise

engines/dm/dm.h

@@ -332,7 +332,7 @@ public:
 	Thing _thingParty;				 // @ C0xFFFF_THING_PARTY
 };
 
-bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader *header);
+WARN_UNUSED_RESULT bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader *header, bool skipThumbnail = true);
 
 } // End of namespace DM
 

engines/dm/loadsave.cpp

@@ -70,7 +70,10 @@ LoadgameResult DMEngine::loadgame(int16 slot) {
 		file = saveFileManager->openForLoading(fileName);
 
 		SaveGameHeader header;
-		readSaveGameHeader(file, &header);
+		if (!readSaveGameHeader(file, &header)) {
+			delete file;
+			return kDMLoadgameFailure;
+		}
 
 		warning("MISSING CODE: missing check for matching format and platform in save in f435_loadgame");
 
@@ -397,7 +400,7 @@ bool DMEngine::writeCompleteSaveFile(int16 saveSlot, Common::String& saveDescrip
 	return true;
 }
 
-bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader *header) {
+WARN_UNUSED_RESULT bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader *header, bool skipThumbnail) {
 	uint32 id = in->readUint32BE();
 
 	// Check if it's a valid ScummVM savegame
@@ -419,7 +422,11 @@ bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader *header) {
 	header->_descr.setDescription(saveName);
 
 	// Get the thumbnail
-	header->_descr.setThumbnail(Graphics::loadThumbnail(*in));
+	Graphics::Surface *thumbnail;
+	if (!Graphics::loadThumbnail(*in, thumbnail, skipThumbnail)) {
+		return false;
+	}
+	header->_descr.setThumbnail(thumbnail);
 
 	uint32 saveDate = in->readUint32BE();
 	uint16 saveTime = in->readUint16BE();

engines/draci/detection.cpp

@@ -132,10 +132,6 @@ SaveStateList DraciMetaEngine::listSaves(const char *target) const {
 				Draci::DraciSavegameHeader header;
 				if (Draci::readSavegameHeader(in, header)) {
 					saveList.push_back(SaveStateDescriptor(slotNum, header.saveName));
-					if (header.thumbnail) {
-						header.thumbnail->free();
-						delete header.thumbnail;
-					}
 				}
 				delete in;
 			}
@@ -157,7 +153,11 @@ SaveStateDescriptor DraciMetaEngine::querySaveMetaInfos(const char *target, int
 
 	if (f) {
 		Draci::DraciSavegameHeader header;
-		Draci::readSavegameHeader(f, header);
+		if (!Draci::readSavegameHeader(f, header, false)) {
+			delete f;
+			return SaveStateDescriptor();
+		}
+
 		delete f;
 
 		// Create the return descriptor

engines/draci/saveload.cpp

@@ -35,9 +35,8 @@ namespace Draci {
 
 static const char *const draciIdentString = "DRACI";
 
-bool readSavegameHeader(Common::InSaveFile *in, DraciSavegameHeader &header) {
+WARN_UNUSED_RESULT bool readSavegameHeader(Common::InSaveFile *in, DraciSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[6];
-	header.thumbnail = NULL;
 
 	// Validate the header Id
 	in->read(saveIdentBuffer, 6);
@@ -59,9 +58,9 @@ bool readSavegameHeader(Common::InSaveFile *in, DraciSavegameHeader &header) {
 	header.playtime = in->readUint32LE();
 
 	// Get the thumbnail
-	header.thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-	if (!header.thumbnail)
 		return false;
+	}
 
 	return true;
 }
@@ -130,10 +129,6 @@ Common::Error loadSavegameData(int saveGameIdx, DraciEngine *vm) {
 	if (!readSavegameHeader(f, header)) {
 		return Common::kNoGameDataFoundError;
 	}
-	if (header.thumbnail) {
-		header.thumbnail->free();
-		delete header.thumbnail;
-	}
 
 	// Pre-processing
 	vm->_game->rememberRoomNumAsPrevious();

engines/draci/saveload.h

@@ -42,7 +42,7 @@ struct DraciSavegameHeader {
 
 class DraciEngine;
 
-bool readSavegameHeader(Common::InSaveFile *in, DraciSavegameHeader &header);
+WARN_UNUSED_RESULT bool readSavegameHeader(Common::InSaveFile *in, DraciSavegameHeader &header, bool skipThumbnail = true);
 void writeSavegameHeader(Common::OutSaveFile *out, const DraciSavegameHeader &header);
 Common::Error saveSavegameData(int saveGameIdx, const Common::String &saveName, DraciEngine &vm);
 Common::Error loadSavegameData(int saveGameIdx, DraciEngine *vm);

engines/drascula/detection.cpp

@@ -430,7 +430,11 @@ SaveStateDescriptor DrasculaMetaEngine::querySaveMetaInfos(const char *target, i
 			return SaveStateDescriptor();
 		}
 
-		Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*in);
+		Graphics::Surface *thumbnail;
+		if (!Graphics::loadThumbnail(*in, thumbnail)) {
+			delete in;
+			return SaveStateDescriptor();
+		}
 		desc.setThumbnail(thumbnail);
 
 		delete in;

engines/dreamweb/detection.cpp

@@ -198,7 +198,12 @@ SaveStateDescriptor DreamWebMetaEngine::querySaveMetaInfos(const char *target, i
 			uint32 saveDate = in->readUint32LE();
 			uint32 saveTime = in->readUint32LE();
 			uint32 playTime = in->readUint32LE();
-			Graphics::Surface *thumbnail = Graphics::loadThumbnail(*in);
+			Graphics::Surface *thumbnail;
+			if (!Graphics::loadThumbnail(*in, thumbnail)) {
+				warning("Missing or broken thumbnail - skipping");
+				delete in;
+				return desc;
+			}
 
 			int day = (saveDate >> 24) & 0xFF;
 			int month = (saveDate >> 16) & 0xFF;

engines/fullpipe/detection.cpp

@@ -184,7 +184,9 @@ SaveStateList FullpipeMetaEngine::listSaves(const char *target) const {
 			Common::ScopedPtr<Common::InSaveFile> in(saveFileMan->openForLoading(*file));
 			if (in) {
 				Fullpipe::FullpipeSavegameHeader header;
-				Fullpipe::readSavegameHeader(in.get(), header);
+				if (!Fullpipe::readSavegameHeader(in.get(), header)) {
+					continue;
+				}
 
 				SaveStateDescriptor desc;
 
@@ -212,7 +214,9 @@ SaveStateDescriptor FullpipeMetaEngine::querySaveMetaInfos(const char *target, i
 
 	if (f) {
 		Fullpipe::FullpipeSavegameHeader header;
-		Fullpipe::readSavegameHeader(f.get(), header);
+		if (!Fullpipe::readSavegameHeader(f.get(), header, false)) {
+			return SaveStateDescriptor();
+		}
 
 		// Create the return descriptor
 		SaveStateDescriptor desc;

engines/fullpipe/gameloader.h

@@ -84,7 +84,7 @@ struct FullpipeSavegameHeader {
 	uint32 date;
 	uint16 time;
 	uint32 playtime;
-	Common::SharedPtr<Graphics::Surface> thumbnail;
+	Graphics::Surface *thumbnail;
 };
 
 struct SaveHeader {
@@ -142,7 +142,7 @@ class GameLoader : public CObject {
 };
 
 const char *getSavegameFile(int saveGameIdx);
-bool readSavegameHeader(Common::InSaveFile *in, FullpipeSavegameHeader &header);
+WARN_UNUSED_RESULT bool readSavegameHeader(Common::InSaveFile *in, FullpipeSavegameHeader &header, bool skipThumbnail = true);
 void parseSavegameHeader(Fullpipe::FullpipeSavegameHeader &header, SaveStateDescriptor &desc);
 
 Inventory2 *getGameLoaderInventory();

engines/fullpipe/modal.cpp

@@ -2142,7 +2142,8 @@ bool ModalSaveGame::getFileInfo(int slot, FileInfo *fileinfo) {
 		return false;
 
 	Fullpipe::FullpipeSavegameHeader header;
-	Fullpipe::readSavegameHeader(f.get(), header);
+	if (!Fullpipe::readSavegameHeader(f.get(), header))
+		return false;
 
 	// Create the return descriptor
 	SaveStateDescriptor desc(slot, header.saveName);

engines/fullpipe/stateloader.cpp

@@ -193,7 +193,7 @@ void fillDummyHeader(Fullpipe::FullpipeSavegameHeader &header) {
 	header.playtime = 0;
 }
 
-bool readSavegameHeader(Common::InSaveFile *in, FullpipeSavegameHeader &header) {
+WARN_UNUSED_RESULT bool readSavegameHeader(Common::InSaveFile *in, FullpipeSavegameHeader &header, bool skipThumbnail) {
 	uint oldPos = in->pos();
 
 	in->seek(-4, SEEK_END);
@@ -237,13 +237,13 @@ bool readSavegameHeader(Common::InSaveFile *in, FullpipeSavegameHeader &header)
 		header.description = header.saveName;
 
 	// Get the thumbnail
-	header.thumbnail = Common::SharedPtr<Graphics::Surface>(Graphics::loadThumbnail(*in), Graphics::SurfaceDeleter());
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
+		in->seek(oldPos, SEEK_SET); // Rewind the file
+		return false;
+	}
 
 	in->seek(oldPos, SEEK_SET); // Rewind the file
 
-	if (!header.thumbnail)
-		return false;
-
 	return true;
 }
 

engines/gnap/detection.cpp

@@ -141,13 +141,8 @@ SaveStateList GnapMetaEngine::listSaves(const char *target) const {
 			Common::InSaveFile *in = g_system->getSavefileManager()->openForLoading(*file);
 
 			if (in) {
-				Gnap::GnapEngine::readSavegameHeader(in, header);
+				if (Gnap::GnapEngine::readSavegameHeader(in, header))
-				saveList.push_back(SaveStateDescriptor(slot, header._saveName));
+					saveList.push_back(SaveStateDescriptor(slot, header._saveName));
-
-				if (header._thumbnail) {
-					header._thumbnail->free();
-					delete header._thumbnail;
-				}
 				delete in;
 			}
 		}
@@ -179,7 +174,11 @@ SaveStateDescriptor GnapMetaEngine::querySaveMetaInfos(const char *target, int s
 		SaveStateDescriptor desc(slot, saveName);
 
 		if (version != 1) {
-			Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*file);
+			Graphics::Surface *thumbnail;
+			if (!Graphics::loadThumbnail(*file, thumbnail)) {
+				delete file;
+				return SaveStateDescriptor();
+			}
 			desc.setThumbnail(thumbnail);
 		}
 

engines/gnap/gnap.h

@@ -319,7 +319,7 @@ public:
 	Common::String generateSaveName(int slot);
 	void synchronize(Common::Serializer &s);
 	void writeSavegameHeader(Common::OutSaveFile *out, GnapSavegameHeader &header);
-	static bool readSavegameHeader(Common::InSaveFile *in, GnapSavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, GnapSavegameHeader &header, bool skipThumbnail = true);
 
 	void delayTicks(int val, int idx, bool updateCursor);
 	void delayTicksA(int val, int idx);

engines/gnap/menu.cpp

@@ -589,9 +589,8 @@ void GnapEngine::writeSavegameHeader(Common::OutSaveFile *out, GnapSavegameHeade
 	out->writeSint16LE(td.tm_min);
 }
 
-bool GnapEngine::readSavegameHeader(Common::InSaveFile *in, GnapSavegameHeader &header) {
+WARN_UNUSED_RESULT bool GnapEngine::readSavegameHeader(Common::InSaveFile *in, GnapSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[SAVEGAME_STR_SIZE + 1];
-	header._thumbnail = nullptr;
 
 	// Validate the header Id
 	in->read(saveIdentBuffer, SAVEGAME_STR_SIZE + 1);
@@ -612,9 +611,9 @@ bool GnapEngine::readSavegameHeader(Common::InSaveFile *in, GnapSavegameHeader &
 	if (header._version == 1)
 		header._thumbnail = nullptr;
 	else {
-		header._thumbnail = Graphics::loadThumbnail(*in);
+		if (!Graphics::loadThumbnail(*in, header._thumbnail, skipThumbnail)) {
-		if (!header._thumbnail)
 			return false;
+		}
 	}
 
 	// Read in save date/time

engines/hopkins/detection.cpp

@@ -168,9 +168,6 @@ SaveStateList HopkinsMetaEngine::listSaves(const char *target) const {
 			if (in) {
 				if (Hopkins::SaveLoadManager::readSavegameHeader(in, header)) {
 					saveList.push_back(SaveStateDescriptor(slot, header._saveName));
-
-					header._thumbnail->free();
-					delete header._thumbnail;
 				}
 
 				delete in;
@@ -198,7 +195,11 @@ SaveStateDescriptor HopkinsMetaEngine::querySaveMetaInfos(const char *target, in
 
 	if (f) {
 		Hopkins::hopkinsSavegameHeader header;
-		Hopkins::SaveLoadManager::readSavegameHeader(f, header);
+		if (!Hopkins::SaveLoadManager::readSavegameHeader(f, header, false)) {
+			delete f;
+			return SaveStateDescriptor();
+		}
+
 		delete f;
 
 		// Create the return descriptor

engines/hopkins/dialogs.cpp

@@ -692,7 +692,7 @@ void DialogsManager::showSaveLoad(SaveLoadMode mode) {
 
 	for (int slotNumber = 1; slotNumber <= 6; ++slotNumber) {
 		hopkinsSavegameHeader header;
-		if (_vm->_saveLoad->readSavegameHeader(slotNumber, header)) {
+		if (_vm->_saveLoad->readSavegameHeader(slotNumber, header, false)) {
 			Graphics::Surface thumb8;
 			_vm->_saveLoad->convertThumb16To8(header._thumbnail, &thumb8);
 

engines/hopkins/saveload.cpp

@@ -77,9 +77,8 @@ void SaveLoadManager::load(const Common::String &file, byte *buf) {
 	delete savefile;
 }
 
-bool SaveLoadManager::readSavegameHeader(Common::InSaveFile *in, hopkinsSavegameHeader &header) {
+WARN_UNUSED_RESULT bool SaveLoadManager::readSavegameHeader(Common::InSaveFile *in, hopkinsSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[SAVEGAME_STR_SIZE + 1];
-	header._thumbnail = NULL;
 
 	// Validate the header Id
 	in->read(saveIdentBuffer, SAVEGAME_STR_SIZE + 1);
@@ -96,9 +95,9 @@ bool SaveLoadManager::readSavegameHeader(Common::InSaveFile *in, hopkinsSavegame
 	while ((ch = (char)in->readByte()) != '\0') header._saveName += ch;
 
 	// Get the thumbnail
-	header._thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header._thumbnail, skipThumbnail)) {
-	if (!header._thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header._year = in->readSint16LE();
@@ -186,10 +185,10 @@ Common::Error SaveLoadManager::loadGame(int slot) {
 
 	// Read in the savegame header
 	hopkinsSavegameHeader header;
-	readSavegameHeader(savefile, header);
+	if (!readSavegameHeader(savefile, header)) {
-	if (header._thumbnail)
+		delete savefile;
-		header._thumbnail->free();
+		return Common::kReadingFailed;
-	delete header._thumbnail;
+	}
 
 	// Read in the savegame data
 	syncSavegameData(serializer, header._version);
@@ -212,14 +211,14 @@ Common::Error SaveLoadManager::loadGame(int slot) {
 	return Common::kNoError;
 }
 
-bool SaveLoadManager::readSavegameHeader(int slot, hopkinsSavegameHeader &header) {
+WARN_UNUSED_RESULT bool SaveLoadManager::readSavegameHeader(int slot, hopkinsSavegameHeader &header, bool skipThumbnail) {
 	// Try and open the save file for reading
 	Common::InSaveFile *savefile = g_system->getSavefileManager()->openForLoading(
 		_vm->generateSaveName(slot));
 	if (!savefile)
 		return false;
 
-	bool result = readSavegameHeader(savefile, header);
+	bool result = readSavegameHeader(savefile, header, skipThumbnail);
 	delete savefile;
 	return result;
 }

engines/hopkins/saveload.h

@@ -61,9 +61,9 @@ public:
 	bool saveFile(const Common::String &file, const void *buf, size_t n);
 	void load(const Common::String &file, byte *buf);
 
-	static bool readSavegameHeader(Common::InSaveFile *in, hopkinsSavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, hopkinsSavegameHeader &header, bool skipThumbnail = true);
 	void writeSavegameHeader(Common::OutSaveFile *out, hopkinsSavegameHeader &header);
-	bool readSavegameHeader(int slot, hopkinsSavegameHeader &header);
+	WARN_UNUSED_RESULT bool readSavegameHeader(int slot, hopkinsSavegameHeader &header, bool skipThumbnail = true);
 	Common::Error saveGame(int slot, const Common::String &saveName);
 	Common::Error loadGame(int slot);
 

engines/hugo/detection.cpp

@@ -241,7 +241,12 @@ SaveStateDescriptor HugoMetaEngine::querySaveMetaInfos(const char *target, int s
 
 		SaveStateDescriptor desc(slot, saveName);
 
-		Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*file);
+		Graphics::Surface *thumbnail;
+		if (!Graphics::loadThumbnail(*file, thumbnail)) {
+			warning("Missing or broken savegame thumbnail");
+			delete file;
+			return SaveStateDescriptor();
+		}
 		desc.setThumbnail(thumbnail);
 
 		uint32 saveDate = file->readUint32BE();

engines/kyra/detection.cpp

@@ -256,7 +256,7 @@ SaveStateList KyraMetaEngine::listSaves(const char *target) const {
 		if (slotNum >= 0 && slotNum <= 999) {
 			Common::InSaveFile *in = saveFileMan->openForLoading(*file);
 			if (in) {
-				if (Kyra::KyraEngine_v1::readSaveHeader(in, false, header) == Kyra::KyraEngine_v1::kRSHENoError) {
+				if (Kyra::KyraEngine_v1::readSaveHeader(in, header) == Kyra::KyraEngine_v1::kRSHENoError) {
 					// WORKAROUND: Old savegames are using 'German' as description for kyra3 restart game save (slot 0),
 					// since that looks odd we replace it by "New Game".
 					if (slotNum == 0 && header.gameID == Kyra::GI_KYRA3)
@@ -298,7 +298,7 @@ SaveStateDescriptor KyraMetaEngine::querySaveMetaInfos(const char *target, int s
 		Kyra::KyraEngine_v1::SaveHeader header;
 		Kyra::KyraEngine_v1::ReadSaveHeaderError error;
 
-		error = Kyra::KyraEngine_v1::readSaveHeader(in, true, header);
+		error = Kyra::KyraEngine_v1::readSaveHeader(in, header, false);
 		delete in;
 
 		if (error == Kyra::KyraEngine_v1::kRSHENoError) {

engines/kyra/kyra_v1.h

@@ -416,7 +416,7 @@ protected:
 		kRSHEIoError = 3
 	};
 
-	static ReadSaveHeaderError readSaveHeader(Common::SeekableReadStream *file, bool loadThumbnail, SaveHeader &header);
+	WARN_UNUSED_RESULT static ReadSaveHeaderError readSaveHeader(Common::SeekableReadStream *file, SaveHeader &header, bool skipThumbnail = true);
 
 	void loadGameStateCheck(int slot);
 	virtual Common::Error loadGameState(int slot) = 0;

engines/kyra/saveload.cpp

@@ -37,12 +37,11 @@
 
 namespace Kyra {
 
-KyraEngine_v1::ReadSaveHeaderError KyraEngine_v1::readSaveHeader(Common::SeekableReadStream *in, bool loadThumbnail, SaveHeader &header) {
+WARN_UNUSED_RESULT KyraEngine_v1::ReadSaveHeaderError KyraEngine_v1::readSaveHeader(Common::SeekableReadStream *in, SaveHeader &header, bool skipThumbnail) {
 	uint32 type = in->readUint32BE();
 	header.originalSave = false;
 	header.oldHeader = false;
 	header.flags = 0;
-	header.thumbnail = 0;
 
 	if (type == MKTAG('K', 'Y', 'R', 'A') || type == MKTAG('A', 'R', 'Y', 'K')) { // old Kyra1 header ID
 		header.gameID = GI_KYRA1;
@@ -125,10 +124,8 @@ KyraEngine_v1::ReadSaveHeaderError KyraEngine_v1::readSaveHeader(Common::Seekabl
 		header.flags = in->readUint32BE();
 
 	if (header.version >= 14) {
-		if (loadThumbnail) {
+		if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-			header.thumbnail = Graphics::loadThumbnail(*in);
+			return kRSHEIoError;
-		} else {
-			Graphics::skipThumbnail(*in);
 		}
 	}
 
@@ -140,7 +137,7 @@ Common::SeekableReadStream *KyraEngine_v1::openSaveForReading(const char *filena
 	if (!(in = _saveFileMan->openForLoading(filename)))
 		return 0;
 
-	ReadSaveHeaderError errorCode = KyraEngine_v1::readSaveHeader(in, false, header);
+	ReadSaveHeaderError errorCode = KyraEngine_v1::readSaveHeader(in, header);
 	if (errorCode != kRSHENoError) {
 		if (errorCode == kRSHEInvalidType)
 			warning("No ScummVM Kyra engine savefile header");

engines/lab/lab.h

@@ -502,7 +502,7 @@ private:
 	void handleTrialWarning();
 };
 
-bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader &header);
+WARN_UNUSED_RESULT bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader &header, bool skipThumbnail = true);
 
 } // End of namespace Lab
 

engines/lab/savegame.cpp

@@ -76,7 +76,7 @@ void LabEngine::writeSaveGameHeader(Common::OutSaveFile *out, const Common::Stri
 	out->writeUint32BE(playTime);
 }
 
-bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader &header) {
+WARN_UNUSED_RESULT bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader &header, bool skipThumbnail) {
 	uint32 id = in->readUint32BE();
 
 	// Check if it's a valid ScummVM savegame
@@ -98,7 +98,11 @@ bool readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader &header) {
 	header._descr.setDescription(saveName);
 
 	// Get the thumbnail
-	header._descr.setThumbnail(Graphics::loadThumbnail(*in));
+	Graphics::Surface *thumbnail;
+	if (!Graphics::loadThumbnail(*in, thumbnail, skipThumbnail)) {
+		return false;
+	}
+	header._descr.setThumbnail(thumbnail);
 
 	uint32 saveDate = in->readUint32BE();
 	uint16 saveTime = in->readUint16BE();
@@ -174,7 +178,11 @@ bool LabEngine::loadGame(int slot) {
 		return false;
 
 	SaveGameHeader header;
-	readSaveGameHeader(file, header);
+	if (!readSaveGameHeader(file, header)) {
+		delete file;
+		return false;
+	}
+
 	_roomNum = file->readUint16LE();
 	_music->checkRoomMusic(1, _roomNum);
 	_direction = file->readUint16LE();

engines/lilliput/detection.cpp

@@ -233,7 +233,11 @@ SaveStateDescriptor LilliputMetaEngine::querySaveMetaInfos(const char *target, i
 
 		SaveStateDescriptor desc(slot, saveName);
 
-		Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*file);
+		Graphics::Surface *thumbnail;
+		if (!Graphics::loadThumbnail(*file, thumbnail)) {
+			delete file;
+			return SaveStateDescriptor();
+		}
 		desc.setThumbnail(thumbnail);
 
 		desc.setDeletableFlag(true);

engines/macventure/detection.cpp

@@ -54,7 +54,7 @@ static const PlainGameDescriptor macventureGames[] = {
 
 namespace MacVenture {
 
-SaveStateDescriptor loadMetaData(Common::SeekableReadStream *s, int slot);
+SaveStateDescriptor loadMetaData(Common::SeekableReadStream *s, int slot, bool skipThumbnail = true);
 
 class MacVentureMetaEngine : public AdvancedMetaEngine {
 public:
@@ -164,7 +164,7 @@ SaveStateDescriptor MacVentureMetaEngine::querySaveMetaInfos(const char *target,
 
 	Common::InSaveFile *in = saveFileMan->openForLoading(saveFileName);
 	if (in) {
-		desc = loadMetaData(in, slot);
+		desc = loadMetaData(in, slot, false);
 		delete in;
 		return desc;
 	}

engines/macventure/saveload.cpp

@@ -42,7 +42,7 @@ namespace MacVenture {
 #define MACVENTURE_SAVE_VERSION 1 //1 BYTE
 #define MACVENTURE_DESC_LENGTH 4 //4 BYTE for the metadata length
 
-SaveStateDescriptor loadMetaData(Common::SeekableReadStream *s, int slot) {
+SaveStateDescriptor loadMetaData(Common::SeekableReadStream *s, int slot, bool skipThumbnail) {
 	// Metadata is stored at the end of the file
 	// |THUMBNAIL						|
 	// |								|
@@ -65,8 +65,11 @@ SaveStateDescriptor loadMetaData(Common::SeekableReadStream *s, int slot) {
 	s->seek(-(5 + MACVENTURE_DESC_LENGTH + metaSize), SEEK_END);
 
 	// Load the thumbnail
-	Graphics::Surface *thumb = Graphics::loadThumbnail(*s);
+	Graphics::Surface *thumbnail;
-	desc.setThumbnail(thumb);
+	if (!Graphics::loadThumbnail(*s, thumbnail, skipThumbnail)) {
+		return desc;
+	}
+	desc.setThumbnail(thumbnail);
 
 	// Load the description
 	Common::String name;

engines/mads/detection.cpp

@@ -203,11 +203,8 @@ SaveStateList MADSMetaEngine::listSaves(const char *target) const {
 			Common::InSaveFile *in = g_system->getSavefileManager()->openForLoading(*file);
 
 			if (in) {
-				MADS::Game::readSavegameHeader(in, header);
+				if (MADS::Game::readSavegameHeader(in, header))
-				saveList.push_back(SaveStateDescriptor(slot, header._saveName));
+					saveList.push_back(SaveStateDescriptor(slot, header._saveName));
-
-				header._thumbnail->free();
-				delete header._thumbnail;
 				delete in;
 			}
 		}
@@ -233,7 +230,10 @@ SaveStateDescriptor MADSMetaEngine::querySaveMetaInfos(const char *target, int s
 
 	if (f) {
 		MADS::MADSSavegameHeader header;
-		MADS::Game::readSavegameHeader(f, header);
+		if (!MADS::Game::readSavegameHeader(f, header, false)) {
+			delete f;
+			return SaveStateDescriptor();
+		}
 		delete f;
 
 		// Create the return descriptor

engines/mads/game.cpp

@@ -485,11 +485,6 @@ void Game::loadGame(int slotNumber) {
 	if (!readSavegameHeader(_saveFile, header))
 		error("Invalid savegame");
 
-	if (header._thumbnail) {
-		header._thumbnail->free();
-		delete header._thumbnail;
-	}
-
 	// Load most of the savegame data with the exception of scene specific info
 	synchronize(s, true);
 
@@ -527,9 +522,8 @@ void Game::saveGame(int slotNumber, const Common::String &saveName) {
 const char *const SAVEGAME_STR = "MADS";
 #define SAVEGAME_STR_SIZE 4
 
-bool Game::readSavegameHeader(Common::InSaveFile *in, MADSSavegameHeader &header) {
+WARN_UNUSED_RESULT bool Game::readSavegameHeader(Common::InSaveFile *in, MADSSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[SAVEGAME_STR_SIZE + 1];
-	header._thumbnail = nullptr;
 
 	// Validate the header Id
 	in->read(saveIdentBuffer, SAVEGAME_STR_SIZE + 1);
@@ -546,9 +540,9 @@ bool Game::readSavegameHeader(Common::InSaveFile *in, MADSSavegameHeader &header
 	while ((ch = (char)in->readByte()) != '\0') header._saveName += ch;
 
 	// Get the thumbnail
-	header._thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header._thumbnail, skipThumbnail)) {
-	if (!header._thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header._year = in->readSint16LE();

engines/mads/game.h

@@ -237,7 +237,7 @@ public:
 	/**
 	 * Read in a savegame header
 	 */
-	static bool readSavegameHeader(Common::InSaveFile *in, MADSSavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, MADSSavegameHeader &header, bool skipThumbnail = true);
 
 	/**
 	 * Creates a temporary thumbnail for use in saving games

engines/mohawk/myst_state.cpp

@@ -272,7 +272,12 @@ SaveStateDescriptor MystGameState::querySaveMetaInfos(int slot) {
 	desc.setSaveDate(metadata.saveYear, metadata.saveMonth, metadata.saveDay);
 	desc.setSaveTime(metadata.saveHour, metadata.saveMinute);
 	desc.setPlayTime(metadata.totalPlayTime);
-	desc.setThumbnail(Graphics::loadThumbnail(*metadataFile));
+	Graphics::Surface *thumbnail;
+	if (!Graphics::loadThumbnail(*metadataFile, thumbnail)) {
+		delete metadataFile;
+		return SaveStateDescriptor();
+	}
+	desc.setThumbnail(thumbnail);
 
 	delete metadataFile;
 

engines/mohawk/riven_saveload.cpp

@@ -148,7 +148,11 @@ SaveStateDescriptor RivenSaveLoad::querySaveMetaInfos(const int slot) {
 		return descriptor;
 	}
 
-	descriptor.setThumbnail(Graphics::loadThumbnail(*thmbStream));
+	Graphics::Surface *thumbnail;
+	if (!Graphics::loadThumbnail(*thmbStream, thumbnail)) {
+		return descriptor;
+	}
+	descriptor.setThumbnail(thumbnail);
 
 	delete thmbStream;
 

engines/mortevielle/saveload.cpp

@@ -92,8 +92,10 @@ bool SavegameManager::loadSavegame(const Common::String &filename) {
 	if (!strncmp(&buffer[0], &SAVEGAME_ID[0], 4)) {
 		// Yes, it is, so skip over the savegame header
 		SavegameHeader header;
-		readSavegameHeader(stream, header);
+		if (!readSavegameHeader(stream, header)) {
-		delete header.thumbnail;
+			delete stream;
+			return false;
+		}
 	} else {
 		stream->seek(0);
 	}
@@ -208,9 +210,7 @@ void SavegameManager::writeSavegameHeader(Common::OutSaveFile *out, const Common
 	out->writeSint16LE(td.tm_min);
 }
 
-bool SavegameManager::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header) {
+WARN_UNUSED_RESULT bool SavegameManager::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header, bool skipThumbnail) {
-	header.thumbnail = NULL;
-
 	// Get the savegame version
 	header.version = in->readByte();
 
@@ -221,9 +221,9 @@ bool SavegameManager::readSavegameHeader(Common::InSaveFile *in, SavegameHeader
 		header.saveName += ch;
 
 	// Get the thumbnail
-	header.thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-	if (!header.thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header.saveYear = in->readSint16LE();
@@ -263,7 +263,6 @@ SaveStateList SavegameManager::listSaves(const Common::String &target) {
 				validFlag = readSavegameHeader(in, header);
 
 				if (validFlag) {
-					delete header.thumbnail;
 					saveDescription = header.saveName;
 				}
 			} else if (file->size() == 497) {
@@ -311,7 +310,10 @@ SaveStateDescriptor SavegameManager::querySaveMetaInfos(const Common::String &fi
 		} else {
 			// Get the savegame header information
 			SavegameHeader header;
-			readSavegameHeader(f, header);
+			if (!readSavegameHeader(f, header, false)) {
+				delete f;
+				return SaveStateDescriptor();
+			}
 			delete f;
 
 			// Create the return descriptor

engines/mortevielle/saveload.h

@@ -65,7 +65,7 @@ public:
 	Common::Error saveGame(int slot);
 
 	void writeSavegameHeader(Common::OutSaveFile *out, const Common::String &saveName);
-	static bool readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header, bool skipThumbnail = true);
 	static SaveStateList listSaves(const Common::String &target);
 	static SaveStateDescriptor querySaveMetaInfos(const Common::String &fileName);
 };

engines/neverhood/detection.cpp

@@ -271,7 +271,7 @@ SaveStateList NeverhoodMetaEngine::listSaves(const char *target) const {
 		if (slotNum >= 0 && slotNum <= 999) {
 			Common::InSaveFile *in = saveFileMan->openForLoading(file->c_str());
 			if (in) {
-				if (Neverhood::NeverhoodEngine::readSaveHeader(in, false, header) == Neverhood::NeverhoodEngine::kRSHENoError) {
+				if (Neverhood::NeverhoodEngine::readSaveHeader(in, header) == Neverhood::NeverhoodEngine::kRSHENoError) {
 					saveList.push_back(SaveStateDescriptor(slotNum, header.description));
 				}
 				delete in;
@@ -302,7 +302,7 @@ SaveStateDescriptor NeverhoodMetaEngine::querySaveMetaInfos(const char *target,
 		Neverhood::NeverhoodEngine::SaveHeader header;
 		Neverhood::NeverhoodEngine::kReadSaveHeaderError error;
 
-		error = Neverhood::NeverhoodEngine::readSaveHeader(in, true, header);
+		error = Neverhood::NeverhoodEngine::readSaveHeader(in, header, false);
 		delete in;
 
 		if (error == Neverhood::NeverhoodEngine::kRSHENoError) {

engines/neverhood/menumodule.cpp

@@ -291,7 +291,7 @@ void MenuModule::loadSavegameList() {
 		if (slotNum >= 0 && slotNum <= 999) {
 			Common::InSaveFile *in = saveFileMan->openForLoading(file->c_str());
 			if (in) {
-				if (Neverhood::NeverhoodEngine::readSaveHeader(in, false, header) == Neverhood::NeverhoodEngine::kRSHENoError) {
+				if (Neverhood::NeverhoodEngine::readSaveHeader(in, header) == Neverhood::NeverhoodEngine::kRSHENoError) {
 					SavegameItem savegameItem;
 					savegameItem.slotNum = slotNum;
 					savegameItem.description = header.description;

engines/neverhood/neverhood.h

@@ -129,7 +129,7 @@ public:
 	bool loadgame(const char *filename);
 	const char *getSavegameFilename(int num);
 	static Common::String getSavegameFilename(const Common::String &target, int num);
-	static kReadSaveHeaderError readSaveHeader(Common::SeekableReadStream *in, bool loadThumbnail, SaveHeader &header);
+	WARN_UNUSED_RESULT static kReadSaveHeaderError readSaveHeader(Common::SeekableReadStream *in, SaveHeader &header, bool skipThumbnail = true);
 
 	GameState& gameState() { return _gameState; }
 	GameModule *gameModule() { return _gameModule; }

engines/neverhood/saveload.cpp

@@ -32,7 +32,7 @@ namespace Neverhood {
 
 #define NEVERHOOD_SAVEGAME_VERSION 0
 
-NeverhoodEngine::kReadSaveHeaderError NeverhoodEngine::readSaveHeader(Common::SeekableReadStream *in, bool loadThumbnail, SaveHeader &header) {
+WARN_UNUSED_RESULT NeverhoodEngine::kReadSaveHeaderError NeverhoodEngine::readSaveHeader(Common::SeekableReadStream *in, SaveHeader &header, bool skipThumbnail) {
 
 	header.version = in->readUint32LE();
 	if (header.version > NEVERHOOD_SAVEGAME_VERSION)
@@ -43,10 +43,8 @@ NeverhoodEngine::kReadSaveHeaderError NeverhoodEngine::readSaveHeader(Common::Se
 	while (descriptionLen--)
 		header.description += (char)in->readByte();
 
-	if (loadThumbnail) {
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-		header.thumbnail = Graphics::loadThumbnail(*in);
+		return kRSHEIoError;
-	} else {
-		Graphics::skipThumbnail(*in);
 	}
 
 	// Not used yet, reserved for future usage
@@ -110,7 +108,7 @@ bool NeverhoodEngine::loadgame(const char *filename) {
 
 	SaveHeader header;
 
-	kReadSaveHeaderError errorCode = readSaveHeader(in, false, header);
+	kReadSaveHeaderError errorCode = readSaveHeader(in, header);
 
 	if (errorCode != kRSHENoError) {
 		warning("Error loading savegame '%s'", filename);

engines/prince/detection.cpp

@@ -208,10 +208,6 @@ SaveStateList PrinceMetaEngine::listSaves(const char *target) const {
 					// Valid savegame
 					if (Prince::PrinceEngine::readSavegameHeader(file, header)) {
 						saveList.push_back(SaveStateDescriptor(slotNum, header.saveName));
-						if (header.thumbnail) {
-							header.thumbnail->free();
-							delete header.thumbnail;
-						}
 					}
 				} else {
 					// Must be an original format savegame
@@ -239,7 +235,7 @@ SaveStateDescriptor PrinceMetaEngine::querySaveMetaInfos(const char *target, int
 		f->read(buffer, kSavegameStrSize + 1);
 
 		bool hasHeader = !strncmp(buffer, kSavegameStr, kSavegameStrSize + 1) &&
-			Prince::PrinceEngine::readSavegameHeader(f, header);
+			Prince::PrinceEngine::readSavegameHeader(f, header, false);
 		delete f;
 
 		if (!hasHeader) {

engines/prince/prince.h

@@ -290,7 +290,7 @@ public:
 
 	void playVideo(Common::String videoFilename);
 
-	static bool readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header, bool skipThumbnail = true);
 	Common::String generateSaveName(int slot);
 	void writeSavegameHeader(Common::OutSaveFile *out, SavegameHeader &header);
 	void syncGame(Common::SeekableReadStream *readStream, Common::WriteStream *writeStream);

engines/prince/saveload.cpp

@@ -44,9 +44,7 @@ namespace Prince {
 class InterpreterFlags;
 class Interpreter;
 
-bool PrinceEngine::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header) {
+WARN_UNUSED_RESULT bool PrinceEngine::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &header, bool skipThumbnail) {
-	header.thumbnail = nullptr;
-
 	// Get the savegame version
 	header.version = in->readByte();
 	if (header.version > kSavegameVersion)
@@ -59,9 +57,9 @@ bool PrinceEngine::readSavegameHeader(Common::InSaveFile *in, SavegameHeader &he
 		header.saveName += ch;
 
 	// Get the thumbnail
-	header.thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-	if (!header.thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header.saveYear = in->readSint16LE();
@@ -416,10 +414,6 @@ bool PrinceEngine::loadGame(int slotNumber) {
 			delete readStream;
 			return false;
 		}
-
-		// Delete the thumbnail
-		saveHeader.thumbnail->free();
-		delete saveHeader.thumbnail;
 	}
 
 	// Get in the savegame

engines/saga/detection.cpp

@@ -256,7 +256,11 @@ SaveStateDescriptor SagaMetaEngine::querySaveMetaInfos(const char *target, int s
 		}
 
 		if (version >= 6) {
-			Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*in);
+			Graphics::Surface *thumbnail;
+			if (!Graphics::loadThumbnail(*in, thumbnail)) {
+				delete in;
+				return SaveStateDescriptor();
+			}
 			desc.setThumbnail(thumbnail);
 
 			uint32 saveDate = in->readUint32BE();

engines/sci/detection.cpp

@@ -871,7 +871,14 @@ SaveStateDescriptor SciMetaEngine::querySaveMetaInfos(const char *target, int sl
 
 		descriptor.setDescription(meta.name);
 
-		Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*in);
+		Graphics::Surface *thumbnail;
+		if (!Graphics::loadThumbnail(*in, thumbnail)) {
+			// invalid
+			delete in;
+
+			descriptor.setDescription("*Invalid*");
+			return descriptor;
+		}
 		descriptor.setThumbnail(thumbnail);
 
 		int day = (meta.saveDate >> 24) & 0xFF;

engines/scumm/saveload.cpp

@@ -706,7 +706,9 @@ bool ScummEngine::querySaveMetaInfos(const char *target, int slot, int heversion
 
 	if (hdr.ver > VER(52)) {
 		if (Graphics::checkThumbnailHeader(*in)) {
-			thumbnail = Graphics::loadThumbnail(*in);
+			if (!Graphics::loadThumbnail(*in, thumbnail)) {
+				return false;
+			}
 		}
 
 		if (hdr.ver > VER(57)) {

engines/sherlock/detection.cpp

@@ -233,7 +233,10 @@ SaveStateDescriptor SherlockMetaEngine::querySaveMetaInfos(const char *target, i
 
 	if (f) {
 		Sherlock::SherlockSavegameHeader header;
-		Sherlock::SaveManager::readSavegameHeader(f, header);
+		if (!Sherlock::SaveManager::readSavegameHeader(f, header, false)) {
+			delete f;
+			return SaveStateDescriptor();
+		}
 		delete f;
 
 		// Create the return descriptor

engines/sherlock/saveload.cpp

@@ -103,13 +103,9 @@ SaveStateList SaveManager::getSavegameList(const Common::String &target) {
 			Common::InSaveFile *in = g_system->getSavefileManager()->openForLoading(*file);
 
 			if (in) {
-				if (!readSavegameHeader(in, header))
+				if (readSavegameHeader(in, header))
-					continue;
+					saveList.push_back(SaveStateDescriptor(slot, header._saveName));
 
-				saveList.push_back(SaveStateDescriptor(slot, header._saveName));
-
-				header._thumbnail->free();
-				delete header._thumbnail;
 				delete in;
 			}
 		}
@@ -119,9 +115,8 @@ SaveStateList SaveManager::getSavegameList(const Common::String &target) {
 	return saveList;
 }
 
-bool SaveManager::readSavegameHeader(Common::InSaveFile *in, SherlockSavegameHeader &header) {
+WARN_UNUSED_RESULT bool SaveManager::readSavegameHeader(Common::InSaveFile *in, SherlockSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[SAVEGAME_STR_SIZE + 1];
-	header._thumbnail = nullptr;
 
 	// Validate the header Id
 	in->read(saveIdentBuffer, SAVEGAME_STR_SIZE + 1);
@@ -138,9 +133,9 @@ bool SaveManager::readSavegameHeader(Common::InSaveFile *in, SherlockSavegameHea
 	while ((ch = (char)in->readByte()) != '\0') header._saveName += ch;
 
 	// Get the thumbnail
-	header._thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header._thumbnail, skipThumbnail)) {
-	if (!header._thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header._year = in->readSint16LE();
@@ -212,11 +207,6 @@ void SaveManager::loadGame(int slot) {
 	if (!readSavegameHeader(saveFile, header))
 		error("Invalid savegame");
 
-	if (header._thumbnail) {
-		header._thumbnail->free();
-		delete header._thumbnail;
-	}
-
 	// Synchronize the savegame data
 	Serializer s(saveFile, nullptr);
 	s.setVersion(header._version);

engines/sherlock/saveload.h

@@ -105,7 +105,7 @@ public:
 	/**
 	 * Read in the header information for a savegame
 	 */
-	static bool readSavegameHeader(Common::InSaveFile *in, SherlockSavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, SherlockSavegameHeader &header, bool skipThumbnail = true);
 
 	/**
 	 * Return the index of the button the mouse is over, if any

engines/supernova/detection.cpp

@@ -200,7 +200,11 @@ SaveStateDescriptor SupernovaMetaEngine::querySaveMetaInfos(const char *target,
 		desc.setPlayTime(playTime * 1000);
 
 		if (Graphics::checkThumbnailHeader(*savefile)) {
-			Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*savefile);
+			Graphics::Surface *thumbnail;
+			if (!Graphics::loadThumbnail(*savefile, thumbnail)) {
+				delete savefile;
+				return SaveStateDescriptor();
+			}
 			desc.setThumbnail(thumbnail);
 		}
 

engines/sword1/detection.cpp

@@ -300,7 +300,11 @@ SaveStateDescriptor SwordMetaEngine::querySaveMetaInfos(const char *target, int
 			in->skip(1);
 
 		if (Graphics::checkThumbnailHeader(*in)) {
-			Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*in);
+			Graphics::Surface *thumbnail;
+			if (!Graphics::loadThumbnail(*in, thumbnail)) {
+				delete in;
+				return SaveStateDescriptor();
+			}
 			desc.setThumbnail(thumbnail);
 		}
 

engines/teenagent/detection.cpp

@@ -178,8 +178,11 @@ public:
 		SaveStateDescriptor ssd(slot, desc);
 
 		//checking for the thumbnail
-		if (Graphics::Surface *const thumb = Graphics::loadThumbnail(*in))
+		Graphics::Surface *thumbnail;
-			ssd.setThumbnail(thumb);
+		if (!Graphics::loadThumbnail(*in, thumbnail)) {
+			return SaveStateDescriptor();
+		}
+		ssd.setThumbnail(thumbnail);
 
 		return ssd;
 	}

engines/titanic/core/project_item.cpp

@@ -190,10 +190,6 @@ void CProjectItem::loadGame(int slotId) {
 	// Load the savegame header in
 	TitanicSavegameHeader header;
 	readSavegameHeader(&file, header);
-	if (header._thumbnail) {
-		header._thumbnail->free();
-		delete header._thumbnail;
-	}
 
 	g_vm->_events->setTotalPlayTicks(header._totalFrames);
 
@@ -488,13 +484,9 @@ SaveStateList CProjectItem::getSavegameList(const Common::String &target) {
 			if (in) {
 				SimpleFile f;
 				f.open(in);
-				if (!readSavegameHeader(&f, header))
+				if (readSavegameHeader(&f, header))
-					continue;
+					saveList.push_back(SaveStateDescriptor(slot, header._saveName));
 
-				saveList.push_back(SaveStateDescriptor(slot, header._saveName));
-
-				header._thumbnail->free();
-				delete header._thumbnail;
 				delete in;
 			}
 		}
@@ -503,7 +495,7 @@ SaveStateList CProjectItem::getSavegameList(const Common::String &target) {
 	return saveList;
 }
 
-bool CProjectItem::readSavegameHeader(SimpleFile *file, TitanicSavegameHeader &header) {
+bool CProjectItem::readSavegameHeader(SimpleFile *file, TitanicSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[SAVEGAME_STR_SIZE + 1];
 	header._thumbnail = nullptr;
 	header._totalFrames = 0;
@@ -526,8 +518,7 @@ bool CProjectItem::readSavegameHeader(SimpleFile *file, TitanicSavegameHeader &h
 	while ((ch = (char)file->readByte()) != '\0') header._saveName += ch;
 
 	// Get the thumbnail
-	header._thumbnail = Graphics::loadThumbnail(*file);
+	if (!Graphics::loadThumbnail(*file, header._thumbnail, skipThumbnail))
-	if (!header._thumbnail)
 		return false;
 
 	// Read in save date/time

engines/titanic/core/project_item.h

@@ -155,7 +155,7 @@ public:
 	/**
 	 * Read in the header information for a savegame
 	 */
-	static bool readSavegameHeader(SimpleFile *file, TitanicSavegameHeader &header);
+	static bool readSavegameHeader(SimpleFile *file, TitanicSavegameHeader &header, bool loadThumbnail = false);
 public:
 	CLASSDEF;
 	CProjectItem();

engines/titanic/detection.cpp

@@ -128,11 +128,6 @@ SaveStateList TitanicMetaEngine::listSaves(const char *target) const {
 				if (Titanic::CProjectItem::readSavegameHeader(&cf, header))
 					saveList.push_back(SaveStateDescriptor(slot, header._saveName));
 
-				if (header._thumbnail) {
-					header._thumbnail->free();
-					delete header._thumbnail;
-				}
-
 				cf.close();
 			}
 		}
@@ -161,7 +156,10 @@ SaveStateDescriptor TitanicMetaEngine::querySaveMetaInfos(const char *target, in
 		file.open(f);
 
 		Titanic::TitanicSavegameHeader header;
-		Titanic::CProjectItem::readSavegameHeader(&file, header);
+		if (!Titanic::CProjectItem::readSavegameHeader(&file, header, false)) {
+			file.close();
+			return SaveStateDescriptor();
+		}
 
 		file.close();
 

engines/titanic/pet_control/pet_load_save.cpp

@@ -135,11 +135,6 @@ void CPetLoadSave::resetSlots() {
 				_slotNames[idx].setText(header._saveName);
 			}
 
-			if (header._thumbnail) {
-				header._thumbnail->free();
-				delete header._thumbnail;
-			}
-
 			file.close();
 		}
 	}

engines/titanic/titanic.cpp

@@ -49,7 +49,6 @@
 #include "common/translation.h"
 #include "engines/util.h"
 #include "graphics/scaler.h"
-#include "graphics/thumbnail.h"
 #include "graphics/screen.h"
 #include "gui/saveload.h"
 
@@ -256,11 +255,6 @@ CString TitanicEngine::getSavegameName(int slot) {
 
 		TitanicSavegameHeader header;
 		bool isValid = CProjectItem::readSavegameHeader(&file, header);
-		if (header._thumbnail) {
-			header._thumbnail->free();
-			delete header._thumbnail;
-		}
-
 		file.close();
 
 		if (isValid)

engines/toltecs/detection.cpp

@@ -276,7 +276,7 @@ SaveStateList ToltecsMetaEngine::listSaves(const char *target) const {
 		if (slotNum >= 0 && slotNum <= 999) {
 			Common::InSaveFile *in = saveFileMan->openForLoading(file->c_str());
 			if (in) {
-				if (Toltecs::ToltecsEngine::readSaveHeader(in, false, header) == Toltecs::ToltecsEngine::kRSHENoError) {
+				if (Toltecs::ToltecsEngine::readSaveHeader(in, header) == Toltecs::ToltecsEngine::kRSHENoError) {
 					saveList.push_back(SaveStateDescriptor(slotNum, header.description));
 				}
 				delete in;
@@ -325,7 +325,7 @@ SaveStateDescriptor ToltecsMetaEngine::querySaveMetaInfos(const char *target, in
 		Toltecs::ToltecsEngine::SaveHeader header;
 		Toltecs::ToltecsEngine::kReadSaveHeaderError error;
 
-		error = Toltecs::ToltecsEngine::readSaveHeader(in, true, header);
+		error = Toltecs::ToltecsEngine::readSaveHeader(in, header, false);
 		delete in;
 
 		if (error == Toltecs::ToltecsEngine::kRSHENoError) {

engines/toltecs/menu.cpp

@@ -525,7 +525,7 @@ int MenuSystem::loadSavegamesList() {
 		if (slotNum >= 0 && slotNum <= 999) {
 			Common::InSaveFile *in = saveFileMan->openForLoading(file->c_str());
 			if (in) {
-				if (Toltecs::ToltecsEngine::readSaveHeader(in, false, header) == Toltecs::ToltecsEngine::kRSHENoError) {
+				if (Toltecs::ToltecsEngine::readSaveHeader(in, header) == Toltecs::ToltecsEngine::kRSHENoError) {
 					_savegames.push_back(SavegameItem(slotNum, header.description));
 					//debug("%s -> %s", file->c_str(), header.description.c_str());
 				}

engines/toltecs/saveload.cpp

@@ -41,7 +41,7 @@ namespace Toltecs {
 
 #define TOLTECS_SAVEGAME_VERSION 4
 
-ToltecsEngine::kReadSaveHeaderError ToltecsEngine::readSaveHeader(Common::SeekableReadStream *in, bool loadThumbnail, SaveHeader &header) {
+WARN_UNUSED_RESULT ToltecsEngine::kReadSaveHeaderError ToltecsEngine::readSaveHeader(Common::SeekableReadStream *in, SaveHeader &header, bool skipThumbnail) {
 
 	header.version = in->readUint32LE();
 	if (header.version > TOLTECS_SAVEGAME_VERSION)
@@ -52,10 +52,8 @@ ToltecsEngine::kReadSaveHeaderError ToltecsEngine::readSaveHeader(Common::Seekab
 	while (descriptionLen--)
 		header.description += (char)in->readByte();
 
-	if (loadThumbnail) {
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-		header.thumbnail = Graphics::loadThumbnail(*in);
+		return kRSHEIoError;
-	} else {
-		Graphics::skipThumbnail(*in);
 	}
 
 	// Not used yet, reserved for future usage
@@ -147,7 +145,7 @@ void ToltecsEngine::loadgame(const char *filename) {
 
 	SaveHeader header;
 
-	kReadSaveHeaderError errorCode = readSaveHeader(in, false, header);
+	kReadSaveHeaderError errorCode = readSaveHeader(in, header);
 
 	if (errorCode != kRSHENoError) {
 		warning("Error loading savegame '%s'", filename);

engines/toltecs/toltecs.h

@@ -225,7 +225,7 @@ public:
 	const char *getSavegameFilename(int num);
 	static Common::String getSavegameFilename(const Common::String &target, int num);
 
-	static kReadSaveHeaderError readSaveHeader(Common::SeekableReadStream *in, bool loadThumbnail, SaveHeader &header);
+	WARN_UNUSED_RESULT static kReadSaveHeaderError readSaveHeader(Common::SeekableReadStream *in, SaveHeader &header, bool skipThumbnail = true);
 
 };
 

engines/toon/detection.cpp

@@ -232,7 +232,11 @@ SaveStateDescriptor ToonMetaEngine::querySaveMetaInfos(const char *target, int s
 
 		SaveStateDescriptor desc(slot, saveName);
 
-		Graphics::Surface *const thumbnail = Graphics::loadThumbnail(*file);
+		Graphics::Surface *thumbnail;
+		if (!Graphics::loadThumbnail(*file, thumbnail)) {
+			delete file;
+			return SaveStateDescriptor();
+		}
 		desc.setThumbnail(thumbnail);
 
 		uint32 saveDate = file->readUint32BE();

engines/tsage/detection.cpp

@@ -131,9 +131,6 @@ public:
 				if (in) {
 					if (TsAGE::Saver::readSavegameHeader(in, header)) {
 						saveList.push_back(SaveStateDescriptor(slot, header._saveName));
-
-						header._thumbnail->free();
-						delete header._thumbnail;
 					}
 
 					delete in;
@@ -161,7 +158,11 @@ public:
 
 		if (f) {
 			TsAGE::tSageSavegameHeader header;
-			TsAGE::Saver::readSavegameHeader(f, header);
+			if (!TsAGE::Saver::readSavegameHeader(f, header, false)) {
+				delete f;
+				return SaveStateDescriptor();
+			}
+
 			delete f;
 
 			// Create the return descriptor

engines/tsage/saveload.cpp

@@ -189,10 +189,10 @@ Common::Error Saver::restore(int slot) {
 
 	// Read in the savegame header
 	tSageSavegameHeader header;
-	readSavegameHeader(saveFile, header);
+	if (!readSavegameHeader(saveFile, header)) {
-	if (header._thumbnail)
+		delete saveFile;
-		header._thumbnail->free();
+		return Common::kReadingFailed;
-	delete header._thumbnail;
+	}
 
 	serializer.setSaveVersion(header._version);
 
@@ -247,9 +247,8 @@ Common::Error Saver::restore(int slot) {
 const char *SAVEGAME_STR = "SCUMMVM_TSAGE";
 #define SAVEGAME_STR_SIZE 13
 
-bool Saver::readSavegameHeader(Common::InSaveFile *in, tSageSavegameHeader &header) {
+WARN_UNUSED_RESULT bool Saver::readSavegameHeader(Common::InSaveFile *in, tSageSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[SAVEGAME_STR_SIZE + 1];
-	header._thumbnail = NULL;
 
 	// Validate the header Id
 	in->read(saveIdentBuffer, SAVEGAME_STR_SIZE + 1);
@@ -266,9 +265,9 @@ bool Saver::readSavegameHeader(Common::InSaveFile *in, tSageSavegameHeader &head
 	while ((ch = (char)in->readByte()) != '\0') header._saveName += ch;
 
 	// Get the thumbnail
-	header._thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header._thumbnail, skipThumbnail)) {
-	if (!header._thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header._saveYear = in->readSint16LE();

engines/tsage/saveload.h

@@ -221,7 +221,7 @@ public:
 
 	Common::Error save(int slot, const Common::String &saveName);
 	Common::Error restore(int slot);
-	static bool readSavegameHeader(Common::InSaveFile *in, tSageSavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, tSageSavegameHeader &header, bool skipThumbnail = true);
 	static void writeSavegameHeader(Common::OutSaveFile *out, tSageSavegameHeader &header);
 
 	void addListener(SaveListener *obj);

engines/tucker/detection.cpp

@@ -175,7 +175,7 @@ public:
 			if (ext && (slot = atoi(ext + 1)) >= 0 && slot <= Tucker::kLastSaveSlot) {
 				Common::InSaveFile *in = g_system->getSavefileManager()->openForLoading(*file);
 				if (in) {
-					if (Tucker::TuckerEngine::readSavegameHeader(in, header, false) == Tucker::TuckerEngine::kSavegameNoError) {
+					if (Tucker::TuckerEngine::readSavegameHeader(in, header) == Tucker::TuckerEngine::kSavegameNoError) {
 						saveList.push_back(SaveStateDescriptor(slot, header.description));
 					}
 
@@ -207,7 +207,7 @@ public:
 		}
 
 		Tucker::TuckerEngine::SavegameHeader header;
-		Tucker::TuckerEngine::SavegameError savegameError = Tucker::TuckerEngine::readSavegameHeader(file, header, true);
+		Tucker::TuckerEngine::SavegameError savegameError = Tucker::TuckerEngine::readSavegameHeader(file, header, false);
 		if (savegameError) {
 			delete file;
 			return SaveStateDescriptor();

engines/tucker/saveload.cpp

@@ -141,7 +141,7 @@ Common::Error TuckerEngine::loadGameState(int slot) {
 }
 
 
-TuckerEngine::SavegameError TuckerEngine::readSavegameHeader(const char *target, int slot, SavegameHeader &header) {
+WARN_UNUSED_RESULT TuckerEngine::SavegameError TuckerEngine::readSavegameHeader(const char *target, int slot, SavegameHeader &header) {
 	Common::String fileName = generateGameStateFileName(target, slot);
 	Common::InSaveFile *file = g_system->getSavefileManager()->openForLoading(fileName);
 
@@ -155,7 +155,7 @@ TuckerEngine::SavegameError TuckerEngine::readSavegameHeader(const char *target,
 	return savegameError;
 }
 
-TuckerEngine::SavegameError TuckerEngine::readSavegameHeader(Common::InSaveFile *file, SavegameHeader &header, bool loadThumbnail) {
+WARN_UNUSED_RESULT TuckerEngine::SavegameError TuckerEngine::readSavegameHeader(Common::InSaveFile *file, SavegameHeader &header, bool skipThumbnail) {
 	header.version   = -1;
 	header.flags     = 0;
 	header.description.clear();
@@ -196,10 +196,8 @@ TuckerEngine::SavegameError TuckerEngine::readSavegameHeader(Common::InSaveFile
 		header.saveTime = file->readUint32LE();
 		header.playTime = file->readUint32LE();
 
-		if (loadThumbnail) {
+		if (!Graphics::loadThumbnail(*file, header.thumbnail, skipThumbnail)) {
-			header.thumbnail = Graphics::loadThumbnail(*file);
+			return kSavegameIoError;
-		} else {
-			Graphics::skipThumbnail(*file);
 		}
 	}
 

engines/tucker/tucker.h

@@ -452,8 +452,8 @@ public:
 	virtual bool hasFeature(EngineFeature f) const;
 	GUI::Debugger *getDebugger() { return _console; }
 
-	static SavegameError readSavegameHeader(Common::InSaveFile *file, SavegameHeader &header, bool loadThumbnail = false);
+	WARN_UNUSED_RESULT static SavegameError readSavegameHeader(Common::InSaveFile *file, SavegameHeader &header, bool skipThumbnail = true);
-	static SavegameError readSavegameHeader(const char *target, int slot, SavegameHeader &header);
+	WARN_UNUSED_RESULT static SavegameError readSavegameHeader(const char *target, int slot, SavegameHeader &header);
 	bool isAutosaveAllowed();
 	static bool isAutosaveAllowed(const char *target);
 protected:

engines/voyeur/detection.cpp

@@ -132,7 +132,6 @@ SaveStateList VoyeurMetaEngine::listSaves(const char *target) const {
 			if (in) {
 				if (header.read(in)) {
 					saveList.push_back(SaveStateDescriptor(slot, header._saveName));
-					header._thumbnail->free();
 				}
 				delete in;
 			}
@@ -159,7 +158,7 @@ SaveStateDescriptor VoyeurMetaEngine::querySaveMetaInfos(const char *target, int
 
 	if (f) {
 		Voyeur::VoyeurSavegameHeader header;
-		header.read(f);
+		header.read(f, false);
 		delete f;
 
 		// Create the return descriptor

engines/voyeur/voyeur.cpp

@@ -789,9 +789,6 @@ void VoyeurEngine::loadGame(int slot) {
 	VoyeurSavegameHeader header;
 	if (!header.read(saveFile))
 		return;
-	if (header._thumbnail)
-		header._thumbnail->free();
-	delete header._thumbnail;
 
 	serializer.setVersion(header._version);
 	synchronize(serializer);
@@ -856,9 +853,7 @@ void VoyeurEngine::synchronize(Common::Serializer &s) {
 
 /*------------------------------------------------------------------------*/
 
-bool VoyeurSavegameHeader::read(Common::InSaveFile *f) {
+bool VoyeurSavegameHeader::read(Common::InSaveFile *f, bool skipThumbnail) {
-	_thumbnail = NULL;
-
 	uint32 signature = f->readUint32BE();
 	if (signature != MKTAG('V', 'O', 'Y', 'R')) {
 		warning("Invalid savegame");
@@ -875,9 +870,9 @@ bool VoyeurSavegameHeader::read(Common::InSaveFile *f) {
 		_saveName += c;
 
 	// Get the thumbnail
-	_thumbnail = Graphics::loadThumbnail(*f);
+	if (!Graphics::loadThumbnail(*f, _thumbnail, skipThumbnail)) {
-	if (!_thumbnail)
 		return false;
+	}
 
 	// Read in the save datet/ime
 	_saveYear = f->readSint16LE();

engines/voyeur/voyeur.h

@@ -312,7 +312,7 @@ struct VoyeurSavegameHeader {
 	/**
 	 * Read in the header from the specified file
 	 */
-	bool read(Common::InSaveFile *f);
+	bool read(Common::InSaveFile *f, bool skipThumbnail = true);
 
 	/**
 	 * Write out header information to the specified file

engines/xeen/detection.cpp

@@ -170,12 +170,9 @@ SaveStateList XeenMetaEngine::listSaves(const char *target) const {
 			Common::InSaveFile *in = g_system->getSavefileManager()->openForLoading(*file);
 
 			if (in) {
-				Xeen::SavesManager::readSavegameHeader(in, header);
+				if (Xeen::SavesManager::readSavegameHeader(in, header))
-				saveList.push_back(SaveStateDescriptor(slot, header._saveName));
+					saveList.push_back(SaveStateDescriptor(slot, header._saveName));
 
-				if (header._thumbnail)
-					header._thumbnail->free();
-				delete header._thumbnail;
 				delete in;
 			}
 		}
@@ -200,7 +197,11 @@ SaveStateDescriptor XeenMetaEngine::querySaveMetaInfos(const char *target, int s
 
 	if (f) {
 		Xeen::XeenSavegameHeader header;
-		Xeen::SavesManager::readSavegameHeader(f, header);
+		if (!Xeen::SavesManager::readSavegameHeader(f, header, false)) {
+			delete f;
+			return SaveStateDescriptor();
+		}
+
 		delete f;
 
 		// Create the return descriptor

engines/xeen/saves.cpp

@@ -48,9 +48,8 @@ SavesManager::~SavesManager() {
 const char *const SAVEGAME_STR = "XEEN";
 #define SAVEGAME_STR_SIZE 6
 
-bool SavesManager::readSavegameHeader(Common::InSaveFile *in, XeenSavegameHeader &header) {
+WARN_UNUSED_RESULT bool SavesManager::readSavegameHeader(Common::InSaveFile *in, XeenSavegameHeader &header, bool skipThumbnail) {
 	char saveIdentBuffer[SAVEGAME_STR_SIZE + 1];
-	header._thumbnail = nullptr;
 
 	// Validate the header Id
 	in->read(saveIdentBuffer, SAVEGAME_STR_SIZE + 1);
@@ -68,9 +67,9 @@ bool SavesManager::readSavegameHeader(Common::InSaveFile *in, XeenSavegameHeader
 		header._saveName += ch;
 
 	// Get the thumbnail
-	header._thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header._thumbnail, skipThumbnail)) {
-	if (!header._thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header._year = in->readSint16LE();
@@ -168,11 +167,6 @@ Common::Error SavesManager::loadGameState(int slot) {
 	if (!readSavegameHeader(saveFile, header))
 		error("Invalid savegame");
 
-	if (header._thumbnail) {
-		header._thumbnail->free();
-		delete header._thumbnail;
-	}
-
 	// Set the total play time
 	events.setPlayTime(header._totalFrames);
 

engines/xeen/saves.h

@@ -65,7 +65,7 @@ public:
 	/**
 	 * Read in a savegame header
 	 */
-	static bool readSavegameHeader(Common::InSaveFile *in, XeenSavegameHeader &header);
+	WARN_UNUSED_RESULT static bool readSavegameHeader(Common::InSaveFile *in, XeenSavegameHeader &header, bool skipThumbnail = true);
 
 	/**
 	 * Write out a savegame header

engines/zvision/detection.cpp

@@ -178,7 +178,7 @@ SaveStateDescriptor ZVisionMetaEngine::querySaveMetaInfos(const char *target, in
 
 		// We only use readSaveGameHeader() here, which doesn't need an engine callback
 		ZVision::SaveManager *zvisionSaveMan = new ZVision::SaveManager(NULL);
-		bool successfulRead = zvisionSaveMan->readSaveGameHeader(in, header);
+		bool successfulRead = zvisionSaveMan->readSaveGameHeader(in, header, false);
 		delete zvisionSaveMan;
 		delete in;
 

engines/zvision/file/save_manager.cpp

@@ -189,7 +189,7 @@ Common::Error SaveManager::loadGame(int slot) {
 	return Common::kNoError;
 }
 
-bool SaveManager::readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader &header) {
+bool SaveManager::readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader &header, bool skipThumbnail) {
 	uint32 tag = in->readUint32BE();
 	// Check if it's original savegame than fill header structure
 	if (tag == MKTAG('Z', 'N', 'S', 'G')) {
@@ -232,9 +232,9 @@ bool SaveManager::readSaveGameHeader(Common::InSaveFile *in, SaveGameHeader &hea
 		header.saveName += ch;
 
 	// Get the thumbnail
-	header.thumbnail = Graphics::loadThumbnail(*in);
+	if (!Graphics::loadThumbnail(*in, header.thumbnail, skipThumbnail)) {
-	if (!header.thumbnail)
 		return false;
+	}
 
 	// Read in save date/time
 	header.saveYear = in->readSint16LE();

engines/zvision/file/save_manager.h

@@ -94,7 +94,7 @@ public:
 	Common::Error loadGame(int slot);
 
 	Common::SeekableReadStream *getSlotFile(uint slot);
-	bool readSaveGameHeader(Common::SeekableReadStream *in, SaveGameHeader &header);
+	bool readSaveGameHeader(Common::SeekableReadStream *in, SaveGameHeader &header, bool skipThumbnail = true);
 
 	void prepareSaveBuffer();
 	void flushSaveBuffer();

graphics/thumbnail.cpp

@@ -147,7 +147,11 @@ bool skipThumbnail(Common::SeekableReadStream &in) {
 	return true;
 }
 
-Graphics::Surface *loadThumbnail(Common::SeekableReadStream &in) {
+bool loadThumbnail(Common::SeekableReadStream &in, Graphics::Surface *&thumbnail, bool skipThumbnail) {
+	if (skipThumbnail) {
+		return Graphics::skipThumbnail(in);
+	}
+
 	const uint32 position = in.pos();
 	ThumbnailHeader header;
 	HeaderState headerState = loadHeader(in, header, true);
@@ -160,32 +164,32 @@ Graphics::Surface *loadThumbnail(Common::SeekableReadStream &in) {
 	// stream at this point then.
 	if (headerState == kHeaderNone) {
 		in.seek(position, SEEK_SET);
-		return 0;
+		return false;
 	} else if (headerState == kHeaderUnsupported) {
 		in.seek(header.size - (in.pos() - position), SEEK_CUR);
-		return 0;
+		return false;
 	}
 
 	if (header.format.bytesPerPixel != 2 && header.format.bytesPerPixel != 4) {
 		warning("trying to load thumbnail with unsupported bit depth %d", header.format.bytesPerPixel);
-		return 0;
+		return false;
 	}
 
-	Graphics::Surface *const to = new Graphics::Surface();
+	thumbnail = new Graphics::Surface();
-	to->create(header.width, header.height, header.format);
+	thumbnail->create(header.width, header.height, header.format);
 
-	for (int y = 0; y < to->h; ++y) {
+	for (int y = 0; y < thumbnail->h; ++y) {
 		switch (header.format.bytesPerPixel) {
 		case 2: {
-			uint16 *pixels = (uint16 *)to->getBasePtr(0, y);
+			uint16 *pixels = (uint16 *)thumbnail->getBasePtr(0, y);
-			for (uint x = 0; x < to->w; ++x) {
+			for (uint x = 0; x < thumbnail->w; ++x) {
 				*pixels++ = in.readUint16BE();
 			}
 			} break;
 
 		case 4: {
-			uint32 *pixels = (uint32 *)to->getBasePtr(0, y);
+			uint32 *pixels = (uint32 *)thumbnail->getBasePtr(0, y);
-			for (uint x = 0; x < to->w; ++x) {
+			for (uint x = 0; x < thumbnail->w; ++x) {
 				*pixels++ = in.readUint32BE();
 			}
 			} break;
@@ -194,7 +198,7 @@ Graphics::Surface *loadThumbnail(Common::SeekableReadStream &in) {
 			assert(0);
 		}
 	}
-	return to;
+	return true;
 }
 
 bool saveThumbnail(Common::WriteStream &out) {

graphics/thumbnail.h

@@ -52,7 +52,7 @@ bool skipThumbnail(Common::SeekableReadStream &in);
 /**
  * Loads a thumbnail from the given input stream.
  */
-Graphics::Surface *loadThumbnail(Common::SeekableReadStream &in);
+bool loadThumbnail(Common::SeekableReadStream &in, Graphics::Surface *&thumbnail, bool skipThumbnail = false);
 
 /**
  * Saves a thumbnail to the given write stream.