4 files changed, 74 insertions, 69 deletions

diff --git a/backends/saves/default/default-saves.cpp b/backends/saves/default/default-saves.cpp
index 9a0ff484ce..c15463e7bb 100644
--- a/backends/saves/default/default-saves.cpp
+++ b/backends/saves/default/default-saves.cpp
@@ -44,8 +44,21 @@ DefaultSaveFileManager::DefaultSaveFileManager(const Common::String &defaultSave
 }
 
 
+void DefaultSaveFileManager::checkPath(const Common::FSNode &dir) {
+	clearError();
+	if (!dir.exists()) {
+		setError(Common::kPathDoesNotExist, "The savepath '"+dir.getPath()+"' does not exist");
+	} else if (!dir.isDirectory()) {
+		setError(Common::kPathNotDirectory, "The savepath '"+dir.getPath()+"' is not a directory");
+	}
+}
+
 Common::StringList DefaultSaveFileManager::listSavefiles(const char *pattern) {
 	Common::FSNode savePath(getSavePath());
+	checkPath(savePath);
+	if (getError() != Common::kNoError)
+		return Common::StringList();
+
 	Common::FSList savefiles;
 	Common::StringList results;
 	Common::String search(pattern);
@@ -59,63 +72,56 @@ Common::StringList DefaultSaveFileManager::listSavefiles(const char *pattern) {
 	return results;
 }
 
-void DefaultSaveFileManager::checkPath(const Common::FSNode &dir) {
-	clearError();
-	if (!dir.exists()) {
-		setError(SFM_DIR_NOENT, "A component of the path does not exist, or the path is an empty string: "+dir.getPath());
-	} else if (!dir.isDirectory()) {
-		setError(SFM_DIR_NOTDIR, "The given savepath is not a directory: "+dir.getPath());
-	}
-}
-
 Common::InSaveFile *DefaultSaveFileManager::openForLoading(const char *filename) {
 	// Ensure that the savepath is valid. If not, generate an appropriate error.
 	Common::FSNode savePath(getSavePath());
 	checkPath(savePath);
+	if (getError() != Common::kNoError)
+		return 0;
 
-	if (getError() == SFM_NO_ERROR) {
-		Common::FSNode file = savePath.getChild(filename);
+	Common::FSNode file = savePath.getChild(filename);
 
-		// Open the file for reading
-		Common::SeekableReadStream *sf = file.openForReading();
+	// Open the file for reading
+	Common::SeekableReadStream *sf = file.openForReading();
 
-		return wrapInSaveFile(sf);
-	} else {
-		return 0;
-	}
+	return wrapInSaveFile(sf);
 }
 
 Common::OutSaveFile *DefaultSaveFileManager::openForSaving(const char *filename) {
 	// Ensure that the savepath is valid. If not, generate an appropriate error.
 	Common::FSNode savePath(getSavePath());
 	checkPath(savePath);
+	if (getError() != Common::kNoError)
+		return 0;
 
-	if (getError() == SFM_NO_ERROR) {
-		Common::FSNode file = savePath.getChild(filename);
+	Common::FSNode file = savePath.getChild(filename);
 
-		// Open the file for saving
-		Common::WriteStream *sf = file.openForWriting();
+	// Open the file for saving
+	Common::WriteStream *sf = file.openForWriting();
 
-		return wrapOutSaveFile(sf);
-	} else {
-		return 0;
-	}
+	return wrapOutSaveFile(sf);
 }
 
 bool DefaultSaveFileManager::removeSavefile(const char *filename) {
 	clearError();
 
 	Common::FSNode savePath(getSavePath());
-	Common::FSNode file = savePath.getChild(filename);
+	checkPath(savePath);
+	if (getError() != Common::kNoError)
+		return false;
 
-	// TODO: Add new method FSNode::remove()
+	Common::FSNode file = savePath.getChild(filename);
+	
+	// FIXME: remove does not exist on all systems. If your port fails to
+	// compile because of this, please let us know (scummvm-devel or Fingolfin).
+	// There is a nicely portable workaround, too: Make this method overloadable.
 	if (remove(file.getPath().c_str()) != 0) {
 #ifndef _WIN32_WCE
 		if (errno == EACCES)
-			setError(SFM_DIR_ACCESS, "Search or write permission denied: "+file.getName());
+			setError(Common::kWritePermissionDenied, "Search or write permission denied: "+file.getName());
 
 		if (errno == ENOENT)
-			setError(SFM_DIR_NOENT, "A component of the path does not exist, or the path is an empty string: "+file.getName());
+			setError(Common::kPathDoesNotExist, "removeSavefile: '"+file.getName()+"' does not exist or path is invalid");
 #endif
 		return false;
 	} else {
diff --git a/backends/saves/posix/posix-saves.cpp b/backends/saves/posix/posix-saves.cpp
index 38d811789f..2466f3d8d9 100644
--- a/backends/saves/posix/posix-saves.cpp
+++ b/backends/saves/posix/posix-saves.cpp
@@ -72,13 +72,13 @@ void POSIXSaveFileManager::checkPath(const Common::FSNode &dir) {
 		// to create the dir (ENOENT case).
 		switch (errno) {
 		case EACCES:
-			setError(SFM_DIR_ACCESS, "Search or write permission denied: "+path);
+			setError(Common::kWritePermissionDenied, "Search or write permission denied: "+path);
 			break;
 		case ELOOP:
-			setError(SFM_DIR_LOOP, "Too many symbolic links encountered while traversing the path: "+path);
+			setError(Common::kUnknownError, "Too many symbolic links encountered while traversing the path: "+path);
 			break;
 		case ENAMETOOLONG:
-			setError(SFM_DIR_NAMETOOLONG, "The path name is too long: "+path);
+			setError(Common::kUnknownError, "The path name is too long: "+path);
 			break;
 		case ENOENT:
 			if (mkdir(path.c_str(), 0755) != 0) {
@@ -89,37 +89,37 @@ void POSIXSaveFileManager::checkPath(const Common::FSNode &dir) {
 
 				switch (errno) {
 				case EACCES:
-					setError(SFM_DIR_ACCESS, "Search or write permission denied: "+path);
+					setError(Common::kWritePermissionDenied, "Search or write permission denied: "+path);
 					break;
 				case EMLINK:
-					setError(SFM_DIR_LINKMAX, "The link count of the parent directory would exceed {LINK_MAX}: "+path);
+					setError(Common::kUnknownError, "The link count of the parent directory would exceed {LINK_MAX}: "+path);
 					break;
 				case ELOOP:
-					setError(SFM_DIR_LOOP, "Too many symbolic links encountered while traversing the path: "+path);
+					setError(Common::kUnknownError, "Too many symbolic links encountered while traversing the path: "+path);
 					break;
 				case ENAMETOOLONG:
-					setError(SFM_DIR_NAMETOOLONG, "The path name is too long: "+path);
+					setError(Common::kUnknownError, "The path name is too long: "+path);
 					break;
 				case ENOENT:
-					setError(SFM_DIR_NOENT, "A component of the path does not exist, or the path is an empty string: "+path);
+					setError(Common::kPathDoesNotExist, "A component of the path does not exist, or the path is an empty string: "+path);
 					break;
 				case ENOTDIR:
-					setError(SFM_DIR_NOTDIR, "A component of the path prefix is not a directory: "+path);
+					setError(Common::kPathDoesNotExist, "A component of the path prefix is not a directory: "+path);
 					break;
 				case EROFS:
-					setError(SFM_DIR_ROFS, "The parent directory resides on a read-only file system:"+path);
+					setError(Common::kWritePermissionDenied, "The parent directory resides on a read-only file system:"+path);
 					break;
 				}
 			}
 			break;
 		case ENOTDIR:
-			setError(SFM_DIR_NOTDIR, "A component of the path prefix is not a directory: "+path);
+			setError(Common::kPathDoesNotExist, "A component of the path prefix is not a directory: "+path);
 			break;
 		}
 	} else {
 		// So stat() succeeded. But is the path actually pointing to a directory?
 		if (!S_ISDIR(sb.st_mode)) {
-			setError(SFM_DIR_NOTDIR, "The given savepath is not a directory: "+path);
+			setError(Common::kPathDoesNotExist, "The given savepath is not a directory: "+path);
 		}
 	}
 }
diff --git a/common/error.h b/common/error.h
index 39d76a8c36..1f5fed7ea6 100644
--- a/common/error.h
+++ b/common/error.h
@@ -26,42 +26,41 @@
 #ifndef COMMON_ERROR_H
 #define COMMON_ERROR_H
 
-/**
- * This file contains enums with error codes commonly used.
- */
+namespace Common {
 
 /**
- * Errors used in the SaveFileManager class.
- *
- * @todo	Merge this partially into Common::Error. We only need a small subset of these errors, though.
+ * This file contains an enum with commonly used error codes.
  */
-enum SFMError {
-	SFM_NO_ERROR,			//Default state, indicates no error has been recorded
-	SFM_DIR_ACCESS,			//stat(), mkdir()::EACCES: Search or write permission denied
-	SFM_DIR_LINKMAX,		//mkdir()::EMLINK: The link count of the parent directory would exceed {LINK_MAX}
-	SFM_DIR_LOOP,			//stat(), mkdir()::ELOOP: Too many symbolic links encountered while traversing the path
-	SFM_DIR_NAMETOOLONG,	//stat(), mkdir()::ENAMETOOLONG: The path name is too long
-	SFM_DIR_NOENT,			//stat(), mkdir()::ENOENT: A component of the path path does not exist, or the path is an empty string
-	SFM_DIR_NOTDIR,			//stat(), mkdir()::ENOTDIR: A component of the path prefix is not a directory
-	SFM_DIR_ROFS			//mkdir()::EROFS: The parent directory resides on a read-only file system
-};
 
-namespace Common {
+
 
 /**
  * Error codes which may be reported by plugins under various circumstances.
  *
- * @todo Clarify the names, and add doxygen comments to each error.
- * @todo Add more error values, e.g. for load/save errors. Use those in SaveFileManager,
- *       (Meta)Engine save/load API, Engine::init() and Engine::go(), ...
- * @todo Maybe add an API which keeps track of an error message,
- *       similiar to SDL_SetError/SDL_GetError/SDL_ClearError?
+ * @todo Clarify the names; add more codes, resp. verify all existing ones are acutally useful.
+ *       Also, try to avoid overlap.
+ * @todo Maybe introduce a naming convention? E.g. k-NOUN/ACTION-CONDITION-Error, so
+ *       kPathInvalidError would be correct, but these would not be: kInvalidPath,
+ *       kPathInvalid, kPathIsInvalid, kInvalidPathError
  */
 enum Error {
 	kNoError = 0,				//!< No error occured
 	kInvalidPathError,			//!< Engine initialization: Invalid game path was passed
 	kNoGameDataFoundError,		//!< Engine initialization: No game data was found in the specified location
 	kUnsupportedGameidError,	//!< Engine initialization: Gameid not supported by this (Meta)Engine
+	
+	
+	kReadPermissionDenied,		//!< Unable to read data due to missing read permission
+	kWritePermissionDenied,		//!< Unable to write data due to missing write permission
+	
+	// The following three overlap a bit with kInvalidPathError and each other. Which to keep?
+	kPathDoesNotExist,			//!< The specified path does not exist
+	kPathNotDirectory,			//!< The specified path does not point to a directory
+	kPathNotFile,				//!< The specified path does not point to a file
+
+	kCreatingFileFailed,
+	kReadingFailed,				//!< Failed creating a (savestate) file
+	kWritingFailed,				//!< Failure to write data -- disk full?
 
 	kUnknownError				//!< Catch-all error, used if no other error code matches
 };
diff --git a/common/savefile.h b/common/savefile.h
index fa446d7726..776f1fa159 100644
--- a/common/savefile.h
+++ b/common/savefile.h
@@ -65,7 +65,7 @@ typedef WriteStream OutSaveFile;
 class SaveFileManager : NonCopyable {
 
 protected:
-	SFMError _error;
+	Error _error;
 	String _errorDesc;
 
 	/**
@@ -73,7 +73,7 @@ protected:
 	 * @param error Code identifying the last error.
 	 * @param errorDesc String describing the last error.
 	 */
-	virtual void setError(SFMError error, const String &errorDesc) { _error = error; _errorDesc = errorDesc; }
+	virtual void setError(Error error, const String &errorDesc) { _error = error; _errorDesc = errorDesc; }
 
 public:
 	virtual ~SaveFileManager() {}
@@ -81,14 +81,14 @@ public:
 	/**
 	 * Clears the last set error code and string.
 	 */
-	virtual void clearError() { _error = SFM_NO_ERROR; _errorDesc = ""; }
+	virtual void clearError() { _error = kNoError; _errorDesc.clear(); }
 
 	/**
-	 * Returns the last occurred error code. If none occurred, returns SFM_NO_ERROR.
+	 * Returns the last occurred error code. If none occurred, returns kNoError.
 	 *
-	 * @return A SFMError indicating the type of the last error.
+	 * @return A value indicating the type of the last error.
 	 */
-	virtual SFMError getError() { return _error; }
+	virtual Error getError() { return _error; }
 
 	/**
 	 * Returns the last occurred error description. If none occurred, returns 0.