summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMax Kellermann <max@musicpd.org>2016-10-29 10:04:43 +0200
committerMax Kellermann <max@musicpd.org>2016-10-29 10:04:43 +0200
commit3ff728ab02644ac4f84d4bc931ade91fc0667cc8 (patch)
treed63f4b148d5e243993a8ed606643eecfd9be3a2c
parent78bf4ef5fa20cad2d91566ca82ad57de2dad0efb (diff)
db/Visitor: remove the Error parameter
Implementations shall use exceptions instead.
-rw-r--r--src/db/DatabasePrint.cxx6
-rw-r--r--src/db/UniqueTags.cxx6
-rw-r--r--src/db/Visitor.hxx11
-rw-r--r--src/db/plugins/ProxyDatabasePlugin.cxx78
-rw-r--r--src/db/plugins/simple/Directory.cxx13
-rw-r--r--src/db/plugins/simple/Mount.cxx28
-rw-r--r--src/db/plugins/simple/SimpleDatabasePlugin.cxx9
-rw-r--r--src/db/plugins/upnp/UpnpDatabasePlugin.cxx137
-rw-r--r--test/DumpDatabase.cxx16
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<int>::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 <assert.h>
-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<bool(const LightDirectory &, Error &)> VisitDirectory;
-typedef std::function<bool(const LightSong &, Error &)> VisitSong;
-typedef std::function<bool(const PlaylistInfo &, const LightDirectory &,
- Error &)> VisitPlaylist;
+typedef std::function<void(const LightDirectory &)> VisitDirectory;
+typedef std::function<void(const LightSong &)> VisitSong;
+typedef std::function<void(const PlaylistInfo &,
+ const LightDirectory &)> VisitPlaylist;
-typedef std::function<bool(const Tag &, Error &)> VisitTag;
+typedef std::function<void(const Tag &)> 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<std::string> &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<std::string> &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