db/Visitor: remove the Error parameter

Implementations shall use exceptions instead.

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,
+		s = [s, window_start, window_end, &i](const LightSong &song){
-						      Error &error2){
 			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);

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))
+		visit_tag(value);
-			return false;
 
 	return true;
 }

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<void(const LightDirectory &)> VisitDirectory;
-typedef std::function<bool(const LightSong &, Error &)> VisitSong;
+typedef std::function<void(const LightSong &)> VisitSong;
-typedef std::function<bool(const PlaylistInfo &, const LightDirectory &,
+typedef std::function<void(const PlaylistInfo &,
-			   Error &)> VisitPlaylist;
+			   const LightDirectory &)> VisitPlaylist;
 
-typedef std::function<bool(const Tag &, Error &)> VisitTag;
+typedef std::function<void(const Tag &)> VisitTag;
 
 #endif

src/db/plugins/ProxyDatabasePlugin.cxx

@@ -563,9 +563,8 @@ Visit(struct mpd_connection *connection,
 	time_t mtime = 0;
 #endif
 
-	if (visit_directory &&
+	if (visit_directory)
-	    !visit_directory(LightDirectory(path, mtime), error))
+		visit_directory(LightDirectory(path, mtime));
-		return false;
 
 	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,
+			Visit(filter, mpd_entity_get_song(entity), visit_song);
-				   mpd_entity_get_song(entity), visit_song,
-				   error))
-				return false;
 			break;
 
 		case MPD_ENTITY_TYPE_PLAYLIST:
-			if (!Visit(mpd_entity_get_playlist(entity),
+			Visit(mpd_entity_get_playlist(entity),
-				   visit_playlist, error))
+			      visit_playlist);
-				return false;
 			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,
+	    VisitSong visit_song)
-	    Error &error)
 {
 	assert(selection.recursive);
 	assert(visit_song);
@@ -705,19 +700,21 @@ SearchSongs(struct mpd_connection *connection,
 	    !mpd_search_commit(connection))
 		ThrowError(connection);
 
-	bool result = true;
+	while (auto *song = mpd_recv_song(connection)) {
-	struct mpd_song *song;
-	while (result && (song = mpd_recv_song(connection)) != nullptr) {
 		AllocatedProxySong song2(song);
 
-		result = !Match(selection.filter, song2) ||
+		if (Match(selection.filter, song2)) {
-			visit_song(song2, error);
+			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;
+	while (auto *pair = mpd_recv_pair_tag(connection, tag_type2)) {
-
-	struct mpd_pair *pair;
-	while (result &&
-	       (pair = mpd_recv_pair_tag(connection, tag_type2)) != nullptr) {
 		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

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)) &&
+			if (filter == nullptr || filter->Match(song2))
-			    !visit_song(song2, error))
+				visit_song(song2);
-				return false;
 		}
 	}
 
 	if (visit_playlist) {
 		for (const PlaylistInfo &p : playlists)
-			if (!visit_playlist(p, Export(), error))
+			visit_playlist(p, Export());
-				return false;
 	}
 
 	for (auto &child : children) {
-		if (visit_directory &&
+		if (visit_directory)
-		    !visit_directory(child.Export(), error))
+			visit_directory(child.Export());
-			return false;
 
 		if (recursive &&
 		    !child.Walk(recursive, filter,

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,
+		    const LightDirectory &directory)
-		    Error &error)
 {
-	return visit_playlist(playlist,
+	visit_playlist(playlist,
-			      PrefixedLightDirectory(directory, base),
+		       PrefixedLightDirectory(directory, base));
-			      error);
 }
 
 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);

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 &&
+		if (selection.recursive && visit_directory)
-		    !visit_directory(r.directory->Export(), error))
+			visit_directory(r.directory->Export());
-			return false;
 
 		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) ||
