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
Ristellise · Oct 17, 2019 · d1d2ac1 · d1d2ac1
1 parent 494fe23
commit d1d2ac1
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -24,6 +24,7 @@
 #include "options.h"
 
 #include <libaegisub/io.h>
+#include <libaegisub/fs.h>
 #include <libaegisub/charset_conv.h>
 #include <libaegisub/make_unique.h>
 
@@ -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) {