From f92abc863eb10db6e285ba42555533983f9f755b Mon Sep 17 00:00:00 2001 From: wangqr Date: Thu, 17 Oct 2019 03:13:55 -0400 Subject: [PATCH] Remove exception in destructor of agi::io::Save Provide Close() for error handling Correctly parse boost error code Handle failure in TextFileWriter Fix wangqr/Aegisub#25 --- libaegisub/common/fs.cpp | 21 +++++++++++++++++++++ libaegisub/common/io.cpp | 10 +++++++++- libaegisub/include/libaegisub/io.h | 3 ++- src/text_file_writer.cpp | 12 +++++++++++- 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/libaegisub/common/fs.cpp b/libaegisub/common/fs.cpp index 6ce45a08e..b472d9cd1 100644 --- a/libaegisub/common/fs.cpp +++ b/libaegisub/common/fs.cpp @@ -30,6 +30,26 @@ namespace ec = boost::system::errc; // boost::filesystem functions throw a single exception type for all // errors, which isn't really what we want, so do some crazy wrapper // shit to map error codes to more useful exceptions. +#ifdef BOOST_WINDOWS_API +#define CHECKED_CALL(exp, src_path, dst_path) \ + boost::system::error_code ec; \ + exp; \ + switch (ec.value()) {\ + case ERROR_SUCCESS: break; \ + case ERROR_FILE_NOT_FOUND: throw FileNotFound(src_path); \ + case ERROR_DIRECTORY: throw NotADirectory(src_path); \ + case ERROR_DISK_FULL: throw DriveFull(dst_path); \ + case ERROR_ACCESS_DENIED: \ + if (!src_path.empty()) \ + acs::CheckFileRead(src_path); \ + if (!dst_path.empty()) \ + acs::CheckFileWrite(dst_path); \ + throw AccessDenied(src_path); \ + default: \ + LOG_D("filesystem") << "Unknown error when calling '" << #exp << "': " << ec << ": " << ec.message(); \ + throw FileSystemUnknownError(ec.message()); \ + } +#else #define CHECKED_CALL(exp, src_path, dst_path) \ boost::system::error_code ec; \ exp; \ @@ -49,6 +69,7 @@ namespace ec = boost::system::errc; LOG_D("filesystem") << "Unknown error when calling '" << #exp << "': " << ec << ": " << ec.message(); \ throw FileSystemUnknownError(ec.message()); \ } +#endif #define CHECKED_CALL_RETURN(exp, src_path) \ CHECKED_CALL(auto ret = exp, src_path, agi::fs::path()); \ diff --git a/libaegisub/common/io.cpp b/libaegisub/common/io.cpp index e133a71ab..f031d6161 100644 --- a/libaegisub/common/io.cpp +++ b/libaegisub/common/io.cpp @@ -56,7 +56,8 @@ Save::Save(fs::path const& file, bool binary) } } -Save::~Save() noexcept(false) { +void Save::Close() { + if (!fp) return; fp.reset(); // Need to close before rename on Windows to unlock the file for (int i = 0; i < 10; ++i) { try { @@ -72,5 +73,12 @@ Save::~Save() noexcept(false) { } } +Save::~Save() { + try { + Close(); + } + catch (agi::fs::FileSystemError const&) {} +} + } // namespace io } // namespace agi diff --git a/libaegisub/include/libaegisub/io.h b/libaegisub/include/libaegisub/io.h index 5ab4f4af3..f29e0cb6c 100644 --- a/libaegisub/include/libaegisub/io.h +++ b/libaegisub/include/libaegisub/io.h @@ -38,8 +38,9 @@ class Save { public: Save(fs::path const& file, bool binary = false); - ~Save() noexcept(false); + ~Save(); std::ostream& Get() { return *fp; } + void Close(); }; } // namespace io diff --git a/src/text_file_writer.cpp b/src/text_file_writer.cpp index c4de4c077..7458b6849 100644 --- a/src/text_file_writer.cpp +++ b/src/text_file_writer.cpp @@ -24,6 +24,7 @@ #include "options.h" #include +#include #include #include @@ -49,7 +50,16 @@ TextFileWriter::TextFileWriter(agi::fs::path const& filename, std::string encodi } TextFileWriter::~TextFileWriter() { - // Explicit empty destructor required with a unique_ptr to an incomplete class + try { + file->Close(); + } + catch (agi::fs::FileSystemError const&e) { + wxString m = wxString::FromUTF8(e.GetMessage()); + if (!m.empty()) + wxMessageBox(m, "Exception in agi::io::Save", wxOK | wxCENTRE | wxICON_ERROR); + else + wxMessageBox(e.GetMessage(), "Exception in agi::io::Save", wxOK | wxCENTRE | wxICON_ERROR); + } } void TextFileWriter::WriteLineToFile(std::string const& line, bool addLineBreak) {