From 3ff728ab02644ac4f84d4bc931ade91fc0667cc8 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 29 Oct 2016 10:04:43 +0200 Subject: [PATCH] db/Visitor: remove the Error parameter Implementations shall use exceptions instead. --- src/db/DatabasePrint.cxx | 6 +- src/db/UniqueTags.cxx | 6 +- src/db/Visitor.hxx | 11 +- src/db/plugins/ProxyDatabasePlugin.cxx | 78 +++++----- src/db/plugins/simple/Directory.cxx | 13 +- src/db/plugins/simple/Mount.cxx | 28 ++-- .../plugins/simple/SimpleDatabasePlugin.cxx | 9 +- src/db/plugins/upnp/UpnpDatabasePlugin.cxx | 137 ++++++++---------- test/DumpDatabase.cxx | 16 +- 9 files changed, 135 insertions(+), 169 deletions(-) diff --git a/src/db/DatabasePrint.cxx b/src/db/DatabasePrint.cxx index 39b645660..024573713 100644 --- a/src/db/DatabasePrint.cxx +++ b/src/db/DatabasePrint.cxx @@ -173,11 +173,11 @@ db_selection_print(Response &r, Partition &partition, if (window_start > 0 || window_end < (unsigned)std::numeric_limits::max()) - s = [s, window_start, window_end, &i](const LightSong &song, - Error &error2){ + s = [s, window_start, window_end, &i](const LightSong &song){ const bool in_window = i >= window_start && i < window_end; ++i; - return !in_window || s(song, error2); + if (in_window) + s(song); }; return db.Visit(selection, d, s, p, error); diff --git a/src/db/UniqueTags.cxx b/src/db/UniqueTags.cxx index 0c399b21b..76a60e674 100644 --- a/src/db/UniqueTags.cxx +++ b/src/db/UniqueTags.cxx @@ -26,7 +26,7 @@ #include -static bool +static void CollectTags(TagSet &set, TagType tag_type, tag_mask_t group_mask, const LightSong &song) { @@ -34,7 +34,6 @@ CollectTags(TagSet &set, TagType tag_type, tag_mask_t group_mask, const Tag &tag = *song.tag; set.InsertUnique(tag, tag_type, group_mask); - return true; } bool @@ -52,8 +51,7 @@ VisitUniqueTags(const Database &db, const DatabaseSelection &selection, return false; for (const auto &value : set) - if (!visit_tag(value, error)) - return false; + visit_tag(value); return true; } diff --git a/src/db/Visitor.hxx b/src/db/Visitor.hxx index 30c1e378e..1e9b86c2f 100644 --- a/src/db/Visitor.hxx +++ b/src/db/Visitor.hxx @@ -26,13 +26,12 @@ struct LightDirectory; struct LightSong; struct PlaylistInfo; struct Tag; -class Error; -typedef std::function VisitDirectory; -typedef std::function VisitSong; -typedef std::function VisitPlaylist; +typedef std::function VisitDirectory; +typedef std::function VisitSong; +typedef std::function VisitPlaylist; -typedef std::function VisitTag; +typedef std::function VisitTag; #endif diff --git a/src/db/plugins/ProxyDatabasePlugin.cxx b/src/db/plugins/ProxyDatabasePlugin.cxx index e378e2c66..a865343b8 100644 --- a/src/db/plugins/ProxyDatabasePlugin.cxx +++ b/src/db/plugins/ProxyDatabasePlugin.cxx @@ -563,9 +563,8 @@ Visit(struct mpd_connection *connection, time_t mtime = 0; #endif - if (visit_directory && - !visit_directory(LightDirectory(path, mtime), error)) - return false; + if (visit_directory) + visit_directory(LightDirectory(path, mtime)); if (recursive && !Visit(connection, path, recursive, filter, @@ -582,29 +581,30 @@ Match(const SongFilter *filter, const LightSong &song) return filter == nullptr || filter->Match(song); } -static bool +static void Visit(const SongFilter *filter, const mpd_song *_song, - VisitSong visit_song, Error &error) + VisitSong visit_song) { if (!visit_song) - return true; + return; const ProxySong song(_song); - return !Match(filter, song) || visit_song(song, error); + if (Match(filter, song)) + visit_song(song); } -static bool +static void Visit(const struct mpd_playlist *playlist, - VisitPlaylist visit_playlist, Error &error) + VisitPlaylist visit_playlist) { if (!visit_playlist) - return true; + return; PlaylistInfo p(mpd_playlist_get_path(playlist), mpd_playlist_get_last_modified(playlist)); - return visit_playlist(p, LightDirectory::Root(), error); + visit_playlist(p, LightDirectory::Root()); } class ProxyEntity { @@ -671,16 +671,12 @@ Visit(struct mpd_connection *connection, const char *uri, break; case MPD_ENTITY_TYPE_SONG: - if (!Visit(filter, - mpd_entity_get_song(entity), visit_song, - error)) - return false; + Visit(filter, mpd_entity_get_song(entity), visit_song); break; case MPD_ENTITY_TYPE_PLAYLIST: - if (!Visit(mpd_entity_get_playlist(entity), - visit_playlist, error)) - return false; + Visit(mpd_entity_get_playlist(entity), + visit_playlist); break; } } @@ -688,11 +684,10 @@ Visit(struct mpd_connection *connection, const char *uri, return true; } -static bool +static void SearchSongs(struct mpd_connection *connection, const DatabaseSelection &selection, - VisitSong visit_song, - Error &error) + VisitSong visit_song) { assert(selection.recursive); assert(visit_song); @@ -705,19 +700,21 @@ SearchSongs(struct mpd_connection *connection, !mpd_search_commit(connection)) ThrowError(connection); - bool result = true; - struct mpd_song *song; - while (result && (song = mpd_recv_song(connection)) != nullptr) { + while (auto *song = mpd_recv_song(connection)) { AllocatedProxySong song2(song); - result = !Match(selection.filter, song2) || - visit_song(song2, error); + if (Match(selection.filter, song2)) { + try { + visit_song(song2); + } catch (...) { + mpd_response_finish(connection); + throw; + } + } } - if (!mpd_response_finish(connection) && result) + if (!mpd_response_finish(connection)) ThrowError(connection); - - return result; } /** @@ -750,10 +747,12 @@ ProxyDatabase::Visit(const DatabaseSelection &selection, if (!visit_directory && !visit_playlist && selection.recursive && (ServerSupportsSearchBase(connection) ? !selection.IsEmpty() - : selection.HasOtherThanBase())) + : selection.HasOtherThanBase())) { /* this optimized code path can only be used under certain conditions */ - return ::SearchSongs(connection, selection, visit_song, error); + ::SearchSongs(connection, selection, visit_song); + return true; + } /* fall back to recursive walk (slow!) */ return ::Visit(connection, selection.uri.c_str(), @@ -787,11 +786,7 @@ ProxyDatabase::VisitUniqueTags(const DatabaseSelection &selection, if (!mpd_search_commit(connection)) ThrowError(connection); - bool result = true; - - struct mpd_pair *pair; - while (result && - (pair = mpd_recv_pair_tag(connection, tag_type2)) != nullptr) { + while (auto *pair = mpd_recv_pair_tag(connection, tag_type2)) { AtScopeExit(this, pair) { mpd_return_pair(connection, pair); }; @@ -807,13 +802,18 @@ ProxyDatabase::VisitUniqueTags(const DatabaseSelection &selection, given tag type to be present */ tag.AddEmptyItem(tag_type); - result = visit_tag(tag.Commit(), error); + try { + visit_tag(tag.Commit()); + } catch (...) { + mpd_response_finish(connection); + throw; + } } - if (!mpd_response_finish(connection) && result) + if (!mpd_response_finish(connection)) ThrowError(connection); - return result; + return true; } bool diff --git a/src/db/plugins/simple/Directory.cxx b/src/db/plugins/simple/Directory.cxx index e1606ac24..4aa5c8aa5 100644 --- a/src/db/plugins/simple/Directory.cxx +++ b/src/db/plugins/simple/Directory.cxx @@ -243,22 +243,19 @@ Directory::Walk(bool recursive, const SongFilter *filter, if (visit_song) { for (auto &song : songs){ const LightSong song2 = song.Export(); - if ((filter == nullptr || filter->Match(song2)) && - !visit_song(song2, error)) - return false; + if (filter == nullptr || filter->Match(song2)) + visit_song(song2); } } if (visit_playlist) { for (const PlaylistInfo &p : playlists) - if (!visit_playlist(p, Export(), error)) - return false; + visit_playlist(p, Export()); } for (auto &child : children) { - if (visit_directory && - !visit_directory(child.Export(), error)) - return false; + if (visit_directory) + visit_directory(child.Export()); if (recursive && !child.Walk(recursive, filter, diff --git a/src/db/plugins/simple/Mount.cxx b/src/db/plugins/simple/Mount.cxx index 05342a13f..9708780a1 100644 --- a/src/db/plugins/simple/Mount.cxx +++ b/src/db/plugins/simple/Mount.cxx @@ -40,29 +40,27 @@ struct PrefixedLightDirectory : LightDirectory { } }; -static bool +static void PrefixVisitDirectory(const char *base, const VisitDirectory &visit_directory, - const LightDirectory &directory, Error &error) + const LightDirectory &directory) { - return visit_directory(PrefixedLightDirectory(directory, base), error); + visit_directory(PrefixedLightDirectory(directory, base)); } -static bool +static void PrefixVisitSong(const char *base, const VisitSong &visit_song, - const LightSong &song, Error &error) + const LightSong &song) { - return visit_song(PrefixedLightSong(song, base), error); + visit_song(PrefixedLightSong(song, base)); } -static bool +static void PrefixVisitPlaylist(const char *base, const VisitPlaylist &visit_playlist, const PlaylistInfo &playlist, - const LightDirectory &directory, - Error &error) + const LightDirectory &directory) { - return visit_playlist(playlist, - PrefixedLightDirectory(directory, base), - error); + visit_playlist(playlist, + PrefixedLightDirectory(directory, base)); } bool @@ -77,17 +75,17 @@ WalkMount(const char *base, const Database &db, VisitDirectory vd; if (visit_directory) vd = std::bind(PrefixVisitDirectory, - base, std::ref(visit_directory), _1, _2); + base, std::ref(visit_directory), _1); VisitSong vs; if (visit_song) vs = std::bind(PrefixVisitSong, - base, std::ref(visit_song), _1, _2); + base, std::ref(visit_song), _1); VisitPlaylist vp; if (visit_playlist) vp = std::bind(PrefixVisitPlaylist, - base, std::ref(visit_playlist), _1, _2, _3); + base, std::ref(visit_playlist), _1, _2); return db.Visit(DatabaseSelection("", recursive, filter), vd, vs, vp, error); diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.cxx b/src/db/plugins/simple/SimpleDatabasePlugin.cxx index 958f52c12..2260e8aca 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.cxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.cxx @@ -274,9 +274,8 @@ SimpleDatabase::Visit(const DatabaseSelection &selection, if (r.uri == nullptr) { /* it's a directory */ - if (selection.recursive && visit_directory && - !visit_directory(r.directory->Export(), error)) - return false; + if (selection.recursive && visit_directory) + visit_directory(r.directory->Export()); return r.directory->Walk(selection.recursive, selection.filter, visit_directory, visit_song, @@ -289,8 +288,8 @@ SimpleDatabase::Visit(const DatabaseSelection &selection, Song *song = r.directory->FindSong(r.uri); if (song != nullptr) { const LightSong song2 = song->Export(); - return !selection.Match(song2) || - visit_song(song2, error); + if (selection.Match(song2)) + visit_song(song2); } } } diff --git a/src/db/plugins/upnp/UpnpDatabasePlugin.cxx b/src/db/plugins/upnp/UpnpDatabasePlugin.cxx index 684ee4c40..702a37602 100644 --- a/src/db/plugins/upnp/UpnpDatabasePlugin.cxx +++ b/src/db/plugins/upnp/UpnpDatabasePlugin.cxx @@ -103,23 +103,21 @@ public: } private: - bool VisitServer(const ContentDirectoryService &server, + void VisitServer(const ContentDirectoryService &server, const std::list &vpath, const DatabaseSelection &selection, VisitDirectory visit_directory, VisitSong visit_song, - VisitPlaylist visit_playlist, - Error &error) const; + VisitPlaylist visit_playlist) const; /** * Run an UPnP search according to MPD parameters, and * visit_song the results. */ - bool SearchSongs(const ContentDirectoryService &server, + void SearchSongs(const ContentDirectoryService &server, const char *objid, const DatabaseSelection &selection, - VisitSong visit_song, - Error &error) const; + VisitSong visit_song) const; UPnPDirContent SearchSongs(const ContentDirectoryService &server, const char *objid, @@ -307,13 +305,13 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server, return server.search(handle, objid, cond.c_str()); } -static bool +static void visitSong(const UPnPDirObject &meta, const char *path, const DatabaseSelection &selection, - VisitSong visit_song, Error& error) + VisitSong visit_song) { if (!visit_song) - return true; + return; LightSong song; song.directory = nullptr; @@ -323,7 +321,8 @@ visitSong(const UPnPDirObject &meta, const char *path, song.mtime = 0; song.start_time = song.end_time = SongTime::zero(); - return !selection.Match(song) || visit_song(song, error); + if (selection.Match(song)) + visit_song(song); } /** @@ -338,15 +337,14 @@ songPath(const std::string &servername, return servername + "/" + rootid + "/" + objid; } -bool +void UpnpDatabase::SearchSongs(const ContentDirectoryService &server, const char *objid, const DatabaseSelection &selection, - VisitSong visit_song, - Error &error) const + VisitSong visit_song) const { if (!visit_song) - return true; + return; for (auto &dirent : SearchSongs(server, objid, selection).objects) { if (dirent.type != UPnPDirObject::Type::ITEM || @@ -371,13 +369,9 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server, // which we later have to detect. const std::string path = songPath(server.getFriendlyName(), dirent.id); - if (!visitSong(std::move(dirent), path.c_str(), - selection, visit_song, - error)) - return false; + visitSong(std::move(dirent), path.c_str(), + selection, visit_song); } - - return true; } UPnPDirObject @@ -444,19 +438,17 @@ UpnpDatabase::Namei(const ContentDirectoryService &server, } } -static bool +static void VisitItem(const UPnPDirObject &object, const char *uri, const DatabaseSelection &selection, - VisitSong visit_song, VisitPlaylist visit_playlist, - Error &error) + VisitSong visit_song, VisitPlaylist visit_playlist) { assert(object.type == UPnPDirObject::Type::ITEM); switch (object.item_class) { case UPnPDirObject::ItemClass::MUSIC: - return !visit_song || - visitSong(object, uri, - selection, visit_song, error); + visitSong(object, uri, selection, visit_song); + break; case UPnPDirObject::ItemClass::PLAYLIST: if (visit_playlist) { @@ -468,23 +460,19 @@ VisitItem(const UPnPDirObject &object, const char *uri, see one... */ } - return true; + break; case UPnPDirObject::ItemClass::UNKNOWN: - return true; + break; } - - assert(false); - gcc_unreachable(); } -static bool +static void VisitObject(const UPnPDirObject &object, const char *uri, const DatabaseSelection &selection, VisitDirectory visit_directory, VisitSong visit_song, - VisitPlaylist visit_playlist, - Error &error) + VisitPlaylist visit_playlist) { switch (object.type) { case UPnPDirObject::Type::UNKNOWN: @@ -492,29 +480,26 @@ VisitObject(const UPnPDirObject &object, const char *uri, gcc_unreachable(); case UPnPDirObject::Type::CONTAINER: - return !visit_directory || - visit_directory(LightDirectory(uri, 0), error); + if (visit_directory) + visit_directory(LightDirectory(uri, 0)); + break; case UPnPDirObject::Type::ITEM: - return VisitItem(object, uri, selection, - visit_song, visit_playlist, - error); + VisitItem(object, uri, selection, + visit_song, visit_playlist); + break; } - - assert(false); - gcc_unreachable(); } // vpath is a parsed and writeable version of selection.uri. There is // really just one path parameter. -bool +void UpnpDatabase::VisitServer(const ContentDirectoryService &server, const std::list &vpath, const DatabaseSelection &selection, VisitDirectory visit_directory, VisitSong visit_song, - VisitPlaylist visit_playlist, - Error &error) const + VisitPlaylist visit_playlist) const { /* If the path begins with rootid, we know that this is a song, not a directory (because that's how we set things @@ -527,7 +512,7 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server, if (!vpath.empty() && vpath.front() == rootid) { switch (vpath.size()) { case 1: - return true; + return; case 2: break; @@ -547,12 +532,11 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server, std::string path = songPath(server.getFriendlyName(), dirent.id); - if (!visitSong(std::move(dirent), path.c_str(), - selection, - visit_song, error)) - return false; + visitSong(std::move(dirent), path.c_str(), + selection, visit_song); } - return true; + + return; } // Translate the target path into an object id and the associated metadata. @@ -562,19 +546,20 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server, if the filter is empty. In this case, we implement limited recursion (1-deep) here, which will handle the "add dir" case. */ - if (selection.recursive && selection.filter) - return SearchSongs(server, tdirent.id.c_str(), selection, - visit_song, error); + if (selection.recursive && selection.filter) { + SearchSongs(server, tdirent.id.c_str(), selection, visit_song); + return; + } const char *const base_uri = selection.uri.empty() ? server.getFriendlyName() : selection.uri.c_str(); if (tdirent.type == UPnPDirObject::Type::ITEM) { - return VisitItem(tdirent, base_uri, - selection, - visit_song, visit_playlist, - error); + VisitItem(tdirent, base_uri, + selection, + visit_song, visit_playlist); + return; } /* Target was a a container. Visit it. We could read slices @@ -583,15 +568,11 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server, for (const auto &dirent : server.readDir(handle, tdirent.id.c_str()).objects) { const std::string uri = PathTraitsUTF8::Build(base_uri, dirent.name.c_str()); - if (!VisitObject(dirent, uri.c_str(), - selection, - visit_directory, - visit_song, visit_playlist, - error)) - return false; + VisitObject(dirent, uri.c_str(), + selection, + visit_directory, + visit_song, visit_playlist); } - - return true; } // Deal with the possibly multiple servers, call VisitServer if needed. @@ -600,22 +581,20 @@ UpnpDatabase::Visit(const DatabaseSelection &selection, VisitDirectory visit_directory, VisitSong visit_song, VisitPlaylist visit_playlist, - Error &error) const + Error &) const { auto vpath = stringToTokens(selection.uri, "/", true); if (vpath.empty()) { for (const auto &server : discovery->GetDirectories()) { if (visit_directory) { const LightDirectory d(server.getFriendlyName(), 0); - if (!visit_directory(d, error)) - return false; + visit_directory(d); } - if (selection.recursive && - !VisitServer(server, vpath, selection, - visit_directory, visit_song, visit_playlist, - error)) - return false; + if (selection.recursive) + VisitServer(server, vpath, selection, + visit_directory, visit_song, + visit_playlist); } return true; @@ -626,15 +605,16 @@ UpnpDatabase::Visit(const DatabaseSelection &selection, vpath.pop_front(); auto server = discovery->GetServer(servername.c_str()); - return VisitServer(server, vpath, selection, - visit_directory, visit_song, visit_playlist, error); + VisitServer(server, vpath, selection, + visit_directory, visit_song, visit_playlist); + return true; } bool UpnpDatabase::VisitUniqueTags(const DatabaseSelection &selection, TagType tag, gcc_unused tag_mask_t group_mask, VisitTag visit_tag, - Error &error) const + Error &) const { // TODO: use group_mask @@ -664,8 +644,7 @@ UpnpDatabase::VisitUniqueTags(const DatabaseSelection &selection, for (const auto& value : values) { TagBuilder builder; builder.AddItem(tag, value.c_str()); - if (!visit_tag(builder.Commit(), error)) - return false; + visit_tag(builder.Commit()); } return true; diff --git a/test/DumpDatabase.cxx b/test/DumpDatabase.cxx index eebf679cc..4324b2088 100644 --- a/test/DumpDatabase.cxx +++ b/test/DumpDatabase.cxx @@ -64,30 +64,26 @@ public: } }; -static bool -DumpDirectory(const LightDirectory &directory, Error &) +static void +DumpDirectory(const LightDirectory &directory) { cout << "D " << directory.GetPath() << endl; - return true; } -static bool -DumpSong(const LightSong &song, Error &) +static void +DumpSong(const LightSong &song) { cout << "S "; if (song.directory != nullptr) cout << song.directory << "/"; cout << song.uri << endl; - return true; } -static bool -DumpPlaylist(const PlaylistInfo &playlist, - const LightDirectory &directory, Error &) +static void +DumpPlaylist(const PlaylistInfo &playlist, const LightDirectory &directory) { cout << "P " << directory.GetPath() << "/" << playlist.name.c_str() << endl; - return true; } int