From 1f184f4aecdafa77405bf80f6cb91892ed91a2b4 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 26 Dec 2015 06:30:25 +0100 Subject: PlaylistFile: throw exception on spl_map_to_fs() failure --- src/PlaylistFile.cxx | 139 ++++++++++++++++----------------------- src/PlaylistFile.hxx | 17 +++-- src/PlaylistPrint.cxx | 11 +--- src/PlaylistPrint.hxx | 6 +- src/PlaylistSave.cxx | 25 +++---- src/PlaylistSave.hxx | 10 ++- src/command/PlaylistCommands.cxx | 38 +++++------ src/db/DatabasePlaylist.cxx | 10 +-- 8 files changed, 102 insertions(+), 154 deletions(-) diff --git a/src/PlaylistFile.cxx b/src/PlaylistFile.cxx index efd7edbf4..7b7a63c4e 100644 --- a/src/PlaylistFile.cxx +++ b/src/PlaylistFile.cxx @@ -45,6 +45,8 @@ #include "util/UriUtil.hxx" #include "util/Error.hxx" +#include + #include #include #include @@ -91,37 +93,34 @@ spl_valid_name(const char *name_utf8) } static const AllocatedPath & -spl_map(Error &error) +spl_map() { const AllocatedPath &path_fs = map_spl_path(); if (path_fs.IsNull()) - error.Set(playlist_domain, int(PlaylistResult::DISABLED), - "Stored playlists are disabled"); + throw PlaylistError(PlaylistResult::DISABLED, + "Stored playlists are disabled"); + return path_fs; } -static bool -spl_check_name(const char *name_utf8, Error &error) +static void +spl_check_name(const char *name_utf8) { - if (!spl_valid_name(name_utf8)) { - error.Set(playlist_domain, int(PlaylistResult::BAD_NAME), + if (!spl_valid_name(name_utf8)) + throw PlaylistError(PlaylistResult::BAD_NAME, "Bad playlist name"); - return false; - } - - return true; } AllocatedPath -spl_map_to_fs(const char *name_utf8, Error &error) +spl_map_to_fs(const char *name_utf8) { - if (spl_map(error).IsNull() || !spl_check_name(name_utf8, error)) - return AllocatedPath::Null(); + spl_map(); + spl_check_name(name_utf8); auto path_fs = map_spl_utf8_to_fs(name_utf8); if (path_fs.IsNull()) - error.Set(playlist_domain, int(PlaylistResult::BAD_NAME), - "Bad playlist name"); + throw PlaylistError(PlaylistResult::BAD_NAME, + "Bad playlist name"); return path_fs; } @@ -179,9 +178,8 @@ ListPlaylistFiles(Error &error) { PlaylistVector list; - const auto &parent_path_fs = spl_map(error); - if (parent_path_fs.IsNull()) - return list; + const auto &parent_path_fs = spl_map(); + assert(!parent_path_fs.IsNull()); DirectoryReader reader(parent_path_fs); if (reader.HasFailed()) { @@ -199,15 +197,13 @@ ListPlaylistFiles(Error &error) return list; } -static bool -SavePlaylistFile(const PlaylistFileContents &contents, const char *utf8path, - Error &error) +static void +SavePlaylistFile(const PlaylistFileContents &contents, const char *utf8path) { assert(utf8path != nullptr); - const auto path_fs = spl_map_to_fs(utf8path, error); - if (path_fs.IsNull()) - return false; + const auto path_fs = spl_map_to_fs(utf8path); + assert(!path_fs.IsNull()); FileOutputStream fos(path_fs); @@ -219,17 +215,15 @@ SavePlaylistFile(const PlaylistFileContents &contents, const char *utf8path, bos.Flush(); fos.Commit(); - return true; } PlaylistFileContents -LoadPlaylistFile(const char *utf8path, Error &error) +LoadPlaylistFile(const char *utf8path) try { PlaylistFileContents contents; - const auto path_fs = spl_map_to_fs(utf8path, error); - if (path_fs.IsNull()) - return contents; + const auto path_fs = spl_map_to_fs(utf8path); + assert(!path_fs.IsNull()); TextFile file(path_fs); @@ -284,24 +278,18 @@ try { throw; } -bool -spl_move_index(const char *utf8path, unsigned src, unsigned dest, - Error &error) +void +spl_move_index(const char *utf8path, unsigned src, unsigned dest) { if (src == dest) /* this doesn't check whether the playlist exists, but what the hell.. */ - return true; + return; - auto contents = LoadPlaylistFile(utf8path, error); - if (contents.empty() && error.IsDefined()) - return false; + auto contents = LoadPlaylistFile(utf8path); - if (src >= contents.size() || dest >= contents.size()) { - error.Set(playlist_domain, int(PlaylistResult::BAD_RANGE), - "Bad range"); - return false; - } + if (src >= contents.size() || dest >= contents.size()) + throw PlaylistError(PlaylistResult::BAD_RANGE, "Bad range"); const auto src_i = std::next(contents.begin(), src); auto value = std::move(*src_i); @@ -310,18 +298,16 @@ spl_move_index(const char *utf8path, unsigned src, unsigned dest, const auto dest_i = std::next(contents.begin(), dest); contents.insert(dest_i, std::move(value)); - bool result = SavePlaylistFile(contents, utf8path, error); + SavePlaylistFile(contents, utf8path); idle_add(IDLE_STORED_PLAYLIST); - return result; } bool spl_clear(const char *utf8path, Error &error) { - const auto path_fs = spl_map_to_fs(utf8path, error); - if (path_fs.IsNull()) - return false; + const auto path_fs = spl_map_to_fs(utf8path); + assert(!path_fs.IsNull()); FILE *file = FOpen(path_fs, FOpenMode::WriteText); if (file == nullptr) { @@ -338,9 +324,8 @@ spl_clear(const char *utf8path, Error &error) bool spl_delete(const char *name_utf8, Error &error) { - const auto path_fs = spl_map_to_fs(name_utf8, error); - if (path_fs.IsNull()) - return false; + const auto path_fs = spl_map_to_fs(name_utf8); + assert(!path_fs.IsNull()); if (!RemoveFile(path_fs)) { playlist_errno(error); @@ -351,41 +336,31 @@ spl_delete(const char *name_utf8, Error &error) return true; } -bool -spl_remove_index(const char *utf8path, unsigned pos, Error &error) +void +spl_remove_index(const char *utf8path, unsigned pos) { - auto contents = LoadPlaylistFile(utf8path, error); - if (contents.empty() && error.IsDefined()) - return false; + auto contents = LoadPlaylistFile(utf8path); - if (pos >= contents.size()) { - error.Set(playlist_domain, int(PlaylistResult::BAD_RANGE), - "Bad range"); - return false; - } + if (pos >= contents.size()) + throw PlaylistError(PlaylistResult::BAD_RANGE, "Bad range"); contents.erase(std::next(contents.begin(), pos)); - bool result = SavePlaylistFile(contents, utf8path, error); - + SavePlaylistFile(contents, utf8path); idle_add(IDLE_STORED_PLAYLIST); - return result; } -bool -spl_append_song(const char *utf8path, const DetachedSong &song, Error &error) +void +spl_append_song(const char *utf8path, const DetachedSong &song) try { - const auto path_fs = spl_map_to_fs(utf8path, error); - if (path_fs.IsNull()) - return false; + const auto path_fs = spl_map_to_fs(utf8path); + assert(!path_fs.IsNull()); AppendFileOutputStream fos(path_fs); - if (fos.Tell() / (MPD_PATH_MAX + 1) >= playlist_max_length) { - error.Set(playlist_domain, int(PlaylistResult::TOO_LARGE), - "Stored playlist is too large"); - return false; - } + if (fos.Tell() / (MPD_PATH_MAX + 1) >= playlist_max_length) + throw PlaylistError(PlaylistResult::TOO_LARGE, + "Stored playlist is too large"); BufferedOutputStream bos(fos); @@ -395,7 +370,6 @@ try { fos.Commit(); idle_add(IDLE_STORED_PLAYLIST); - return true; } catch (const std::system_error &e) { if (IsFileNotFound(e)) throw PlaylistError::NoSuchList(); @@ -407,13 +381,12 @@ spl_append_uri(const char *utf8file, const SongLoader &loader, const char *url, Error &error) { - DetachedSong *song = loader.LoadSong(url, error); + std::unique_ptr song(loader.LoadSong(url, error)); if (song == nullptr) return false; - bool success = spl_append_song(utf8file, *song, error); - delete song; - return success; + spl_append_song(utf8file, *song); + return true; } static bool @@ -444,13 +417,11 @@ spl_rename_internal(Path from_path_fs, Path to_path_fs, bool spl_rename(const char *utf8from, const char *utf8to, Error &error) { - const auto from_path_fs = spl_map_to_fs(utf8from, error); - if (from_path_fs.IsNull()) - return false; + const auto from_path_fs = spl_map_to_fs(utf8from); + assert(!from_path_fs.IsNull()); - const auto to_path_fs = spl_map_to_fs(utf8to, error); - if (to_path_fs.IsNull()) - return false; + const auto to_path_fs = spl_map_to_fs(utf8to); + assert(!to_path_fs.IsNull()); return spl_rename_internal(from_path_fs, to_path_fs, error); } diff --git a/src/PlaylistFile.hxx b/src/PlaylistFile.hxx index 02cd77ec2..ccea39a3b 100644 --- a/src/PlaylistFile.hxx +++ b/src/PlaylistFile.hxx @@ -47,7 +47,7 @@ bool spl_valid_name(const char *name_utf8); AllocatedPath -spl_map_to_fs(const char *name_utf8, Error &error); +spl_map_to_fs(const char *name_utf8); /** * Returns a list of stored_playlist_info struct pointers. Returns @@ -57,11 +57,10 @@ PlaylistVector ListPlaylistFiles(Error &error); PlaylistFileContents -LoadPlaylistFile(const char *utf8path, Error &error); +LoadPlaylistFile(const char *utf8path); -bool -spl_move_index(const char *utf8path, unsigned src, unsigned dest, - Error &error); +void +spl_move_index(const char *utf8path, unsigned src, unsigned dest); bool spl_clear(const char *utf8path, Error &error); @@ -69,11 +68,11 @@ spl_clear(const char *utf8path, Error &error); bool spl_delete(const char *name_utf8, Error &error); -bool -spl_remove_index(const char *utf8path, unsigned pos, Error &error); +void +spl_remove_index(const char *utf8path, unsigned pos); -bool -spl_append_song(const char *utf8path, const DetachedSong &song, Error &error); +void +spl_append_song(const char *utf8path, const DetachedSong &song); bool spl_append_uri(const char *path_utf8, diff --git a/src/PlaylistPrint.cxx b/src/PlaylistPrint.cxx index c8abd89be..c97dc0981 100644 --- a/src/PlaylistPrint.cxx +++ b/src/PlaylistPrint.cxx @@ -141,19 +141,16 @@ PrintSongDetails(Response &r, Partition &partition, const char *uri_utf8) #endif -bool +void spl_print(Response &r, Partition &partition, - const char *name_utf8, bool detail, - Error &error) + const char *name_utf8, bool detail) { #ifndef ENABLE_DATABASE (void)partition; (void)detail; #endif - PlaylistFileContents contents = LoadPlaylistFile(name_utf8, error); - if (contents.empty() && error.IsDefined()) - return false; + PlaylistFileContents contents = LoadPlaylistFile(name_utf8); for (const auto &uri_utf8 : contents) { #ifdef ENABLE_DATABASE @@ -162,6 +159,4 @@ spl_print(Response &r, Partition &partition, #endif r.Format(SONG_FILE "%s\n", uri_utf8.c_str()); } - - return true; } diff --git a/src/PlaylistPrint.hxx b/src/PlaylistPrint.hxx index b12162262..422dba117 100644 --- a/src/PlaylistPrint.hxx +++ b/src/PlaylistPrint.hxx @@ -97,11 +97,9 @@ playlist_print_changes_position(Response &r, * @param client the client which requested the playlist * @param name_utf8 the name of the stored playlist in UTF-8 encoding * @param detail true if all details should be printed - * @return true on success, false if the playlist does not exist */ -bool +void spl_print(Response &r, Partition &partition, - const char *name_utf8, bool detail, - Error &error); + const char *name_utf8, bool detail); #endif diff --git a/src/PlaylistSave.cxx b/src/PlaylistSave.cxx index 9ed4d832b..f59be6036 100644 --- a/src/PlaylistSave.cxx +++ b/src/PlaylistSave.cxx @@ -67,18 +67,15 @@ playlist_print_uri(BufferedOutputStream &os, const char *uri) os.Format("%s\n", NarrowPath(path).c_str()); } -bool -spl_save_queue(const char *name_utf8, const Queue &queue, Error &error) +void +spl_save_queue(const char *name_utf8, const Queue &queue) { - const auto path_fs = spl_map_to_fs(name_utf8, error); - if (path_fs.IsNull()) - return false; + const auto path_fs = spl_map_to_fs(name_utf8); + assert(!path_fs.IsNull()); - if (FileExists(path_fs)) { - error.Set(playlist_domain, int(PlaylistResult::LIST_EXISTS), - "Playlist already exists"); - return false; - } + if (FileExists(path_fs)) + throw PlaylistError(PlaylistResult::LIST_EXISTS, + "Playlist already exists"); FileOutputStream fos(path_fs); BufferedOutputStream bos(fos); @@ -90,12 +87,10 @@ spl_save_queue(const char *name_utf8, const Queue &queue, Error &error) fos.Commit(); idle_add(IDLE_STORED_PLAYLIST); - return true; } -bool -spl_save_playlist(const char *name_utf8, const playlist &playlist, - Error &error) +void +spl_save_playlist(const char *name_utf8, const playlist &playlist) { - return spl_save_queue(name_utf8, playlist.queue, error); + spl_save_queue(name_utf8, playlist.queue); } diff --git a/src/PlaylistSave.hxx b/src/PlaylistSave.hxx index 536c5c8d1..d5c5cfba7 100644 --- a/src/PlaylistSave.hxx +++ b/src/PlaylistSave.hxx @@ -24,7 +24,6 @@ struct Queue; struct playlist; class BufferedOutputStream; class DetachedSong; -class Error; void playlist_print_song(BufferedOutputStream &os, const DetachedSong &song); @@ -35,14 +34,13 @@ playlist_print_uri(BufferedOutputStream &os, const char *uri); /** * Saves a queue object into a stored playlist file. */ -bool -spl_save_queue(const char *name_utf8, const Queue &queue, Error &error); +void +spl_save_queue(const char *name_utf8, const Queue &queue); /** * Saves a playlist object into a stored playlist file. */ -bool -spl_save_playlist(const char *name_utf8, const playlist &playlist, - Error &error); +void +spl_save_playlist(const char *name_utf8, const playlist &playlist); #endif diff --git a/src/command/PlaylistCommands.cxx b/src/command/PlaylistCommands.cxx index 2da64a80e..ba74fc84a 100644 --- a/src/command/PlaylistCommands.cxx +++ b/src/command/PlaylistCommands.cxx @@ -59,12 +59,10 @@ print_spl_list(Response &r, const PlaylistVector &list) } CommandResult -handle_save(Client &client, Request args, Response &r) +handle_save(Client &client, Request args, gcc_unused Response &r) { - Error error; - return spl_save_playlist(args.front(), client.playlist, error) - ? CommandResult::OK - : print_error(r, error); + spl_save_playlist(args.front(), client.playlist); + return CommandResult::OK; } CommandResult @@ -94,10 +92,8 @@ handle_listplaylist(Client &client, Request args, Response &r) name, false)) return CommandResult::OK; - Error error; - return spl_print(r, client.partition, name, false, error) - ? CommandResult::OK - : print_error(r, error); + spl_print(r, client.partition, name, false); + return CommandResult::OK; } CommandResult @@ -109,10 +105,8 @@ handle_listplaylistinfo(Client &client, Request args, Response &r) name, true)) return CommandResult::OK; - Error error; - return spl_print(r, client.partition, name, true, error) - ? CommandResult::OK - : print_error(r, error); + spl_print(r, client.partition, name, true); + return CommandResult::OK; } CommandResult @@ -139,28 +133,26 @@ handle_rename(gcc_unused Client &client, Request args, Response &r) } CommandResult -handle_playlistdelete(gcc_unused Client &client, Request args, Response &r) +handle_playlistdelete(gcc_unused Client &client, + Request args, gcc_unused Response &r) { const char *const name = args[0]; unsigned from = args.ParseUnsigned(1); - Error error; - return spl_remove_index(name, from, error) - ? CommandResult::OK - : print_error(r, error); + spl_remove_index(name, from); + return CommandResult::OK; } CommandResult -handle_playlistmove(gcc_unused Client &client, Request args, Response &r) +handle_playlistmove(gcc_unused Client &client, + Request args, gcc_unused Response &r) { const char *const name = args.front(); unsigned from = args.ParseUnsigned(1); unsigned to = args.ParseUnsigned(2); - Error error; - return spl_move_index(name, from, to, error) - ? CommandResult::OK - : print_error(r, error); + spl_move_index(name, from, to); + return CommandResult::OK; } CommandResult diff --git a/src/db/DatabasePlaylist.cxx b/src/db/DatabasePlaylist.cxx index d1e042c6a..18a5c22aa 100644 --- a/src/db/DatabasePlaylist.cxx +++ b/src/db/DatabasePlaylist.cxx @@ -30,11 +30,11 @@ static bool AddSong(const Storage &storage, const char *playlist_path_utf8, - const LightSong &song, Error &error) + const LightSong &song) { - return spl_append_song(playlist_path_utf8, - DatabaseDetachSong(storage, song), - error); + spl_append_song(playlist_path_utf8, + DatabaseDetachSong(storage, song)); + return true; } bool @@ -47,6 +47,6 @@ search_add_to_playlist(const Database &db, const Storage &storage, using namespace std::placeholders; const auto f = std::bind(AddSong, std::ref(storage), - playlist_path_utf8, _1, _2); + playlist_path_utf8, _1); return db.Visit(selection, f, error); } -- cgit v1.2.3