db/upnp/Discovery: eliminate Error attribute

Move code to method Start() and add Error& parameters to several
methods.

3 files changed, 32 insertions, 51 deletions

diff --git a/src/db/UpnpDatabasePlugin.cxx b/src/db/UpnpDatabasePlugin.cxx
index 054e20377..98c8a5ff3 100644
--- a/src/db/UpnpDatabasePlugin.cxx
+++ b/src/db/UpnpDatabasePlugin.cxx
@@ -165,8 +165,7 @@ UpnpDatabase::Open(Error &error)
 	}
 
 	m_superdir = new UPnPDeviceDirectory(m_lib);
-	if (!m_superdir->ok()) {
-		error.Set(m_superdir->GetError());
+	if (!m_superdir->Start(error)) {
 		delete m_superdir;
 		delete m_lib;
 		return false;
@@ -215,20 +214,12 @@ upnpItemToSong(const UPnPDirObject &dirent, const char *uri)
 Song *
 UpnpDatabase::GetSong(const char *uri, Error &error) const
 {
-	if (!m_superdir->ok()) {
-		error.Set(upnp_domain,
-			  "UpnpDatabase::GetSong() superdir is sick");
-		return nullptr;
-	}
-
 	Song *song = nullptr;
 	auto vpath = stringToTokens(uri, "/", true);
 	if (vpath.size() >= 2) {
 		ContentDirectoryService server;
-		if (!m_superdir->getServer(vpath[0].c_str(), server)) {
-			error.Set(upnp_domain, "server not found");
+		if (!m_superdir->getServer(vpath[0].c_str(), server, error))
 			return nullptr;
-		}
 
 		vpath.erase(vpath.begin());
 		UPnPDirObject dirent;
@@ -688,12 +679,8 @@ UpnpDatabase::Visit(const DatabaseSelection &selection,
 		    Error &error) const
 {
 	std::vector<ContentDirectoryService> servers;
-	if (!m_superdir->ok() ||
-	    !m_superdir->getDirServices(servers)) {
-		error.Set(upnp_domain,
-			  "UpnpDatabase::Visit() superdir is sick");
+	if (!m_superdir->getDirServices(servers, error))
 		return false;
-	}
 
 	auto vpath = stringToTokens(selection.uri, "/", true);
 	if (vpath.empty()) {
@@ -748,12 +735,8 @@ UpnpDatabase::VisitUniqueTags(const DatabaseSelection &selection,
 		return true;
 
 	std::vector<ContentDirectoryService> servers;
-	if (!m_superdir->ok() ||
-	    !m_superdir->getDirServices(servers)) {
-		error.Set(upnp_domain,
-			  "UpnpDatabase::Visit() superdir is sick");
+	if (!m_superdir->getDirServices(servers, error))
 		return false;
-	}
 
 	std::set<std::string> values;
 	for (auto& server : servers) {
diff --git a/src/db/upnp/Discovery.cxx b/src/db/upnp/Discovery.cxx
index 5e6479b3d..f47741240 100644
--- a/src/db/upnp/Discovery.cxx
+++ b/src/db/upnp/Discovery.cxx
@@ -160,8 +160,8 @@ UPnPDeviceDirectory::cluCallBack(Upnp_EventType et, void *evp)
 	return UPNP_E_SUCCESS;
 }
 
-void
-UPnPDeviceDirectory::expireDevices()
+bool
+UPnPDeviceDirectory::expireDevices(Error &error)
 {
 	const ScopeLock protect(mutex);
 	time_t now = time(0);
@@ -178,7 +178,9 @@ UPnPDeviceDirectory::expireDevices()
 	}
 
 	if (didsomething)
-		search();
+		return search(error);
+
+	return true;
 }
 
 UPnPDeviceDirectory::UPnPDeviceDirectory(LibUPnP *_lib)
@@ -186,20 +188,25 @@ UPnPDeviceDirectory::UPnPDeviceDirectory(LibUPnP *_lib)
 	 discoveredQueue("DiscoveredQueue"),
 	 m_searchTimeout(2), m_lastSearch(0)
 {
+}
+
+bool
+UPnPDeviceDirectory::Start(Error &error)
+{
 	if (!discoveredQueue.start(1, discoExplorer, this)) {
 		error.Set(upnp_domain, "Discover work queue start failed");
-		return;
+		return false;
 	}
 
 	lib->SetHandler([this](Upnp_EventType type, void *event){
 			cluCallBack(type, event);
 		});
 
-	search();
+	return search(error);
 }
 
 bool
-UPnPDeviceDirectory::search()
+UPnPDeviceDirectory::search(Error &error)
 {
 	time_t now = time(0);
 	if (now - m_lastSearch < 10)
@@ -229,13 +236,12 @@ UPnPDeviceDirectory::search()
 }
 
 bool
-UPnPDeviceDirectory::getDirServices(std::vector<ContentDirectoryService> &out)
+UPnPDeviceDirectory::getDirServices(std::vector<ContentDirectoryService> &out,
+				    Error &error)
 {
-	if (!ok())
-		return false;
-
 	// Has locking, do it before our own lock
-	expireDevices();
+	if (!expireDevices(error))
+		return false;
 
 	const ScopeLock protect(mutex);
 
@@ -253,12 +259,12 @@ UPnPDeviceDirectory::getDirServices(std::vector<ContentDirectoryService> &out)
 
 bool
 UPnPDeviceDirectory::getServer(const char *friendlyName,
-			       ContentDirectoryService &server)
+			       ContentDirectoryService &server,
+			       Error &error)
 {
 	std::vector<ContentDirectoryService> ds;
-	if (!getDirServices(ds)) {
+	if (!getDirServices(ds, error))
 		return false;
-	}
 
 	for (const auto &i : ds) {
 		if (strcmp(friendlyName, i.getFriendlyName()) == 0) {
@@ -267,5 +273,6 @@ UPnPDeviceDirectory::getServer(const char *friendlyName,
 		}
 	}
 
+	error.Set(upnp_domain, "Server not found");
 	return false;
 }
diff --git a/src/db/upnp/Discovery.hxx b/src/db/upnp/Discovery.hxx
index edda3a8de..59eed9331 100644
--- a/src/db/upnp/Discovery.hxx
+++ b/src/db/upnp/Discovery.hxx
@@ -82,8 +82,6 @@ class UPnPDeviceDirectory {
 
 	LibUPnP *const lib;
 
-	Error error;
-
 	Mutex mutex;
 	std::map<std::string, ContentDirectoryDescriptor> directories;
 	WorkQueue<DiscoveredTask *> discoveredQueue;
@@ -103,8 +101,10 @@ public:
 	UPnPDeviceDirectory(const UPnPDeviceDirectory &) = delete;
 	UPnPDeviceDirectory& operator=(const UPnPDeviceDirectory &) = delete;
 
+	bool Start(Error &error);
+
 	/** Retrieve the directory services currently seen on the network */
-	bool getDirServices(std::vector<ContentDirectoryService> &);
+	bool getDirServices(std::vector<ContentDirectoryService> &, Error &);
 
 	/**
 	 * Get server by friendly name. It's a bit wasteful to copy
@@ -112,27 +112,18 @@ public:
 	 * there isn't going to be millions...
 	 */
 	bool getServer(const char *friendlyName,
-		       ContentDirectoryService &server);
-
-	/** My health */
-	bool ok() const {
-		return !error.IsDefined();
-	}
-
-	/** My diagnostic if health is bad */
-	const Error &GetError() const {
-		return error;
-	}
+		       ContentDirectoryService &server,
+		       Error &error);
 
 private:
-	bool search();
+	bool search(Error &error);
 
 	/**
 	 * Look at the devices and get rid of those which have not
 	 * been seen for too long. We do this when listing the top
 	 * directory.
 	 */
-	void expireDevices();
+	bool expireDevices(Error &error);
 
 	/**
 	 * Worker routine for the discovery queue. Get messages about