diff options
-rw-r--r-- | backends/saves/default/default-saves.cpp | 64 | ||||
-rw-r--r-- | backends/saves/posix/posix-saves.cpp | 24 | ||||
-rw-r--r-- | common/error.h | 43 | ||||
-rw-r--r-- | common/savefile.h | 12 |
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. |