+				if (selection.Match(song2))
-					visit_song(song2, error);
+					visit_song(song2);
 			}
 		}
 	}

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,
+			 VisitPlaylist visit_playlist) const;
-			 Error &error) 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,
+			 VisitSong visit_song) const;
-			 Error &error) 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,
+			  VisitSong visit_song) const
-			  Error &error) 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(),
+		visitSong(std::move(dirent), path.c_str(),
-			       selection, visit_song,
+			  selection, visit_song);
-			       error))
-			return false;
 	}
-
-	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,
+	  VisitSong visit_song, VisitPlaylist visit_playlist)
-	  Error &error)
 {
 	assert(object.type == UPnPDirObject::Type::ITEM);
 
 	switch (object.item_class) {
 	case UPnPDirObject::ItemClass::MUSIC:
-		return !visit_song ||
+		visitSong(object, uri, selection, visit_song);
-			visitSong(object, uri,
+		break;
-				  selection, visit_song, error);
 
 	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);
+static void
-	gcc_unreachable();
-}
-
-static bool
 VisitObject(const UPnPDirObject &object, const char *uri,
 	    const DatabaseSelection &selection,
 	    VisitDirectory visit_directory,
 	    VisitSong visit_song,
-	    VisitPlaylist visit_playlist,
+	    VisitPlaylist visit_playlist)
-	    Error &error)
 {
 	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 ||
+		if (visit_directory)
-			visit_directory(LightDirectory(uri, 0), error);
+			visit_directory(LightDirectory(uri, 0));
+		break;
 
 	case UPnPDirObject::Type::ITEM:
-		return VisitItem(object, uri, selection,
+		VisitItem(object, uri, selection,
-				 visit_song, visit_playlist,
+			  visit_song, visit_playlist);
-				 error);
+		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,
+			  VisitPlaylist visit_playlist) const
-			  Error &error) 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(),
+			visitSong(std::move(dirent), path.c_str(),
-				       selection,
+				  selection, visit_song);
-				       visit_song, error))
-				return false;
 		}
-		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)
+	if (selection.recursive && selection.filter) {
-		return SearchSongs(server, tdirent.id.c_str(), selection,
+		SearchSongs(server, tdirent.id.c_str(), selection, visit_song);
-				   visit_song, error);
+		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,
+		VisitItem(tdirent, base_uri,
 			  selection,
-				 visit_song, visit_playlist,
+			  visit_song, visit_playlist);
-				 error);
+		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(),
+		VisitObject(dirent, uri.c_str(),
 			    selection,
 			    visit_directory,
-				 visit_song, visit_playlist,
+			    visit_song, visit_playlist);
-				 error))
-			return false;
 	}
-
-	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))
+				visit_directory(d);
-					return false;
 			}
 
-			if (selection.recursive &&
+			if (selection.recursive)
-			    !VisitServer(server, vpath, selection,
+				VisitServer(server, vpath, selection,
-					 visit_directory, visit_song, visit_playlist,
+					    visit_directory, visit_song,
-					 error))
+					    visit_playlist);
-				return false;
 		}
 
 		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,
+	VisitServer(server, vpath, selection,
-			   visit_directory, visit_song, visit_playlist, error);
+		    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))
+		visit_tag(builder.Commit());
-			return false;
 	}
 
 	return true;

test/DumpDatabase.cxx

@@ -64,30 +64,26 @@ public:
 	}
 };
 
-static bool
+static void
-DumpDirectory(const LightDirectory &directory, Error &)
+DumpDirectory(const LightDirectory &directory)
 {
 	cout << "D " << directory.GetPath() << endl;
-	return true;
 }
 
-static bool
+static void
-DumpSong(const LightSong &song, Error &)
+DumpSong(const LightSong &song)
 {
 	cout << "S ";
 	if (song.directory != nullptr)
 		cout << song.directory << "/";
 	cout << song.uri << endl;
-	return true;
 }
 
-static bool
+static void
-DumpPlaylist(const PlaylistInfo &playlist,
+DumpPlaylist(const PlaylistInfo &playlist, const LightDirectory &directory)
-	     const LightDirectory &directory, Error &)
 {
 	cout << "P " << directory.GetPath()
 	     << "/" << playlist.name.c_str() << endl;
-	return true;
 }
 
 int