From 24b2198668ad5d0b3f965bc34762bfac4004bcf2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Tue, 15 Dec 2015 22:26:26 +0100 Subject: [PATCH] fs/io/FileOutputStream: use C++ exceptions in constructor --- src/PlaylistFile.cxx | 12 ++----- src/PlaylistSave.cxx | 7 +--- src/StateFile.cxx | 16 +++++++--- .../plugins/simple/SimpleDatabasePlugin.cxx | 4 +-- src/db/update/Service.cxx | 10 ++++-- src/fs/io/FileOutputStream.cxx | 32 +++++++------------ src/fs/io/FileOutputStream.hxx | 8 ++--- src/output/plugins/RecorderOutputPlugin.cxx | 17 ++++++---- test/WriteFile.cxx | 30 +++++++++-------- 9 files changed, 64 insertions(+), 72 deletions(-) diff --git a/src/PlaylistFile.cxx b/src/PlaylistFile.cxx index d8c15841b..259c26974 100644 --- a/src/PlaylistFile.cxx +++ b/src/PlaylistFile.cxx @@ -232,11 +232,7 @@ SavePlaylistFile(const PlaylistFileContents &contents, const char *utf8path, if (path_fs.IsNull()) return false; - FileOutputStream fos(path_fs, error); - if (!fos.IsDefined()) { - TranslatePlaylistError(error); - return false; - } + FileOutputStream fos(path_fs); BufferedOutputStream bos(fos); @@ -403,11 +399,7 @@ spl_append_song(const char *utf8path, const DetachedSong &song, Error &error) if (path_fs.IsNull()) return false; - AppendFileOutputStream fos(path_fs, error); - if (!fos.IsDefined()) { - TranslatePlaylistError(error); - return false; - } + AppendFileOutputStream fos(path_fs); if (fos.Tell() / (MPD_PATH_MAX + 1) >= playlist_max_length) { error.Set(playlist_domain, int(PlaylistResult::TOO_LARGE), diff --git a/src/PlaylistSave.cxx b/src/PlaylistSave.cxx index a2f48ba54..8d822ce8f 100644 --- a/src/PlaylistSave.cxx +++ b/src/PlaylistSave.cxx @@ -80,12 +80,7 @@ spl_save_queue(const char *name_utf8, const Queue &queue, Error &error) return false; } - FileOutputStream fos(path_fs, error); - if (!fos.IsDefined()) { - TranslatePlaylistError(error); - return false; - } - + FileOutputStream fos(path_fs); BufferedOutputStream bos(fos); for (unsigned i = 0; i < queue.GetLength(); i++) diff --git a/src/StateFile.cxx b/src/StateFile.cxx index ebf6cf576..8d17a1827 100644 --- a/src/StateFile.cxx +++ b/src/StateFile.cxx @@ -32,6 +32,8 @@ #include "util/Domain.hxx" #include "Log.hxx" +#include + #include static constexpr Domain state_file_domain("state_file"); @@ -87,11 +89,15 @@ StateFile::Write() FormatDebug(state_file_domain, "Saving state file %s", path_utf8.c_str()); - Error error; - FileOutputStream fos(path, error); - if (!fos.IsDefined() || !Write(fos, error) || !fos.Commit(error)) { - LogError(error); - return; + try { + Error error; + FileOutputStream fos(path); + if (!Write(fos, error) || !fos.Commit(error)) { + LogError(error); + return; + } + } catch (const std::exception &e) { + LogError(e); } RememberVersions(); diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.cxx b/src/db/plugins/simple/SimpleDatabasePlugin.cxx index d36438761..8538ca502 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.cxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.cxx @@ -379,9 +379,7 @@ SimpleDatabase::Save(Error &error) LogDebug(simple_db_domain, "writing DB"); - FileOutputStream fos(path, error); - if (!fos.IsDefined()) - return false; + FileOutputStream fos(path); OutputStream *os = &fos; diff --git a/src/db/update/Service.cxx b/src/db/update/Service.cxx index c0a536779..f1dc581d8 100644 --- a/src/db/update/Service.cxx +++ b/src/db/update/Service.cxx @@ -129,9 +129,13 @@ UpdateService::Task() next.discard); if (modified || !next.db->FileExists()) { - Error error; - if (!next.db->Save(error)) - LogError(error, "Failed to save database"); + try { + Error error; + if (!next.db->Save(error)) + LogError(error, "Failed to save database"); + } catch (const std::exception &e) { + LogError(e, "Failed to save database"); + } } if (!next.path_utf8.empty()) diff --git a/src/fs/io/FileOutputStream.cxx b/src/fs/io/FileOutputStream.cxx index a4ef8f6b4..bf23cafc1 100644 --- a/src/fs/io/FileOutputStream.cxx +++ b/src/fs/io/FileOutputStream.cxx @@ -20,23 +20,14 @@ #include "config.h" #include "FileOutputStream.hxx" #include "fs/FileSystem.hxx" +#include "system/Error.hxx" #include "util/Error.hxx" -FileOutputStream * -FileOutputStream::Create(Path path, Error &error) -{ - FileOutputStream *f = new FileOutputStream(path, error); - if (!f->IsDefined()) { - delete f; - f = nullptr; - } - - return f; -} +#include #ifdef WIN32 -FileOutputStream::FileOutputStream(Path _path, Error &error) +FileOutputStream::FileOutputStream(Path _path) :BaseFileOutputStream(_path) { SetHandle(CreateFile(_path.c_str(), GENERIC_WRITE, 0, nullptr, @@ -44,7 +35,7 @@ FileOutputStream::FileOutputStream(Path _path, Error &error) FILE_ATTRIBUTE_NORMAL|FILE_FLAG_WRITE_THROUGH, nullptr)); if (!IsDefined()) - error.FormatLastError("Failed to create %s", + throw FormatLastError("Failed to create %s", GetPath().ToUTF8().c_str()); } @@ -128,7 +119,7 @@ OpenTempFile(FileDescriptor &fd, Path path) #endif /* HAVE_LINKAT */ -FileOutputStream::FileOutputStream(Path _path, Error &error) +FileOutputStream::FileOutputStream(Path _path) :BaseFileOutputStream(_path) { #ifdef HAVE_LINKAT @@ -140,7 +131,7 @@ FileOutputStream::FileOutputStream(Path _path, Error &error) if (!SetFD().Open(GetPath().c_str(), O_WRONLY|O_CREAT|O_TRUNC, 0666)) - error.FormatErrno("Failed to create %s", + throw FormatErrno("Failed to create %s", GetPath().c_str()); #ifdef HAVE_LINKAT } @@ -216,7 +207,7 @@ FileOutputStream::Cancel() #endif -AppendFileOutputStream::AppendFileOutputStream(Path _path, Error &error) +AppendFileOutputStream::AppendFileOutputStream(Path _path) :BaseFileOutputStream(_path) { #ifdef WIN32 @@ -225,18 +216,19 @@ AppendFileOutputStream::AppendFileOutputStream(Path _path, Error &error) FILE_ATTRIBUTE_NORMAL|FILE_FLAG_WRITE_THROUGH, nullptr)); if (!IsDefined()) - error.FormatLastError("Failed to append to %s", + throw FormatLastError("Failed to append to %s", GetPath().ToUTF8().c_str()); if (!SeekEOF()) { - error.FormatLastError("Failed seek end-of-file of %s", - GetPath().ToUTF8().c_str()); + auto code = GetLastError(); Close(); + throw FormatLastError(code, "Failed seek end-of-file of %s", + GetPath().ToUTF8().c_str()); } #else if (!SetFD().Open(GetPath().c_str(), O_WRONLY|O_APPEND)) - error.FormatErrno("Failed to append to %s", + throw FormatErrno("Failed to append to %s", GetPath().c_str()); #endif } diff --git a/src/fs/io/FileOutputStream.hxx b/src/fs/io/FileOutputStream.hxx index b182fd03f..ec83aa615 100644 --- a/src/fs/io/FileOutputStream.hxx +++ b/src/fs/io/FileOutputStream.hxx @@ -102,7 +102,6 @@ protected: } #endif -public: bool IsDefined() const { #ifdef WIN32 return handle != INVALID_HANDLE_VALUE; @@ -111,6 +110,7 @@ public: #endif } +public: Path GetPath() const { return path; } @@ -132,22 +132,20 @@ class FileOutputStream final : public BaseFileOutputStream { #endif public: - FileOutputStream(Path _path, Error &error); + FileOutputStream(Path _path); ~FileOutputStream() { if (IsDefined()) Cancel(); } - static FileOutputStream *Create(Path path, Error &error); - bool Commit(Error &error); void Cancel(); }; class AppendFileOutputStream final : public BaseFileOutputStream { public: - AppendFileOutputStream(Path _path, Error &error); + AppendFileOutputStream(Path _path); ~AppendFileOutputStream() { if (IsDefined()) diff --git a/src/output/plugins/RecorderOutputPlugin.cxx b/src/output/plugins/RecorderOutputPlugin.cxx index 4f9231c70..ee1c5ef63 100644 --- a/src/output/plugins/RecorderOutputPlugin.cxx +++ b/src/output/plugins/RecorderOutputPlugin.cxx @@ -179,7 +179,6 @@ inline bool RecorderOutput::EncoderToFile(Error &error) { assert(file != nullptr); - assert(file->IsDefined()); return EncoderToOutputStream(*file, *encoder, error); } @@ -192,9 +191,12 @@ RecorderOutput::Open(AudioFormat &audio_format, Error &error) if (!HasDynamicPath()) { assert(!path.IsNull()); - file = FileOutputStream::Create(path, error); - if (file == nullptr) + try { + file = new FileOutputStream(path); + } catch (const std::exception &e) { + error.Set(recorder_domain, e.what()); return false; + } } else { /* don't open the file just yet; wait until we have a tag that we can use to build the path */ @@ -294,10 +296,13 @@ RecorderOutput::ReopenFormat(AllocatedPath &&new_path, Error &error) assert(path.IsNull()); assert(file == nullptr); - FileOutputStream *new_file = - FileOutputStream::Create(new_path, error); - if (new_file == nullptr) + FileOutputStream *new_file; + try { + new_file = new FileOutputStream(path); + } catch (const std::exception &e) { + error.Set(recorder_domain, e.what()); return false; + } AudioFormat new_audio_format = effective_audio_format; if (!encoder->Open(new_audio_format, error)) { diff --git a/test/WriteFile.cxx b/test/WriteFile.cxx index 706ca65eb..1366323c7 100644 --- a/test/WriteFile.cxx +++ b/test/WriteFile.cxx @@ -19,6 +19,7 @@ #include "config.h" #include "fs/io/FileOutputStream.hxx" +#include "Log.hxx" #include "util/Error.hxx" #include @@ -59,20 +60,21 @@ main(int argc, char **argv) const Path path = Path::FromFS(argv[1]); - Error error; - FileOutputStream fos(path, error); - if (!fos.IsDefined()) { - fprintf(stderr, "%s\n", error.GetMessage()); + try { + Error error; + FileOutputStream fos(path); + + if (!Copy(fos, STDIN_FILENO)) + return EXIT_FAILURE; + + if (!fos.Commit(error)) { + fprintf(stderr, "%s\n", error.GetMessage()); + return EXIT_FAILURE; + } + + return EXIT_SUCCESS; + } catch (const std::exception &e) { + LogError(e); return EXIT_FAILURE; } - - if (!Copy(fos, STDIN_FILENO)) - return EXIT_FAILURE; - - if (!fos.Commit(error)) { - fprintf(stderr, "%s\n", error.GetMessage()); - return EXIT_FAILURE; - } - - return EXIT_SUCCESS; }