From 39fd9b93c3647b9d7b01777ca9bf6ba0720b0c65 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Wed, 8 May 2024 20:37:10 +0200 Subject: [PATCH] Introduce proper error handling for file streams --- src/client/client.cpp | 7 +- src/client/filecache.cpp | 20 ++--- src/client/shader.cpp | 4 +- src/client/shadows/dynamicshadowsrender.cpp | 3 +- src/content/content.cpp | 38 +++------ src/database/database-files.cpp | 17 ++-- src/filesys.cpp | 89 ++++++++++++++------- src/filesys.h | 63 ++++++++++++++- src/gettext.cpp | 19 ++--- src/gui/guiEngine.cpp | 5 +- src/log.cpp | 23 +++--- src/map_settings_manager.cpp | 8 +- src/mapgen/mg_schematic.cpp | 7 +- src/porting.cpp | 17 +++- src/porting.h | 3 + src/script/lua_api/l_areastore.cpp | 2 +- src/server.cpp | 12 +-- src/server/ban.cpp | 3 +- src/settings.cpp | 5 +- src/texture_override.cpp | 3 +- src/unittest/test.cpp | 6 +- src/unittest/test_ban.cpp | 4 +- src/unittest/test_filesys.cpp | 26 ++++++ 23 files changed, 235 insertions(+), 149 deletions(-) diff --git a/src/client/client.cpp b/src/client/client.cpp index ca3f91e78..61140d87a 100644 --- a/src/client/client.cpp +++ b/src/client/client.cpp @@ -278,9 +278,7 @@ void Client::scanModSubfolder(const std::string &mod_name, const std::string &mo << "\" as \"" << vfs_path << "\"." << std::endl; std::string contents; - if (!fs::ReadFile(real_path, contents)) { - errorstream << "Client::scanModSubfolder(): Can't read file \"" - << real_path << "\"." << std::endl; + if (!fs::ReadFile(real_path, contents, true)) { continue; } @@ -1945,8 +1943,7 @@ void Client::makeScreenshot() while (serial < SCREENSHOT_MAX_SERIAL_TRIES) { filename = filename_base + (serial > 0 ? ("_" + itos(serial)) : "") + filename_ext; - std::ifstream tmp(filename.c_str()); - if (!tmp.good()) + if (!fs::PathExists(filename)) break; // File did not apparently exist, we'll go with it serial++; } diff --git a/src/client/filecache.cpp b/src/client/filecache.cpp index cb9f5440b..ffd011866 100644 --- a/src/client/filecache.cpp +++ b/src/client/filecache.cpp @@ -38,13 +38,9 @@ void FileCache::createDir() bool FileCache::loadByPath(const std::string &path, std::ostream &os) { - std::ifstream fis(path.c_str(), std::ios_base::binary); - - if(!fis.good()){ - verbosestream<<"FileCache: File not found in cache: " - <(is)), - std::istreambuf_iterator()); + fs::ReadFile(spec.path + DIR_DELIM + "description.txt", spec.desc); } } diff --git a/src/database/database-files.cpp b/src/database/database-files.cpp index 518c776ea..27f42fc97 100644 --- a/src/database/database-files.cpp +++ b/src/database/database-files.cpp @@ -165,11 +165,9 @@ void PlayerDatabaseFiles::savePlayer(RemotePlayer *player) } // Open and deserialize file to check player name - std::ifstream is(path.c_str(), std::ios_base::binary); - if (!is.good()) { - errorstream << "Failed to open " << path << std::endl; + auto is = open_ifstream(path.c_str(), true); + if (!is.good()) return; - } deSerialize(&testplayer, is, path, NULL); is.close(); @@ -205,7 +203,7 @@ bool PlayerDatabaseFiles::removePlayer(const std::string &name) RemotePlayer temp_player("", NULL); for (u32 i = 0; i < PLAYER_FILE_ALTERNATE_TRIES; i++) { // Open file and deserialize - std::ifstream is(path.c_str(), std::ios_base::binary); + auto is = open_ifstream(path.c_str(), false); if (!is.good()) continue; @@ -231,7 +229,7 @@ bool PlayerDatabaseFiles::loadPlayer(RemotePlayer *player, PlayerSAO *sao) const std::string player_to_load = player->getName(); for (u32 i = 0; i < PLAYER_FILE_ALTERNATE_TRIES; i++) { // Open file and deserialize - std::ifstream is(path.c_str(), std::ios_base::binary); + auto is = open_ifstream(path.c_str(), false); if (!is.good()) continue; @@ -260,7 +258,7 @@ void PlayerDatabaseFiles::listPlayers(std::vector &res) const std::string &filename = it->name; std::string full_path = m_savedir + DIR_DELIM + filename; - std::ifstream is(full_path.c_str(), std::ios_base::binary); + auto is = open_ifstream(full_path.c_str(), true); if (!is.good()) continue; @@ -332,7 +330,7 @@ void AuthDatabaseFiles::reload() bool AuthDatabaseFiles::readAuthFile() { std::string path = m_savedir + DIR_DELIM + "auth.txt"; - std::ifstream file(path, std::ios::binary); + auto file = open_ifstream(path.c_str(), false); if (!file.good()) { return false; } @@ -528,8 +526,7 @@ Json::Value *ModStorageDatabaseFiles::getOrCreateJson(const std::string &modname std::string path = m_storage_dir + DIR_DELIM + modname; if (fs::PathExists(path)) { - std::ifstream is(path.c_str(), std::ios_base::binary); - + auto is = open_ifstream(path.c_str(), true); Json::CharReaderBuilder builder; builder.settings_["collectComments"] = false; std::string errs; diff --git a/src/filesys.cpp b/src/filesys.cpp index f7a03617b..a61f7f0aa 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -41,6 +41,13 @@ with this program; if not, write to the Free Software Foundation, Inc., #endif #endif +// Error from last OS call as string +#ifdef _WIN32 +#define LAST_OS_ERROR() porting::ConvertError(GetLastError()) +#else +#define LAST_OS_ERROR() strerror(errno) +#endif + namespace fs { @@ -849,40 +856,44 @@ const char *GetFilenameFromPath(const char *path) bool safeWriteToFile(const std::string &path, std::string_view content) { - std::string tmp_file = path + ".~mt"; + // Note: this is not safe if two MT processes try this at the same time (FIXME?) + const std::string tmp_file = path + ".~mt"; - // Write to a tmp file - bool tmp_success = false; + // Write data to a temporary file + std::string write_error; #ifdef _WIN32 // We've observed behavior suggesting that the MSVC implementation of std::ofstream::flush doesn't // actually flush, so we use win32 APIs. - HANDLE tmp_handle = CreateFile( - tmp_file.c_str(), GENERIC_WRITE, 0, nullptr, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); - if (tmp_handle == INVALID_HANDLE_VALUE) { + HANDLE handle = CreateFile(tmp_file.c_str(), GENERIC_WRITE, 0, nullptr, + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr); + if (handle == INVALID_HANDLE_VALUE) { + errorstream << "Failed to open file: " << LAST_OS_ERROR() << std::endl; return false; } DWORD bytes_written; - tmp_success = (WriteFile(tmp_handle, content.data(), content.size(), &bytes_written, nullptr) && - FlushFileBuffers(tmp_handle)); - CloseHandle(tmp_handle); + if (!WriteFile(handle, content.data(), content.size(), &bytes_written, nullptr)) + write_error = LAST_OS_ERROR(); + else if (!FlushFileBuffers(handle)) + write_error = LAST_OS_ERROR(); + CloseHandle(handle); #else - std::ofstream os(tmp_file.c_str(), std::ios::binary); - if (!os.good()) { + auto os = open_ofstream(tmp_file.c_str(), true); + if (!os.good()) return false; - } - os << content; - os.flush(); + os << content << std::flush; os.close(); - tmp_success = !os.fail(); + if (os.fail()) + write_error = "iostream fail"; #endif - if (!tmp_success) { + if (!write_error.empty()) { + errorstream << "Failed to write file: " << write_error << std::endl; remove(tmp_file.c_str()); return false; } - bool rename_success = false; + std::string rename_error; // Move the finished temporary file over the real file #ifdef _WIN32 @@ -890,22 +901,25 @@ bool safeWriteToFile(const std::string &path, std::string_view content) // to query the file. This can make the move file call below fail. // We retry up to 5 times, with a 1ms sleep between, before we consider the whole operation failed for (int attempt = 0; attempt < 5; attempt++) { - rename_success = MoveFileEx(tmp_file.c_str(), path.c_str(), + auto ok = MoveFileEx(tmp_file.c_str(), path.c_str(), MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH); - if (rename_success) + if (ok) { + rename_error.clear(); break; + } + rename_error = LAST_OS_ERROR(); sleep_ms(1); } #else // On POSIX compliant systems rename() is specified to be able to swap the // file in place of the destination file, making this a truly error-proof // transaction. - rename_success = rename(tmp_file.c_str(), path.c_str()) == 0; + if (rename(tmp_file.c_str(), path.c_str()) != 0) + rename_error = LAST_OS_ERROR(); #endif - if (!rename_success) { - warningstream << "Failed to write to file: " << path.c_str() << std::endl; - // Remove the temporary file because moving it over the target file - // failed. + + if (!rename_error.empty()) { + errorstream << "Failed to overwrite \"" << path << "\": " << rename_error << std::endl; remove(tmp_file.c_str()); return false; } @@ -949,7 +963,7 @@ bool extractZipFile(io::IFileSystem *fs, const char *filename, const std::string irr_ptr toread(opened_zip->createAndOpenFile(i)); - std::ofstream os(fullpath.c_str(), std::ios::binary); + auto os = open_ofstream(fullpath.c_str(), true); if (!os.good()) return false; @@ -976,12 +990,11 @@ bool extractZipFile(io::IFileSystem *fs, const char *filename, const std::string } #endif -bool ReadFile(const std::string &path, std::string &out) +bool ReadFile(const std::string &path, std::string &out, bool log_error) { - std::ifstream is(path, std::ios::binary | std::ios::ate); - if (!is.good()) { + auto is = open_ifstream(path.c_str(), log_error, std::ios::ate); + if (!is.good()) return false; - } auto size = is.tellg(); out.resize(size); @@ -996,4 +1009,22 @@ bool Rename(const std::string &from, const std::string &to) return rename(from.c_str(), to.c_str()) == 0; } +bool OpenStream(std::filebuf &stream, const char *filename, + std::ios::openmode mode, bool log_error, bool log_warn) +{ + assert((mode & std::ios::in) || (mode & std::ios::out)); + assert(!stream.is_open()); + // C++ dropped the ball hard for file opening error handling, there's not even + // an implementation-defined mechanism for returning any kind of error code or message. + if (!stream.open(filename, mode)) { + if (log_warn || log_error) { + std::string msg = LAST_OS_ERROR(); + (log_error ? errorstream : warningstream) + << "Failed to open \"" << filename << "\": " << msg << std::endl; + } + return false; + } + return true; +} + } // namespace fs diff --git a/src/filesys.h b/src/filesys.h index 3edc3eef6..bb71f22e5 100644 --- a/src/filesys.h +++ b/src/filesys.h @@ -23,6 +23,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include #include +#include #ifdef _WIN32 #define DIR_DELIM "\\" @@ -148,14 +149,74 @@ std::string AbsolutePath(const std::string &path); // delimiter is found. const char *GetFilenameFromPath(const char *path); +// Replace the content of a file on disk in a way that is safe from +// corruption/truncation on a crash. +// logs and returns false on error bool safeWriteToFile(const std::string &path, std::string_view content); #ifndef SERVER bool extractZipFile(irr::io::IFileSystem *fs, const char *filename, const std::string &destination); #endif -bool ReadFile(const std::string &path, std::string &out); +bool ReadFile(const std::string &path, std::string &out, bool log_error = false); bool Rename(const std::string &from, const std::string &to); +/** + * Open a file buffer with error handling, commonly used with `std::fstream.rdbuf()`. + * + * @param stream stream references, must not already be open + * @param filename filename to open + * @param mode mode bits (used as-is) + * @param log_error log failure to errorstream? + * @param log_warn log failure to warningstream? + * @return true if success +*/ +bool OpenStream(std::filebuf &stream, const char *filename, + std::ios::openmode mode, bool log_error, bool log_warn); + } // namespace fs + +// outside of namespace for convenience: + +/** + * Helper function to open an output file stream (= writing). + * + * For compatibility with fopen() binary mode and truncate are always set. + * Use `fs::OpenStream` for more control. + * @param name file name + * @param log if true, failure to open the file will be logged to errorstream + * @param mode additional mode bits (e.g. std::ios::app) + * @return file stream, will be !good in case of error +*/ +inline std::ofstream open_ofstream(const char *name, bool log, + std::ios::openmode mode = std::ios::openmode()) +{ + mode |= std::ios::out | std::ios::binary; + if (!(mode & std::ios::app)) + mode |= std::ios::trunc; + std::ofstream ofs; + if (!fs::OpenStream(*ofs.rdbuf(), name, mode, log, false)) + ofs.setstate(std::ios::failbit); + return ofs; +} + +/** + * Helper function to open an input file stream (= reading). + * + * For compatibility with fopen() binary mode is always set. + * Use `fs::OpenStream` for more control. + * @param name file name + * @param log if true, failure to open the file will be logged to errorstream + * @param mode additional mode bits (e.g. std::ios::ate) + * @return file stream, will be !good in case of error +*/ +inline std::ifstream open_ifstream(const char *name, bool log, + std::ios::openmode mode = std::ios::openmode()) +{ + mode |= std::ios::in | std::ios::binary; + std::ifstream ifs; + if (!fs::OpenStream(*ifs.rdbuf(), name, mode, log, false)) + ifs.setstate(std::ios::failbit); + return ifs; +} diff --git a/src/gettext.cpp b/src/gettext.cpp index 68b6f728a..58efebb7c 100644 --- a/src/gettext.cpp +++ b/src/gettext.cpp @@ -147,18 +147,15 @@ static void MSVC_LocaleWorkaround(int argc, char* argv[]) exit(0); // NOTREACHED } else { - char buffer[1024]; + auto e = GetLastError(); - FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), - MAKELANGID(LANG_NEUTRAL,SUBLANG_DEFAULT), buffer, - sizeof(buffer) - 1, NULL); - - errorstream << "*******************************************************" << std::endl; - errorstream << "CMD: " << app_name << std::endl; - errorstream << "Failed to restart with current locale: " << std::endl; - errorstream << buffer; - errorstream << "Expect language to be broken!" << std::endl; - errorstream << "*******************************************************" << std::endl; + errorstream + << "*******************************************************" << '\n' + << "CMD: " << app_name << '\n'; + << "Failed to restart with current locale: "; + << porting::ConvertError(e) << '\n'; + << "Expect language to be broken!" << '\n'; + << "*******************************************************" << std::endl; } } diff --git a/src/gui/guiEngine.cpp b/src/gui/guiEngine.cpp index 4d35f38ab..66f9f9864 100644 --- a/src/gui/guiEngine.cpp +++ b/src/gui/guiEngine.cpp @@ -620,10 +620,9 @@ bool GUIEngine::setTexture(texture_layer layer, const std::string &texturepath, bool GUIEngine::downloadFile(const std::string &url, const std::string &target) { #if USE_CURL - std::ofstream target_file(target.c_str(), std::ios::out | std::ios::binary); - if (!target_file.good()) { + auto target_file = open_ofstream(target.c_str(), true); + if (!target_file.good()) return false; - } HTTPFetchRequest fetch_request; HTTPFetchResult fetch_result; diff --git a/src/log.cpp b/src/log.cpp index 8ab0ca4ba..5ee66c070 100644 --- a/src/log.cpp +++ b/src/log.cpp @@ -28,6 +28,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "exceptions.h" #include "util/numeric.h" #include "log.h" +#include "filesys.h" #ifdef __ANDROID__ #include @@ -36,7 +37,6 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include #include -#include #include class LevelTarget : public LogTarget { @@ -299,25 +299,24 @@ void FileLogOutput::setFile(const std::string &filename, s64 file_size_max) bool is_too_large = false; if (file_size_max > 0) { std::ifstream ifile(filename, std::ios::binary | std::ios::ate); - is_too_large = ifile.tellg() > file_size_max; - ifile.close(); + if (ifile.good()) + is_too_large = ifile.tellg() > file_size_max; } - if (is_too_large) { std::string filename_secondary = filename + ".1"; actionstream << "The log file grew too big; it is moved to " << filename_secondary << std::endl; - remove(filename_secondary.c_str()); - rename(filename.c_str(), filename_secondary.c_str()); + fs::DeleteSingleFileOrEmptyDirectory(filename_secondary); + fs::Rename(filename, filename_secondary); } - m_stream.open(filename, std::ios::app | std::ios::ate); - if (!m_stream.good()) - throw FileNotGoodException("Failed to open log file " + - filename + ": " + strerror(errno)); + // Intentionally not using open_ofstream() to keep the text mode + if (!fs::OpenStream(*m_stream.rdbuf(), filename.c_str(), std::ios::out | std::ios::app, true, false)) + throw FileNotGoodException("Failed to open log file"); + m_stream << "\n\n" - "-------------" << std::endl << - " Separator" << std::endl << + "-------------\n" << + " Separator\n" << "-------------\n" << std::endl; } diff --git a/src/map_settings_manager.cpp b/src/map_settings_manager.cpp index 264a941c2..bef7ad1c6 100644 --- a/src/map_settings_manager.cpp +++ b/src/map_settings_manager.cpp @@ -96,13 +96,9 @@ bool MapSettingsManager::setMapSettingNoiseParams( bool MapSettingsManager::loadMapMeta() { - std::ifstream is(m_map_meta_path.c_str(), std::ios_base::binary); - - if (!is.good()) { - errorstream << "loadMapMeta: could not open " - << m_map_meta_path << std::endl; + auto is = open_ifstream(m_map_meta_path.c_str(), true); + if (!is.good()) return false; - } if (!m_map_settings->parseConfigLines(is)) { errorstream << "loadMapMeta: Format error. '[end_of_params]' missing?" << std::endl; diff --git a/src/mapgen/mg_schematic.cpp b/src/mapgen/mg_schematic.cpp index b6b78f9e9..5ce0b8844 100644 --- a/src/mapgen/mg_schematic.cpp +++ b/src/mapgen/mg_schematic.cpp @@ -485,12 +485,9 @@ bool Schematic::serializeToLua(std::ostream *os, bool use_comments, bool Schematic::loadSchematicFromFile(const std::string &filename, const NodeDefManager *ndef, StringMap *replace_names) { - std::ifstream is(filename.c_str(), std::ios_base::binary); - if (!is.good()) { - errorstream << __FUNCTION__ << ": unable to open file '" - << filename << "'" << std::endl; + auto is = open_ifstream(filename.c_str(), true); + if (!is.good()) return false; - } if (!m_ndef) m_ndef = ndef; diff --git a/src/porting.cpp b/src/porting.cpp index db769d901..2b2405d38 100644 --- a/src/porting.cpp +++ b/src/porting.cpp @@ -582,7 +582,7 @@ static void createCacheDirTag() if (fs::PathExists(path)) return; fs::CreateAllDirs(path_cache); - std::ofstream ofs(path, std::ios::out | std::ios::binary); + auto ofs = open_ofstream(path.c_str(), false); if (!ofs.good()) return; ofs << "Signature: 8a477f597d28d172789f06886806bc55\n" @@ -800,6 +800,21 @@ std::string QuoteArgv(const std::string &arg) ret.push_back('"'); return ret; } + +std::string ConvertError(DWORD error_code) +{ + wchar_t buffer[320]; + + auto r = FormatMessageW( + FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, + nullptr, error_code, 0, buffer, ARRLEN(buffer) - 1, nullptr); + if (!r) + return std::to_string(error_code); + + if (!buffer[0]) // should not happen normally + return "?"; + return wide_to_utf8(buffer); +} #endif int mt_snprintf(char *buf, const size_t buf_size, const char *fmt, ...) diff --git a/src/porting.h b/src/porting.h index 0f97a1ca3..a98e66bcf 100644 --- a/src/porting.h +++ b/src/porting.h @@ -293,6 +293,9 @@ void attachOrCreateConsole(); #ifdef _WIN32 // Quotes an argument for use in a CreateProcess() commandline (not cmd.exe!!) std::string QuoteArgv(const std::string &arg); + +// Convert an error code (e.g. from GetLastError()) into a string. +std::string ConvertError(DWORD error_code); #endif int mt_snprintf(char *buf, const size_t buf_size, const char *fmt, ...); diff --git a/src/script/lua_api/l_areastore.cpp b/src/script/lua_api/l_areastore.cpp index 82fb945ae..c261338a4 100644 --- a/src/script/lua_api/l_areastore.cpp +++ b/src/script/lua_api/l_areastore.cpp @@ -298,7 +298,7 @@ int LuaAreaStore::l_from_file(lua_State *L) const char *filename = luaL_checkstring(L, 2); CHECK_SECURE_PATH(L, filename, false); - std::ifstream is(filename, std::ios::binary); + auto is = open_ifstream(filename, true); return deserialization_helper(L, o->as, is); } diff --git a/src/server.cpp b/src/server.cpp index 6e12017bc..316f349b2 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2545,9 +2545,7 @@ bool Server::addMediaFile(const std::string &filename, // Read data std::string filedata; - if (!fs::ReadFile(filepath, filedata)) { - errorstream << "Server::addMediaFile(): Failed to open \"" - << filename << "\" for reading" << std::endl; + if (!fs::ReadFile(filepath, filedata, true)) { return false; } @@ -2715,9 +2713,7 @@ void Server::sendRequestedMedia(session_t peer_id, // Read data std::string data; - if (!fs::ReadFile(m.path, data)) { - errorstream << "Server::sendRequestedMedia(): Failed to read \"" - << name << "\"" << std::endl; + if (!fs::ReadFile(m.path, data, true)) { continue; } file_size_bunch_total += data.size(); @@ -3618,7 +3614,7 @@ namespace { auto filepath = fs::CreateTempFile(); if (filepath.empty()) return ""; - std::ofstream os(filepath, std::ios::binary); + auto os = open_ofstream(filepath.c_str(), true); if (!os.good()) return ""; os << content; @@ -4200,7 +4196,7 @@ Translations *Server::getTranslationLanguage(const std::string &lang_code) for (const auto &i : m_media) { if (str_ends_with(i.first, suffix)) { std::string data; - if (fs::ReadFile(i.second.path, data)) { + if (fs::ReadFile(i.second.path, data, true)) { translations->loadTranslation(data); } } diff --git a/src/server/ban.cpp b/src/server/ban.cpp index 47a6ccd5e..4af108795 100644 --- a/src/server/ban.cpp +++ b/src/server/ban.cpp @@ -48,9 +48,8 @@ void BanManager::load() { MutexAutoLock lock(m_mutex); infostream<<"BanManager: loading from "< #include @@ -48,7 +49,7 @@ static const std::map override_LUT = { TextureOverrideSource::TextureOverrideSource(const std::string &filepath) { - std::ifstream infile(filepath.c_str()); + auto infile = open_ifstream(filepath.c_str(), false); std::string line; int line_index = 0; while (std::getline(infile, line)) { diff --git a/src/unittest/test.cpp b/src/unittest/test.cpp index e2d19b3e1..7141ee0c6 100644 --- a/src/unittest/test.cpp +++ b/src/unittest/test.cpp @@ -25,6 +25,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "modchannels.h" #include "util/numeric.h" #include "porting.h" +#include "debug.h" content_t t_CONTENT_STONE; content_t t_CONTENT_GRASS; @@ -348,11 +349,14 @@ void TestBase::runTest(const char *name, std::function &&test) rawstream << " at " << e.file << ":" << e.line << std::endl; rawstream << "[FAIL] "; num_tests_failed++; - } catch (std::exception &e) { + } +#if CATCH_UNHANDLED_EXCEPTIONS == 1 + catch (std::exception &e) { rawstream << "Caught unhandled exception: " << e.what() << std::endl; rawstream << "[FAIL] "; num_tests_failed++; } +#endif num_tests_run++; u64 tdiff = porting::getTimeMs() - t1; rawstream << name << " - " << tdiff << "ms" << std::endl; diff --git a/src/unittest/test_ban.cpp b/src/unittest/test_ban.cpp index 6b36211ab..58132e20a 100644 --- a/src/unittest/test_ban.cpp +++ b/src/unittest/test_ban.cpp @@ -80,13 +80,13 @@ void TestBan::testCreate() BanManager bm(m_testbm); } - UASSERT(std::ifstream(m_testbm, std::ios::binary).is_open()); + UASSERT(fs::IsFile(m_testbm)); // test manual save { BanManager bm(m_testbm2); bm.save(); - UASSERT(std::ifstream(m_testbm2, std::ios::binary).is_open()); + UASSERT(fs::IsFile(m_testbm2)); } } diff --git a/src/unittest/test_filesys.cpp b/src/unittest/test_filesys.cpp index 302371361..8fc9db3d8 100644 --- a/src/unittest/test_filesys.cpp +++ b/src/unittest/test_filesys.cpp @@ -41,6 +41,7 @@ public: void testRemoveRelativePathComponent(); void testSafeWriteToFile(); void testCopyFileContents(); + void testNonExist(); }; static TestFileSys g_test_instance; @@ -54,6 +55,7 @@ void TestFileSys::runTests(IGameDef *gamedef) TEST(testRemoveRelativePathComponent); TEST(testSafeWriteToFile); TEST(testCopyFileContents); + TEST(testNonExist); } //////////////////////////////////////////////////////////////////////////////// @@ -311,3 +313,27 @@ void TestFileSys::testCopyFileContents() UASSERT(fs::ReadFile(file2, contents_actual)); UASSERTEQ(auto, contents_actual, test_data); } + +void TestFileSys::testNonExist() +{ + const auto path = getTestTempFile(); + fs::DeleteSingleFileOrEmptyDirectory(path); + + UASSERT(!fs::IsFile(path)); + UASSERT(!fs::IsDir(path)); + UASSERT(!fs::IsExecutable(path)); + + std::string s; + UASSERT(!fs::ReadFile(path, s)); + UASSERT(s.empty()); + + UASSERT(!fs::Rename(path, getTestTempFile())); + + std::filebuf buf; + // with logging enabled to test that code path + UASSERT(!fs::OpenStream(buf, path.c_str(), std::ios::in, false, true)); + UASSERT(!buf.is_open()); + + auto ifs = open_ifstream(path.c_str(), false); + UASSERT(!ifs.good()); +}