Patch #1857121: Refactoring of the getSavePath method

svn-id: r30032

9 files changed, 135 insertions, 107 deletions

diff --git a/backends/platform/ds/arm9/source/gbampsave.h b/backends/platform/ds/arm9/source/gbampsave.h
index a131b6beba..6fd3098b99 100644
--- a/backends/platform/ds/arm9/source/gbampsave.h
+++ b/backends/platform/ds/arm9/source/gbampsave.h
@@ -24,7 +24,7 @@
 #define _GBAMPSAVE_H_
 
 #include "system.h"
-#include "common/savefile.h"
+#include "saves/default/default-saves.h"
 #include "ds-fs.h"
 
 #define SAVE_BUFFER_SIZE 100000
@@ -58,9 +58,7 @@ public:
 };
 
 
-class GBAMPSaveFileManager : public Common::SaveFileManager {
-
-	
+class GBAMPSaveFileManager : public DefaultSaveFileManager {
 public:
 	GBAMPSaveFileManager();
 	~GBAMPSaveFileManager();
@@ -81,5 +79,4 @@ public:
 	void listFiles();
 };
 
-
 #endif
diff --git a/backends/platform/ps2/savefile.cpp b/backends/platform/ps2/savefile.cpp
index b0f792f386..84bee4cf35 100644
--- a/backends/platform/ps2/savefile.cpp
+++ b/backends/platform/ps2/savefile.cpp
@@ -484,10 +484,6 @@ void Ps2SaveFileManager::listSavefiles(const char *prefix, bool *marks, int num)
 	_screen->wantAnim(false);
 }
 
