SCI: Clean-ups to file handling code

Preparations for adding support for RAMA's custom save game format.

4 files changed, 158 insertions, 144 deletions

diff --git a/engines/sci/engine/file.cpp b/engines/sci/engine/file.cpp
index 7e218bc81c..f3c04dd48c 100644
--- a/engines/sci/engine/file.cpp
+++ b/engines/sci/engine/file.cpp
@@ -34,28 +34,7 @@
 namespace Sci {
 
 #ifdef ENABLE_SCI32
-/**
- * A MemoryWriteStreamDynamic with additional read functionality.
- * The read and write functions share a single stream position.
- */
-class MemoryDynamicRWStream : public Common::MemoryWriteStreamDynamic, public Common::SeekableReadStream {
-protected:
-	bool _eos;
-public:
-	MemoryDynamicRWStream(DisposeAfterUse::Flag disposeMemory = DisposeAfterUse::NO) : MemoryWriteStreamDynamic(disposeMemory), _eos(false) { }
-
-	uint32 read(void *dataPtr, uint32 dataSize);
-
-	bool eos() const { return _eos; }
-	int32 pos() const { return _pos; }
-	int32 size() const { return _size; }
-	void clearErr() { _eos = false; Common::MemoryWriteStreamDynamic::clearErr(); }
-	bool seek(int32 offs, int whence = SEEK_SET) { return Common::MemoryWriteStreamDynamic::seek(offs, whence); }
-
-};
-
-uint32 MemoryDynamicRWStream::read(void *dataPtr, uint32 dataSize)
-{
+uint32 MemoryDynamicRWStream::read(void *dataPtr, uint32 dataSize) {
 	// Read at most as many bytes as are still available...
 	if (dataSize > _size - _pos) {
 		dataSize = _size - _pos;
@@ -69,40 +48,18 @@ uint32 MemoryDynamicRWStream::read(void *dataPtr, uint32 dataSize)
 	return dataSize;
 }
 
-/**
- * A MemoryDynamicRWStream intended to re-write a file.
- * It reads the contents of `inFile` in the constructor, and writes back
- * the changes to `fileName` in the destructor (and when calling commit() ).
- */
-class SaveFileRewriteStream : public MemoryDynamicRWStream {
-public:
-	SaveFileRewriteStream(Common::String fileName,
-	                      Common::SeekableReadStream *inFile,
-	                      kFileOpenMode mode, bool compress);
-	virtual ~SaveFileRewriteStream();
-
-	virtual uint32 write(const void *dataPtr, uint32 dataSize) { _changed = true; return MemoryDynamicRWStream::write(dataPtr, dataSize); }
-
-	void commit(); //< Save back to disk
-
-protected:
-	Common::String _fileName;
-	bool _compress;
-	bool _changed;
-};
-
-SaveFileRewriteStream::SaveFileRewriteStream(Common::String fileName,
+SaveFileRewriteStream::SaveFileRewriteStream(const Common::String &fileName,
                                              Common::SeekableReadStream *inFile,
                                              kFileOpenMode mode,
-                                             bool compress)
-: MemoryDynamicRWStream(DisposeAfterUse::YES),
-  _fileName(fileName), _compress(compress)
-{
-	const bool truncate = mode == _K_FILE_MODE_CREATE;
-	const bool seekToEnd = mode == _K_FILE_MODE_OPEN_OR_CREATE;
+                                             bool compress) :
+	MemoryDynamicRWStream(DisposeAfterUse::YES),
+	_fileName(fileName),
+	_compress(compress) {
+	const bool truncate = (mode == kFileOpenModeCreate);
+	const bool seekToEnd = (mode == kFileOpenModeOpenOrCreate);
 
 	if (!truncate && inFile) {
-		unsigned int s = inFile->size();
+		const uint s = inFile->size();
 		ensureCapacity(s);
 		inFile->read(_data, s);
 		if (seekToEnd) {
@@ -119,16 +76,14 @@ SaveFileRewriteStream::~SaveFileRewriteStream() {
 }
 
 void SaveFileRewriteStream::commit() {
-	// Write contents of buffer back to file
-
-	if (_changed) {
-		Common::WriteStream *outFile = g_sci->getSaveFileManager()->openForSaving(_fileName, _compress);
-		outFile->write(_data, _size);
-		delete outFile;
-		_changed = false;
+	if (!_changed) {
+		return;
 	}
-}
 
+	Common::ScopedPtr<Common::WriteStream> outFile(g_sci->getSaveFileManager()->openForSaving(_fileName, _compress));
+	outFile->write(_data, _size);
+	_changed = false;
+}
 #endif
 
 uint findFreeFileHandle(EngineState *s) {
@@ -217,8 +172,19 @@ reg_t file_open(EngineState *s, const Common::String &filename, kFileOpenMode mo
 	}
 
 #ifdef ENABLE_SCI32
-	if ((g_sci->getGameId() == GID_PHANTASMAGORIA && (filename == "phantsg.dir" || filename == "chase.dat" || filename == "tmp.dat")) ||
-	    (g_sci->getGameId() == GID_PQSWAT && filename == "swat.dat")) {
+	bool isRewritableFile;
+	switch (g_sci->getGameId()) {
+	case GID_PHANTASMAGORIA:
+		isRewritableFile = (filename == "phantsg.dir" || filename == "chase.dat" || filename == "tmp.dat");
+		break;
+	case GID_PQSWAT:
+		isRewritableFile = (filename == "swat.dat");
+		break;
+	default:
+		isRewritableFile = false;
+	}
+
+	if (isRewritableFile) {
 		debugC(kDebugLevelFile, "  -> file_open opening %s for rewriting", wrappedName.c_str());
 
 		inFile = saveFileMan->openForLoading(wrappedName);
@@ -227,8 +193,8 @@ reg_t file_open(EngineState *s, const Common::String &filename, kFileOpenMode mo
 		if (!inFile)
 			inFile = SearchMan.createReadStreamForMember(englishName);
 
-		if (mode == _K_FILE_MODE_OPEN_OR_FAIL && !inFile) {
-			debugC(kDebugLevelFile, "  -> file_open(_K_FILE_MODE_OPEN_OR_FAIL): failed to open file '%s'", englishName.c_str());
+		if (mode == kFileOpenModeOpenOrFail && !inFile) {
+			debugC(kDebugLevelFile, "  -> file_open(kFileOpenModeOpenOrFail): failed to open file '%s'", englishName.c_str());
 			return SIGNAL_REG;
 		}
 
@@ -241,7 +207,7 @@ reg_t file_open(EngineState *s, const Common::String &filename, kFileOpenMode mo
 		outFile = stream;
 	} else
 #endif
-	if (mode == _K_FILE_MODE_OPEN_OR_FAIL) {
+	if (mode == kFileOpenModeOpenOrFail) {
 		// Try to open file, abort if not possible
 		inFile = saveFileMan->openForLoading(wrappedName);
 		// If no matching savestate exists: fall back to reading from a regular
@@ -250,19 +216,19 @@ reg_t file_open(EngineState *s, const Common::String &filename, kFileOpenMode mo
 			inFile = SearchMan.createReadStreamForMember(englishName);
 
 		if (!inFile)
-			debugC(kDebugLevelFile, "  -> file_open(_K_FILE_MODE_OPEN_OR_FAIL): failed to open file '%s'", englishName.c_str());
-	} else if (mode == _K_FILE_MODE_CREATE) {
+			debugC(kDebugLevelFile, "  -> file_open(kFileOpenModeOpenOrFail): failed to open file '%s'", englishName.c_str());
+	} else if (mode == kFileOpenModeCreate) {
 		// Create the file, destroying any content it might have had
 		outFile = saveFileMan->openForSaving(wrappedName, isCompressed);
 		if (!outFile)
-			debugC(kDebugLevelFile, "  -> file_open(_K_FILE_MODE_CREATE): failed to create file '%s'", englishName.c_str());
-	} else if (mode == _K_FILE_MODE_OPEN_OR_CREATE) {
+			debugC(kDebugLevelFile, "  -> file_open(kFileOpenModeCreate): failed to create file '%s'", englishName.c_str());
+	} else if (mode == kFileOpenModeOpenOrCreate) {
 		// Try to open file, create it if it doesn't exist
 		outFile = saveFileMan->openForSaving(wrappedName, isCompressed);
 		if (!outFile)
-			debugC(kDebugLevelFile, "  -> file_open(_K_FILE_MODE_CREATE): failed to create file '%s'", englishName.c_str());
+			debugC(kDebugLevelFile, "  -> file_open(kFileOpenModeCreate): failed to create file '%s'", englishName.c_str());
 
-		// QfG1 opens the character export file with _K_FILE_MODE_CREATE first,
+		// QfG1 opens the character export file with kFileOpenModeCreate first,
 		// closes it immediately and opens it again with this here. Perhaps
 		// other games use this for read access as well. I guess changing this
 		// whole code into using virtual files and writing them after close
@@ -287,7 +253,7 @@ reg_t file_open(EngineState *s, const Common::String &filename, kFileOpenMode mo
 }
 
 FileHandle *getFileFromHandle(EngineState *s, uint handle) {
-	if ((handle == 0) || ((handle >= VIRTUALFILE_HANDLE_START) && (handle <= VIRTUALFILE_HANDLE_END))) {
+	if ((handle == 0) || ((handle >= kVirtualFileHandleStart) && (handle <= kVirtualFileHandleEnd))) {
 		error("Attempt to use invalid file handle (%d)", handle);
 		return 0;
 	}
@@ -333,38 +299,34 @@ static bool _savegame_sort_byDate(const SavegameDesc &l, const SavegameDesc &r)
 	return (l.time > r.time);
 }
 
-bool fillSavegameDesc(const Common::String &filename, SavegameDesc *desc) {
+bool fillSavegameDesc(const Common::String &filename, SavegameDesc &desc) {
 	Common::SaveFileManager *saveFileMan = g_sci->getSaveFileManager();
-	Common::SeekableReadStream *in;
-	if ((in = saveFileMan->openForLoading(filename)) == nullptr) {
+	Common::ScopedPtr<Common::SeekableReadStream> in(saveFileMan->openForLoading(filename));
+	if (!in) {
 		return false;
 	}
 
 	SavegameMetadata meta;
-	if (!get_savegame_metadata(in, &meta) || meta.name.empty()) {
-		// invalid
-		delete in;
+	if (!get_savegame_metadata(in.get(), &meta) || meta.name.empty()) {
 		return false;
 	}
-	delete in;
 
 	const int id = strtol(filename.end() - 3, NULL, 10);
-	desc->id = id;
-	desc->date = meta.saveDate;
+	desc.id = id;
 	// We need to fix date in here, because we save DDMMYYYY instead of
 	// YYYYMMDD, so sorting wouldn't work
-	desc->date = ((desc->date & 0xFFFF) << 16) | ((desc->date & 0xFF0000) >> 8) | ((desc->date & 0xFF000000) >> 24);
-	desc->time = meta.saveTime;
-	desc->version = meta.version;
-	desc->gameVersion = meta.gameVersion;
-	desc->script0Size = meta.script0Size;
-	desc->gameObjectOffset = meta.gameObjectOffset;
+	desc.date = ((meta.saveDate & 0xFFFF) << 16) | ((meta.saveDate & 0xFF0000) >> 8) | ((meta.saveDate & 0xFF000000) >> 24);
+	desc.time = meta.saveTime;
+	desc.version = meta.version;
+	desc.gameVersion = meta.gameVersion;
+	desc.script0Size = meta.script0Size;
+	desc.gameObjectOffset = meta.gameObjectOffset;
 #ifdef ENABLE_SCI32
 	if (g_sci->getGameId() == GID_SHIVERS) {
-		desc->lowScore = meta.lowScore;
-		desc->highScore = meta.highScore;
+		desc.lowScore = meta.lowScore;
+		desc.highScore = meta.highScore;
 	} else if (g_sci->getGameId() == GID_MOTHERGOOSEHIRES) {
-		desc->avatarId = meta.avatarId;
+		desc.avatarId = meta.avatarId;
 	}
 #endif
 
@@ -372,10 +334,10 @@ bool fillSavegameDesc(const Common::String &filename, SavegameDesc *desc) {
 		meta.name.deleteLastChar();
 
 	// At least Phant2 requires use of strncpy, since it creates save game
-	// names of exactly SCI_MAX_SAVENAME_LENGTH
-	strncpy(desc->name, meta.name.c_str(), SCI_MAX_SAVENAME_LENGTH);
+	// names of exactly kMaxSaveNameLength
+	strncpy(desc.name, meta.name.c_str(), kMaxSaveNameLength);
 
-	return desc;
+	return true;
 }
 
 // Create an array containing all found savedgames, sorted by creation date
@@ -394,8 +356,9 @@ void listSavegames(Common::Array<SavegameDesc> &saves) {
 #endif
 
 		SavegameDesc desc;
-		fillSavegameDesc(filename, &desc);
-		saves.push_back(desc);
+		if (fillSavegameDesc(filename, desc)) {
+			saves.push_back(desc);
+		}
 	}
 
 	// Sort the list by creation date of the saves
diff --git a/engines/sci/engine/file.h b/engines/sci/engine/file.h
index 1657dd3c7c..28a3c5801b 100644
--- a/engines/sci/engine/file.h
+++ b/engines/sci/engine/file.h
@@ -23,20 +23,21 @@
 #ifndef SCI_ENGINE_FILE_H
 #define SCI_ENGINE_FILE_H
 
+#include "common/memstream.h"
 #include "common/str-array.h"
 #include "common/stream.h"
 
 namespace Sci {
 
 enum kFileOpenMode {
-	_K_FILE_MODE_OPEN_OR_CREATE = 0,
-	_K_FILE_MODE_OPEN_OR_FAIL = 1,
-	_K_FILE_MODE_CREATE = 2
+	kFileOpenModeOpenOrCreate = 0,
+	kFileOpenModeOpenOrFail = 1,
+	kFileOpenModeCreate = 2
 };
 
 enum {
-	SCI_MAX_SAVENAME_LENGTH = 36, ///< Maximum length of a savegame name (including optional terminator character).
-	MAX_SAVEGAME_NR = 20 ///< Maximum number of savegames
+	kMaxSaveNameLength = 36, ///< Maximum length of a savegame name (including optional terminator character).
+	kMaxNumSaveGames = 20 ///< Maximum number of savegames
 };
 
 #ifdef ENABLE_SCI32
@@ -51,10 +52,12 @@ enum {
 };
 #endif
 
-#define VIRTUALFILE_HANDLE_START 32000
-#define VIRTUALFILE_HANDLE_SCI32SAVE 32100
-#define VIRTUALFILE_HANDLE_SCIAUDIO 32300
-#define VIRTUALFILE_HANDLE_END 32300
+enum {
+	kVirtualFileHandleStart = 32000,
+	kVirtualFileHandleSci32Save = 32100,
+	kVirtualFileHandleSciAudio = 32300,
+	kVirtualFileHandleEnd = 32300
+};
 
 struct SavegameDesc {
 	int16 id;
@@ -62,7 +65,7 @@ struct SavegameDesc {
 	int date;
 	int time;
 	int version;
-	char name[SCI_MAX_SAVENAME_LENGTH];
+	char name[kMaxSaveNameLength];
 	Common::String gameVersion;
 	uint32 script0Size;
 	uint32 gameObjectOffset;
@@ -112,7 +115,58 @@ private:
 	void addAsVirtualFiles(Common::String title, Common::String fileMask);
 };
 
+#ifdef ENABLE_SCI32
+/**
+ * A MemoryWriteStreamDynamic with additional read functionality.
+ * The read and write functions share a single stream position.
+ */
+class MemoryDynamicRWStream : public Common::MemoryWriteStreamDynamic, public Common::SeekableReadStream {
+public:
+	MemoryDynamicRWStream(DisposeAfterUse::Flag disposeMemory = DisposeAfterUse::NO) : MemoryWriteStreamDynamic(disposeMemory), _eos(false) { }
+
+	uint32 read(void *dataPtr, uint32 dataSize);
+
+	bool eos() const { return _eos; }
+	int32 pos() const { return _pos; }
+	int32 size() const { return _size; }
+	void clearErr() { _eos = false; Common::MemoryWriteStreamDynamic::clearErr(); }
+	bool seek(int32 offs, int whence = SEEK_SET) { return Common::MemoryWriteStreamDynamic::seek(offs, whence); }
+
+protected:
+	bool _eos;
+};
+
+/**
+ * A MemoryDynamicRWStream intended to re-write a file.
+ * It reads the contents of `inFile` in the constructor, and writes back
+ * the changes to `fileName` in the destructor (and when calling commit() ).
+ */
+class SaveFileRewriteStream : public MemoryDynamicRWStream {
+public:
+	SaveFileRewriteStream(const Common::String &fileName,
+	                      Common::SeekableReadStream *inFile,
+	                      kFileOpenMode mode, bool compress);
+	virtual ~SaveFileRewriteStream();
+
+	virtual uint32 write(const void *dataPtr, uint32 dataSize) { _changed = true; return MemoryDynamicRWStream::write(dataPtr, dataSize); }
+
+	void commit(); //< Save back to disk
+
+protected:
+	Common::String _fileName;
+	bool _compress;
+	bool _changed;
+};
+
+#endif
+
 uint findFreeFileHandle(EngineState *s);
+reg_t file_open(EngineState *s, const Common::String &filename, kFileOpenMode mode, bool unwrapFilename);
+FileHandle *getFileFromHandle(EngineState *s, uint handle);
+int fgets_wrapper(EngineState *s, char *dest, int maxsize, int handle);
+void listSavegames(Common::Array<SavegameDesc> &saves);
+int findSavegame(Common::Array<SavegameDesc> &saves, int16 savegameId);
+bool fillSavegameDesc(const Common::String &filename, SavegameDesc &desc);
 
 } // End of namespace Sci
 
diff --git a/engines/sci/engine/kfile.cpp b/engines/sci/engine/kfile.cpp
index 3b8420849a..b5aac43d10 100644
--- a/engines/sci/engine/kfile.cpp
+++ b/engines/sci/engine/kfile.cpp
@@ -48,13 +48,6 @@
 
 namespace Sci {
 
-extern reg_t file_open(EngineState *s, const Common::String &filename, kFileOpenMode mode, bool unwrapFilename);
-extern FileHandle *getFileFromHandle(EngineState *s, uint handle);
-extern int fgets_wrapper(EngineState *s, char *dest, int maxsize, int handle);
-extern void listSavegames(Common::Array<SavegameDesc> &saves);
-extern int findSavegame(Common::Array<SavegameDesc> &saves, int16 savegameId);
-extern bool fillSavegameDesc(const Common::String &filename, SavegameDesc *desc);
-
 /**
  * Writes the cwd to the supplied address and returns the address in acc.
  */
@@ -319,11 +312,11 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 	if (g_sci->getGameId() == GID_SHIVERS && name.hasSuffix(".SG")) {
 		// Shivers stores the name and score of save games in separate %d.SG
 		// files, which are used by the save/load screen
-		if (mode == _K_FILE_MODE_OPEN_OR_CREATE || mode == _K_FILE_MODE_CREATE) {
+		if (mode == kFileOpenModeOpenOrCreate || mode == kFileOpenModeCreate) {
 			// Suppress creation of the SG file, since it is not necessary
 			debugC(kDebugLevelFile, "Not creating unused file %s", name.c_str());
 			return SIGNAL_REG;
-		} else if (mode == _K_FILE_MODE_OPEN_OR_FAIL) {
+		} else if (mode == kFileOpenModeOpenOrFail) {
 			// Create a virtual file containing the save game description
 			// and current score progress, as the game scripts expect.
 			int saveNo;
@@ -331,7 +324,9 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 			saveNo += kSaveIdShift;
 
 			SavegameDesc save;
-			fillSavegameDesc(g_sci->getSavegameName(saveNo), &save);
+			if (!fillSavegameDesc(g_sci->getSavegameName(saveNo), save)) {
+				return SIGNAL_REG;
+			}
 
 			Common::String score;
 			if (!save.highScore) {
@@ -340,9 +335,9 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 				score = Common::String::format("%u%03u", save.highScore, save.lowScore);
 			}
 
-			const uint nameLength = Common::strnlen(save.name, SCI_MAX_SAVENAME_LENGTH);
+			const uint nameLength = Common::strnlen(save.name, kMaxSaveNameLength);
 			const uint size = nameLength + /* \r\n */ 2 + score.size();
-			char *buffer = (char *)malloc(size);
+			byte *buffer = (byte *)malloc(size);
 			memcpy(buffer, save.name, nameLength);
 			buffer[nameLength] = '\r';
 			buffer[nameLength + 1] = '\n';
@@ -350,7 +345,7 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 
 			const uint handle = findFreeFileHandle(s);
 
-			s->_fileHandles[handle]._in = new Common::MemoryReadStream((byte *)buffer, size, DisposeAfterUse::YES);
+			s->_fileHandles[handle]._in = new Common::MemoryReadStream(buffer, size, DisposeAfterUse::YES);
 			s->_fileHandles[handle]._out = nullptr;
 			s->_fileHandles[handle]._name = "";
 
@@ -359,11 +354,11 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 	} else if (g_sci->getGameId() == GID_MOTHERGOOSEHIRES && name.hasSuffix(".DTA")) {
 		// MGDX stores the name and avatar ID in separate %d.DTA files, which
 		// are used by the save/load screen
-		if (mode == _K_FILE_MODE_OPEN_OR_CREATE || mode == _K_FILE_MODE_CREATE) {
+		if (mode == kFileOpenModeOpenOrCreate || mode == kFileOpenModeCreate) {
 			// Suppress creation of the DTA file, since it is not necessary
 			debugC(kDebugLevelFile, "Not creating unused file %s", name.c_str());
 			return SIGNAL_REG;
-		} else if (mode == _K_FILE_MODE_OPEN_OR_FAIL) {
+		} else if (mode == kFileOpenModeOpenOrFail) {
 			// Create a virtual file containing the save game description
 			// and avatar ID, as the game scripts expect.
 			int saveNo;
@@ -380,10 +375,12 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 			saveNo += kSaveIdShift;
 
 			SavegameDesc save;
-			fillSavegameDesc(g_sci->getSavegameName(saveNo), &save);
+			if (!fillSavegameDesc(g_sci->getSavegameName(saveNo), save)) {
+				return SIGNAL_REG;
+			}
 
 			const Common::String avatarId = Common::String::format("%02d", save.avatarId);
-			const uint nameLength = Common::strnlen(save.name, SCI_MAX_SAVENAME_LENGTH);
+			const uint nameLength = Common::strnlen(save.name, kMaxSaveNameLength);
 			const uint size = nameLength + /* \r\n */ 2 + avatarId.size() + 1;
 			char *buffer = (char *)malloc(size);
 			memcpy(buffer, save.name, nameLength);
@@ -404,21 +401,21 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 		// catalogue, but since we do not create catalogues for most SCI32
 		// games, ignore the write
 		if (name == "temp.tmp") {
-			return make_reg(0, VIRTUALFILE_HANDLE_SCI32SAVE);
+			return make_reg(0, kVirtualFileHandleSci32Save);
 		}
 
 		// KQ7 tries to read out game information from catalogues directly
 		// instead of using the standard kSaveGetFiles function
 		if (name == "kq7cdsg.cat") {
-			if (mode == _K_FILE_MODE_OPEN_OR_CREATE || mode == _K_FILE_MODE_CREATE) {
+			if (mode == kFileOpenModeOpenOrCreate || mode == kFileOpenModeCreate) {
 				// Suppress creation of the catalogue file, since it is not necessary
 				debugC(kDebugLevelFile, "Not creating unused file %s", name.c_str());
 				return SIGNAL_REG;
-			} else if (mode == _K_FILE_MODE_OPEN_OR_FAIL) {
+			} else if (mode == kFileOpenModeOpenOrFail) {
 				Common::Array<SavegameDesc> saves;
 				listSavegames(saves);
 
-				const uint recordSize = sizeof(int16) + SCI_MAX_SAVENAME_LENGTH;
+				const uint recordSize = sizeof(int16) + kMaxSaveNameLength;
 				const uint numSaves = MIN<uint>(saves.size(), 10);
 				const uint size = numSaves * recordSize + /* terminator */ 2;
 				byte *const buffer = (byte *)malloc(size);
@@ -426,7 +423,7 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 				byte *out = buffer;
 				for (uint i = 0; i < numSaves; ++i) {
 					WRITE_UINT16(out, saves[i].id - kSaveIdShift);
-					strncpy((char *)out + sizeof(int16), saves[i].name, SCI_MAX_SAVENAME_LENGTH);
+					strncpy((char *)out + sizeof(int16), saves[i].name, kMaxSaveNameLength);
 					out += recordSize;
 				}
 				WRITE_UINT16(out, 0xFFFF);
@@ -453,7 +450,7 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 
 	// See kMakeSaveCatName
 	if (name == "fake.cat") {
-		return make_reg(0, VIRTUALFILE_HANDLE_SCI32SAVE);
+		return make_reg(0, kVirtualFileHandleSci32Save);
 	}
 
 	if (isSaveCatalogue(name)) {
@@ -461,7 +458,7 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 		if (exists) {
 			// Dummy handle is used to represent the catalogue and ignore any
 			// direct game script writes
-			return make_reg(0, VIRTUALFILE_HANDLE_SCI32SAVE);
+			return make_reg(0, kVirtualFileHandleSci32Save);
 		} else {
 			return SIGNAL_REG;
 		}
@@ -472,7 +469,7 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 
 	if (name.hasPrefix("sciAudio\\")) {
 		// fan-made sciAudio extension, don't create those files and instead return a virtual handle
-		return make_reg(0, VIRTUALFILE_HANDLE_SCIAUDIO);
+		return make_reg(0, kVirtualFileHandleSciAudio);
 	}
 
 	// QFG import rooms get a virtual filelisting instead of an actual one
@@ -495,7 +492,7 @@ reg_t kFileIOClose(EngineState *s, int argc, reg_t *argv) {
 
 	uint16 handle = argv[0].toUint16();
 
-	if (handle >= VIRTUALFILE_HANDLE_START) {
+	if (handle >= kVirtualFileHandleStart) {
 		// it's a virtual handle? ignore it
 		return getSciVersion() >= SCI_VERSION_2 ? TRUE_REG : SIGNAL_REG;
 	}
@@ -539,7 +536,7 @@ reg_t kFileIOWriteRaw(EngineState *s, int argc, reg_t *argv) {
 	uint16 size = argv[2].toUint16();
 
 #ifdef ENABLE_SCI32
-	if (handle == VIRTUALFILE_HANDLE_SCI32SAVE) {
+	if (handle == kVirtualFileHandleSci32Save) {
 		return make_reg(0, size);
 	}
 #endif
@@ -673,7 +670,7 @@ reg_t kFileIOWriteString(EngineState *s, int argc, reg_t *argv) {
 	// We skip creating these files, and instead handle the calls
 	// directly. Since the sciAudio calls are only creating text files,
 	// this is probably the most straightforward place to handle them.
-	if (handle == VIRTUALFILE_HANDLE_SCIAUDIO) {
+	if (handle == kVirtualFileHandleSciAudio) {
 		Common::List<ExecStack>::const_iterator iter = s->_executionStack.reverse_begin();
 		iter--;	// sciAudio
 		iter--;	// sciAudio child
@@ -872,7 +869,7 @@ reg_t kFileIOWriteWord(EngineState *s, int argc, reg_t *argv) {
 	uint16 handle = argv[0].toUint16();
 
 #ifdef ENABLE_SCI32
-	if (handle == VIRTUALFILE_HANDLE_SCI32SAVE) {
+	if (handle == kVirtualFileHandleSci32Save) {
 		return s->r_acc;
 	}
 #endif
@@ -1174,7 +1171,7 @@ reg_t kGetSaveFiles(EngineState *s, int argc, reg_t *argv) {
 
 	Common::Array<SavegameDesc> saves;
 	listSavegames(saves);
-	uint totalSaves = MIN<uint>(saves.size(), MAX_SAVEGAME_NR);
+	uint totalSaves = MIN<uint>(saves.size(), kMaxNumSaveGames);
 
 	Common::String game_id = s->_segMan->getString(argv[0]);
 
@@ -1187,14 +1184,14 @@ reg_t kGetSaveFiles(EngineState *s, int argc, reg_t *argv) {
 		totalSaves = 0;
 	}
 
-	const uint bufSize = (totalSaves * SCI_MAX_SAVENAME_LENGTH) + 1;
+	const uint bufSize = (totalSaves * kMaxSaveNameLength) + 1;
 	char *saveNames = new char[bufSize];
 	char *saveNamePtr = saveNames;
 
 	for (uint i = 0; i < totalSaves; i++) {
 		*slot++ = make_reg(0, saves[i].id + SAVEGAMEID_OFFICIALRANGE_START); // Store the virtual savegame ID (see above)
 		strcpy(saveNamePtr, saves[i].name);
-		saveNamePtr += SCI_MAX_SAVENAME_LENGTH;
+		saveNamePtr += kMaxSaveNameLength;
 	}
 
 	*saveNamePtr = 0; // Terminate list
@@ -1321,7 +1318,7 @@ reg_t kCheckSaveGame32(EngineState *s, int argc, reg_t *argv) {
 	}
 
 	SavegameDesc save;
-	if (!fillSavegameDesc(g_sci->getSavegameName(saveNo), &save)) {
+	if (!fillSavegameDesc(g_sci->getSavegameName(saveNo), save)) {
 		return NULL_REG;
 	}
 
@@ -1369,19 +1366,19 @@ reg_t kGetSaveFiles32(EngineState *s, int argc, reg_t *argv) {
 
 	// Normally SSCI limits to 20 games per directory, but ScummVM allows more
 	// than that with games that use the standard save-load dialogue
-	descriptions.resize(SCI_MAX_SAVENAME_LENGTH * saves.size() + 1, true);
+	descriptions.resize(kMaxSaveNameLength * saves.size() + 1, true);
 	saveIds.resize(saves.size() + 1, true);
 
 	for (uint i = 0; i < saves.size(); ++i) {
 		const SavegameDesc &save = saves[i];
-		char *target = &descriptions.charAt(SCI_MAX_SAVENAME_LENGTH * i);
+		char *target = &descriptions.charAt(kMaxSaveNameLength * i);
 		// At least Phant2 requires use of strncpy, since it creates save game
-		// names of exactly SCI_MAX_SAVENAME_LENGTH
-		strncpy(target, save.name, SCI_MAX_SAVENAME_LENGTH);
+		// names of exactly kMaxSaveNameLength
+		strncpy(target, save.name, kMaxSaveNameLength);
 		saveIds.setFromInt16(i, save.id - kSaveIdShift);
 	}
 
-	descriptions.charAt(SCI_MAX_SAVENAME_LENGTH * saves.size()) = '\0';
+	descriptions.charAt(kMaxSaveNameLength * saves.size()) = '\0';
 	saveIds.setFromInt16(saves.size(), 0);
 
 	return make_reg(0, saves.size());
diff --git a/engines/sci/engine/kgraphics.cpp b/engines/sci/engine/kgraphics.cpp
index 28e054bea9..e5336b37bc 100644
--- a/engines/sci/engine/kgraphics.cpp
+++ b/engines/sci/engine/kgraphics.cpp
@@ -932,7 +932,7 @@ void _k_GenericDrawControl(EngineState *s, reg_t controlObject, bool hilite) {
 			}
 		}
 
-		debugC(kDebugLevelGraphics, "drawing list control %04x:%04x to %d,%d, diff %d", PRINT_REG(controlObject), x, y, SCI_MAX_SAVENAME_LENGTH);
+		debugC(kDebugLevelGraphics, "drawing list control %04x:%04x to %d,%d", PRINT_REG(controlObject), x, y);
 		g_sci->_gfxControls16->kernelDrawList(rect, controlObject, maxChars, listCount, listStrings, fontId, style, upperPos, cursorPos, isAlias, hilite);
 		delete[] listStrings;
 		return;