-const char *Ps2SaveFileManager::getSavePath(void) const {
-	return "mc0:";
-}
-
 bool Ps2SaveFileManager::setupIcon(const char *dest, const char *ico, const char *descr1, const char *descr2) {
 	mcIcon icon_sys;
 	memset(&icon_sys, 0, sizeof(mcIcon));
diff --git a/backends/platform/ps2/savefile.h b/backends/platform/ps2/savefile.h
index 1eafd7eadb..5e3c12beb7 100644
--- a/backends/platform/ps2/savefile.h
+++ b/backends/platform/ps2/savefile.h
@@ -42,12 +42,10 @@ public:
 	virtual Common::OutSaveFile *openForSaving(const char *filename);
 	virtual void listSavefiles(const char *prefix, bool *marks, int num);
 
-	/** Get the path to the save game directory. */
-	virtual const char *getSavePath() const;
-
 	void writeSaveNonblocking(char *name, void *buf, uint32 size);
 	void saveThread(void);
 	void quit(void);
+	
 private:
 	bool setupIcon(const char *dest, const char *ico, const char *descr1, const char *descr2);
 
diff --git a/backends/saves/default/default-saves.cpp b/backends/saves/default/default-saves.cpp
index f4355e48cd..a969866d94 100644
--- a/backends/saves/default/default-saves.cpp
+++ b/backends/saves/default/default-saves.cpp
@@ -29,6 +29,7 @@
 #include "common/util.h"
 #include "common/fs.h"
 #include "common/file.h"
+#include "common/config-manager.h"
 #include "backends/saves/default/default-saves.h"
 #include "backends/saves/compressed/compressed-saves.h"
 
@@ -128,120 +129,132 @@ Common::StringList DefaultSaveFileManager::listSavefiles(const char *regex) {
 	return results;
 }
 
-Common::InSaveFile *DefaultSaveFileManager::openForLoading(const char *filename) {
-	char buf[256];
-	join_paths(filename, getSavePath(), buf, sizeof(buf));
-
-	StdioSaveFile *sf = new StdioSaveFile(buf, false);
-
-	if (!sf->isOpen()) {
-		delete sf;
-		sf = 0;
-	}
-	return wrapInSaveFile(sf);
-}
-
-Common::OutSaveFile *DefaultSaveFileManager::openForSaving(const char *filename) {
-	char buf[256];
-
-	// Ensure that the savepath exists and is writeable. If not, generate
-	// an appropriate error
-	const char *savePath = getSavePath();
+void DefaultSaveFileManager::checkPath(const char *path) {
+	clearError();
+	Common::String pathStr(path);
 
 #if defined(UNIX) || defined(__SYMBIAN32__)
 	struct stat sb;
-	clearError();
 
 	// Check whether the dir exists
-	if (stat(savePath, &sb) == -1) {
+	if (stat(path, &sb) == -1) {
 		// The dir does not exist, or stat failed for some other reason.
 		// If the problem was that the path pointed to nothing, try
 		// to create the dir (ENOENT case).
 		switch (errno) {
 		case EACCES:
-			setError(SFM_DIR_ACCESS, Common::String("Search or write permission denied"));
+			setError(SFM_DIR_ACCESS, "Search or write permission denied: "+pathStr);
 			break;
 #if !defined(__SYMBIAN32__)
 		case ELOOP:
-			setError(SFM_DIR_LOOP, Common::String("Too many symbolic links encountered while traversing the path"));
+			setError(SFM_DIR_LOOP, "Too many symbolic links encountered while traversing the path: "+pathStr);
 			break;
 #endif
 		case ENAMETOOLONG:
-			setError(SFM_DIR_NAMETOOLONG, Common::String("The path name is too long"));
+			setError(SFM_DIR_NAMETOOLONG, "The path name is too long: "+pathStr);
 			break;
 		case ENOENT:
-			if (mkdir(savePath, 0755) != 0) {
+			if (mkdir(path, 0755) != 0) {
 				// mkdir could fail for various reasons: The parent dir doesn't exist,
 				// or is not writeable, the path could be completly bogus, etc.
-				warning("mkdir for '%s' failed!", savePath);
+				warning("mkdir for '%s' failed!", path);
 				perror("mkdir");
 
 				switch (errno) {
 				case EACCES:
-					setError(SFM_DIR_ACCESS, Common::String("Search or write permission denied"));
+					setError(SFM_DIR_ACCESS, "Search or write permission denied: "+pathStr);
 					break;
 				case EMLINK:
-					setError(SFM_DIR_LINKMAX, Common::String("The link count of the parent directory would exceed {LINK_MAX}"));
+					setError(SFM_DIR_LINKMAX, "The link count of the parent directory would exceed {LINK_MAX}: "+pathStr);
 					break;
 #if !defined(__SYMBIAN32__)
 				case ELOOP:
-					setError(SFM_DIR_LOOP, Common::String("Too many symbolic links encountered while traversing the path"));
+					setError(SFM_DIR_LOOP, "Too many symbolic links encountered while traversing the path: "+pathStr);
 					break;
 #endif
 				case ENAMETOOLONG:
-					setError(SFM_DIR_NAMETOOLONG, Common::String("The path name is too long"));
+					setError(SFM_DIR_NAMETOOLONG, "The path name is too long: "+pathStr);
 					break;
 				case ENOENT:
-					setError(SFM_DIR_NOENT, Common::String("A component of the path does not exist, or the path is an empty string"));
+					setError(SFM_DIR_NOENT, "A component of the path does not exist, or the path is an empty string: "+pathStr);
 					break;
 				case ENOTDIR:
-					setError(SFM_DIR_NOTDIR, Common::String("A component of the path prefix is not a directory"));
+					setError(SFM_DIR_NOTDIR, "A component of the path prefix is not a directory: "+pathStr);
 					break;
 				case EROFS:
-					setError(SFM_DIR_ROFS, Common::String("The parent directory resides on a read-only file system"));
+					setError(SFM_DIR_ROFS, "The parent directory resides on a read-only file system:"+pathStr);
 					break;
 				}
-
-				return 0;
 			}
 			break;
 		case ENOTDIR:
-			setError(SFM_DIR_NOTDIR, Common::String("A component of the path prefix is not a directory"));
+			setError(SFM_DIR_NOTDIR, "A component of the path prefix is not a directory: "+pathStr);
 			break;
 		}
 	} else {
 		// So stat() succeeded. But is the path actually pointing to a directory?
 		if (!S_ISDIR(sb.st_mode)) {
-			setError(SFM_DIR_NOTDIR, Common::String("The given savepath is not a directory"));
-
-			return 0;
+			setError(SFM_DIR_NOTDIR, "The given savepath is not a directory: "+pathStr);
 		}
 	}
 #endif
+}
 
-	join_paths(filename, savePath, buf, sizeof(buf));
+Common::InSaveFile *DefaultSaveFileManager::openForLoading(const char *filename) {
+	// Ensure that the savepath is valid. If not, generate an appropriate error.
+	char buf[256];
+	const char *savePath = getSavePath();
+	checkPath(savePath);
+	
+	if (getError() == SFM_NO_ERROR) {
+		join_paths(filename, savePath, buf, sizeof(buf));
+		StdioSaveFile *sf = new StdioSaveFile(buf, false);
+	
+		if (!sf->isOpen()) {
+			delete sf;
+			sf = 0;
+		}
+		
+		return wrapInSaveFile(sf);
+	} else {
+		return 0;
+	}
+}
 
-	StdioSaveFile *sf = new StdioSaveFile(buf, true);
+Common::OutSaveFile *DefaultSaveFileManager::openForSaving(const char *filename) {
+	// Ensure that the savepath is valid. If not, generate an appropriate error.
+	char buf[256];
+	const char *savePath = getSavePath();
+	checkPath(savePath);
 
-	if (!sf->isOpen()) {
-		delete sf;
-		sf = 0;
+	if (getError() == SFM_NO_ERROR) {
+		join_paths(filename, savePath, buf, sizeof(buf));
+		StdioSaveFile *sf = new StdioSaveFile(buf, true);
+	
+		if (!sf->isOpen()) {
+			delete sf;
+			sf = 0;
+		}
+	
+		return wrapOutSaveFile(sf);
+	} else {
+		return 0;
 	}
-
-	return wrapOutSaveFile(sf);
 }
 
 bool DefaultSaveFileManager::removeSavefile(const char *filename) {
 	char buf[256];
+	clearError();
+	Common::String filenameStr;
 	join_paths(filename, getSavePath(), buf, sizeof(buf));
 	
 	if (remove(buf) != 0) {
 #ifndef _WIN32_WCE
 		if (errno == EACCES)
-			setError(SFM_DIR_ACCESS, Common::String("Search or write permission denied"));
+			setError(SFM_DIR_ACCESS, "Search or write permission denied: "+filenameStr);
 		
 		if (errno == ENOENT)
-			setError(SFM_DIR_NOENT, Common::String("A component of the path does not exist, or the path is an empty string"));
+			setError(SFM_DIR_NOENT, "A component of the path does not exist, or the path is an empty string: "+filenameStr);
 #endif
 		return false;
 	} else {
@@ -249,4 +262,34 @@ bool DefaultSaveFileManager::removeSavefile(const char *filename) {
 	}
 }
 
+const char *DefaultSaveFileManager::getSavePath() const {
+
+#if defined(PALMOS_MODE) || defined(__PSP__)
+	return SCUMMVM_SAVEPATH;
+#else
+
+	const char *dir = NULL;
+
+	// Try to use game specific savepath from config
+	dir = ConfMan.get("savepath").c_str();
+
+	// Work around a bug (#999122) in the original 0.6.1 release of
+	// ScummVM, which would insert a bad savepath value into config files.
+	if (0 == strcmp(dir, "None")) {
+		ConfMan.removeKey("savepath", ConfMan.getActiveDomainName());
+		ConfMan.flushToDisk();
+		dir = ConfMan.get("savepath").c_str();
+	}
+
+#ifdef _WIN32_WCE
+	if (dir[0] == 0)
+		dir = ConfMan.get("path").c_str();
+#endif
+
+	assert(dir);
+
+	return dir;
+#endif
+}
+
 #endif // !defined(DISABLE_DEFAULT_SAVEFILEMANAGER)
diff --git a/backends/saves/default/default-saves.h b/backends/saves/default/default-saves.h
index 8c4fe54bb7..8dd7db0367 100644
--- a/backends/saves/default/default-saves.h
+++ b/backends/saves/default/default-saves.h
@@ -29,12 +29,29 @@
 #include "common/savefile.h"
 #include "common/str.h"
 
+/**
+ * Provides a default savefile manager implementation for common platforms.
+ */
 class DefaultSaveFileManager : public Common::SaveFileManager {
 public:
 	virtual Common::StringList listSavefiles(const char *regex);
 	virtual Common::InSaveFile *openForLoading(const char *filename);
 	virtual Common::OutSaveFile *openForSaving(const char *filename);
 	virtual bool removeSavefile(const char *filename);
+
+protected:
+	/**
+	 * Get the path to the savegame directory.
+	 * Should only be used internally since some platforms
+	 * might implement savefiles in a completely different way.
+	 */
+	virtual const char *getSavePath() const;
+	
+	/**
+	 * Checks the given path for read access, existence, etc.
+	 * Sets the internal error and error message accordingly.
+	 */
+	void checkPath(const char *path);
 };
 
 #endif
diff --git a/backends/saves/savefile.cpp b/backends/saves/savefile.cpp
index 59ad144159..31caf275d7 100644
--- a/backends/saves/savefile.cpp
+++ b/backends/saves/savefile.cpp
@@ -24,7 +24,6 @@
  */
 
 #include "common/util.h"
-#include "common/config-manager.h"
 #include "common/savefile.h"
 
 #include <stdio.h>
@@ -72,34 +71,11 @@ bool SaveFileManager::renameSavefile(const char *oldFilename, const char *newFil
 	return success;
 }
 
-const char *SaveFileManager::getSavePath() const {
-
-#if defined(PALMOS_MODE) || defined(__PSP__)
-	return SCUMMVM_SAVEPATH;
-#else
-
-	const char *dir = NULL;
-
-	// Try to use game specific savepath from config
-	dir = ConfMan.get("savepath").c_str();
-
-	// Work around a bug (#999122) in the original 0.6.1 release of
-	// ScummVM, which would insert a bad savepath value into config files.
-	if (0 == strcmp(dir, "None")) {
-		ConfMan.removeKey("savepath", ConfMan.getActiveDomainName());
-		ConfMan.flushToDisk();
-		dir = ConfMan.get("savepath").c_str();
-	}
-
-#ifdef _WIN32_WCE
-	if (dir[0] == 0)
-		dir = ConfMan.get("path").c_str();
-#endif
-
-	assert(dir);
-
-	return dir;
-#endif
+String SaveFileManager::popErrorDesc() {
+	String err = _errorDesc;
+	clearError();
+	
+	return err;
 }
 
 } // End of namespace Common
diff --git a/common/savefile.h b/common/savefile.h
index 58ac48ea4a..4f9288ac05 100644
--- a/common/savefile.h
+++ b/common/savefile.h
@@ -109,7 +109,15 @@ public:
 	 * @return A string describing the last error.
 	 */
 	virtual String getErrorDesc() { return _errorDesc; }
-
+	
+	/**
+	 * Returns the last ocurred error description. If none ocurred, returns 0.
+	 * Also clears the last error state and description.
+	 * 
+	 * @return A string describing the last error.
+	 */
+	virtual String popErrorDesc();
+	
 	/**
 	 * Open the file with name filename in the given directory for saving.
 	 * @param filename	the filename
@@ -145,13 +153,6 @@ public:
 	 * returns a list of strings for all present file names.
 	 */
 	virtual Common::StringList listSavefiles(const char *regex) = 0;
-
-	/**
-	 * Get the path to the save game directory.
-	 * Should only be used for error messages, *never* to construct file paths
-	 * from it, since that is highly unportable!
-	 */
-	virtual const char *getSavePath() const;
 };
 
 } // End of namespace Common
diff --git a/engines/sky/control.cpp b/engines/sky/control.cpp
index ee8fee9ee8..eaea67e918 100644
--- a/engines/sky/control.cpp
+++ b/engines/sky/control.cpp
@@ -944,7 +944,7 @@ uint16 Control::saveRestorePanel(bool allowSave) {
 						refreshNames = true;
 					}
 					if (clickRes == NO_DISK_SPACE) {
-						displayMessage(0, "Could not save game in directory '%s'", _saveFileMan->getSavePath());
+						displayMessage(0, "Could not save the game. (%s)", _saveFileMan->popErrorDesc().c_str());
 						quitPanel = true;
 					}
 					if ((clickRes == CANCEL_PRESSED) || (clickRes == GAME_RESTORED))
@@ -1153,7 +1153,7 @@ void Control::saveDescriptions(uint8 *srcBuf) {
 		delete outf;
 	}
 	if (ioFailed)
-		displayMessage(NULL, "Unable to store Savegame names to file SKY-VM.SAV in directory %s", _saveFileMan->getSavePath());
+		displayMessage(NULL, "Unable to store Savegame names to file SKY-VM.SAV. (%s)", _saveFileMan->popErrorDesc().c_str());
 	free(tmpBuf);
 }
 
@@ -1167,7 +1167,7 @@ void Control::doAutoSave(void) {
 
 	outf = _saveFileMan->openForSaving(fName);
 	if (outf == NULL) {
-		displayMessage(0, "Unable to create autosave file '%s' in directory '%s'", fName, _saveFileMan->getSavePath());
+		displayMessage(0, "Unable to create autosave file '%s'. (%s)", fName, _saveFileMan->popErrorDesc().c_str());
 		return;
 	}
 	uint8 *saveData = (uint8 *)malloc(0x20000);
@@ -1177,7 +1177,7 @@ void Control::doAutoSave(void) {
 	outf->finalize();
 
 	if (outf->ioFailed())
-		displayMessage(0, "Unable to write autosave file '%s' in directory '%s'. Disk full?", fName, _saveFileMan->getSavePath());
+		displayMessage(0, "Unable to write autosave file '%s'. Disk full?", fName, _saveFileMan->popErrorDesc().c_str());
 
 	delete outf;
 	free(saveData);
diff --git a/engines/sword1/control.cpp b/engines/sword1/control.cpp
index 10097c0a1f..0ac32162b0 100644
--- a/engines/sword1/control.cpp
+++ b/engines/sword1/control.cpp
@@ -738,7 +738,7 @@ void Control::writeSavegameDescriptions(void) {
 
 	if (!outf) {
 		// Display an error message, and do nothing
-		displayMessage(0, "Can't create SAVEGAME.INF in directory '%s'", _saveFileMan->getSavePath());
+		displayMessage(0, "Can't create SAVEGAME.INF. (%s)", _saveFileMan->popErrorDesc().c_str());
 		return;
 	}
 
@@ -757,7 +757,7 @@ void Control::writeSavegameDescriptions(void) {
 	}
 	outf->finalize();
 	if (outf->ioFailed())
-		displayMessage(0, "Can't write to SAVEGAME.INF in directory '%s'. Device full?", _saveFileMan->getSavePath());
+		displayMessage(0, "Can't write to SAVEGAME.INF. Device full? (%s)", _saveFileMan->popErrorDesc().c_str());
 	delete outf;
 }
 
@@ -928,7 +928,7 @@ void Control::saveGameToFile(uint8 slot) {
 	outf = _saveFileMan->openForSaving(fName);
 	if (!outf) {
 		// Display an error message and do nothing
-		displayMessage(0, "Unable to create file '%s' in directory '%s'", fName, _saveFileMan->getSavePath());
+		displayMessage(0, "Unable to create file '%s'. (%s)", fName, _saveFileMan->popErrorDesc().c_str());
 		return;
 	}
 
@@ -952,7 +952,7 @@ void Control::saveGameToFile(uint8 slot) {
 		outf->writeUint32LE(playerRaw[cnt2]);
 	outf->finalize();
 	if (outf->ioFailed())
-		displayMessage(0, "Couldn't write to file '%s' in directory '%s'. Device full?", fName, _saveFileMan->getSavePath());
+		displayMessage(0, "Couldn't write to file '%s'. Device full? (%s)", fName, _saveFileMan->popErrorDesc().c_str());
 	delete outf;
 }
 
@@ -964,7 +964,7 @@ bool Control::restoreGameFromFile(uint8 slot) {
 	inf = _saveFileMan->openForLoading(fName);
 	if (!inf) {
 		// Display an error message, and do nothing
-		displayMessage(0, "Can't open file '%s' in directory '%s'", fName, _saveFileMan->getSavePath());
+		displayMessage(0, "Can't open file '%s'. (%s)", fName, _saveFileMan->popErrorDesc().c_str());
 		return false;
 	}
 
@@ -988,7 +988,7 @@ bool Control::restoreGameFromFile(uint8 slot) {
 		playerBuf[cnt2] = inf->readUint32LE();
 
 	if (inf->ioFailed()) {
-		displayMessage(0, "Can't read from file '%s' in directory '%s'", fName, _saveFileMan->getSavePath());
+		displayMessage(0, "Can't read from file '%s'. (%s)", fName, _saveFileMan->popErrorDesc().c_str());
 		delete inf;
 		free(_restoreBuf);
 		_restoreBuf = NULL;