lib/upnp: use C++ exceptions instead of class Error

src/db/plugins/upnp/ContentDirectoryService.cxx

@@ -25,6 +25,7 @@
 #include "Directory.hxx"
 #include "util/NumberParser.hxx"
 #include "util/UriUtil.hxx"
+#include "util/RuntimeError.hxx"
 #include "util/Error.hxx"
 
 #include <stdio.h>
@@ -59,21 +60,16 @@ ContentDirectoryService::readDirSlice(UpnpClient_Handle hdl,
 				 "SortCriteria", "",
 				 "StartingIndex", ofbuf,
 				 "RequestedCount", cntbuf);
-	if (request == nullptr) {
+	if (request == nullptr)
-		error.Set(upnp_domain, "UpnpMakeAction() failed");
+		throw std::runtime_error("UpnpMakeAction() failed");
-		return false;
-	}
 
 	IXML_Document *response;
 	int code = UpnpSendAction(hdl, m_actionURL.c_str(), m_serviceType.c_str(),
 				  0 /*devUDN*/, request, &response);
 	ixmlDocument_free(request);
-	if (code != UPNP_E_SUCCESS) {
+	if (code != UPNP_E_SUCCESS)
-		error.Format(upnp_domain, code,
+		throw FormatRuntimeError("UpnpSendAction() failed: %s",
-			     "UpnpSendAction() failed: %s",
+					 UpnpGetErrorMessage(code));
-			     UpnpGetErrorMessage(code));
-		return false;
-	}
 
 	const char *value = ixmlwrap::getFirstElementValue(response, "NumberReturned");
 	didreadp = value != nullptr
@@ -129,22 +125,17 @@ ContentDirectoryService::search(UpnpClient_Handle hdl,
 					 "SortCriteria", "",
 					 "StartingIndex", ofbuf,
 					 "RequestedCount", "0"); // Setting a value here gets twonky into fits
-		if (request == 0) {
+		if (request == 0)
-			error.Set(upnp_domain, "UpnpMakeAction() failed");
+			throw std::runtime_error("UpnpMakeAction() failed");
-			return false;
-		}
 
 		IXML_Document *response;
 		auto code = UpnpSendAction(hdl, m_actionURL.c_str(),
 					   m_serviceType.c_str(),
 					   0 /*devUDN*/, request, &response);
 		ixmlDocument_free(request);
-		if (code != UPNP_E_SUCCESS) {
+		if (code != UPNP_E_SUCCESS)
-			error.Format(upnp_domain, code,
+			throw FormatRuntimeError("UpnpSendAction() failed: %s",
-				     "UpnpSendAction() failed: %s",
+						 UpnpGetErrorMessage(code));
-				     UpnpGetErrorMessage(code));
-			return false;
-		}
 
 		const char *value =
 			ixmlwrap::getFirstElementValue(response, "NumberReturned");
@@ -182,22 +173,17 @@ ContentDirectoryService::getMetadata(UpnpClient_Handle hdl,
 				 "SortCriteria", "",
 				 "StartingIndex", "0",
 				 "RequestedCount", "1");
-	if (request == nullptr) {
+	if (request == nullptr)
-		error.Set(upnp_domain, "UpnpMakeAction() failed");
+		throw std::runtime_error("UpnpMakeAction() failed");
-		return false;
-	}
 
 	IXML_Document *response;
 	auto code = UpnpSendAction(hdl, m_actionURL.c_str(),
 				   m_serviceType.c_str(),
 				   0 /*devUDN*/, request, &response);
 	ixmlDocument_free(request);
-	if (code != UPNP_E_SUCCESS) {
+	if (code != UPNP_E_SUCCESS)
-		error.Format(upnp_domain, code,
+		throw FormatRuntimeError("UpnpSendAction() failed: %s",
-			     "UpnpSendAction() failed: %s",
+					 UpnpGetErrorMessage(code));
-			     UpnpGetErrorMessage(code));
-		return false;
-	}
 
 	bool success = ReadResultTag(dirbuf, response, error);
 	ixmlDocument_free(response);

src/db/plugins/upnp/UpnpDatabasePlugin.cxx

@@ -179,16 +179,17 @@ UpnpDatabase::Configure(const ConfigBlock &, Error &)
 }
 
 bool
-UpnpDatabase::Open(Error &error)
+UpnpDatabase::Open(gcc_unused Error &error)
 {
-	if (!UpnpClientGlobalInit(handle, error))
+	UpnpClientGlobalInit(handle);
-		return false;
 
 	discovery = new UPnPDeviceDirectory(handle);
-	if (!discovery->Start(error)) {
+	try {
+		discovery->Start();
+	} catch (...) {
 		delete discovery;
 		UpnpClientGlobalFinish();
-		return false;
+		throw;
 	}
 
 	return true;
@@ -216,15 +217,11 @@ const LightSong *
 UpnpDatabase::GetSong(const char *uri, Error &error) const
 {
 	auto vpath = stringToTokens(uri, "/", true);
-	if (vpath.size() < 2) {
+	if (vpath.size() < 2)
-		error.Format(db_domain, (int)DatabaseErrorCode::NOT_FOUND,
+		throw DatabaseError(DatabaseErrorCode::NOT_FOUND,
-			     "No such song: %s", uri);
+				    "No such song");
-		return nullptr;
-	}
 
-	ContentDirectoryService server;
+	auto server = discovery->GetServer(vpath.front().c_str());
-	if (!discovery->GetServer(vpath.front().c_str(), server, error))
-		return nullptr;
 
 	vpath.pop_front();
 
@@ -276,10 +273,7 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 	if (selection.filter == nullptr)
 		return true;
 
-	std::list<std::string> searchcaps;
+	const auto searchcaps = server.getSearchCapabilities(handle);
-	if (!server.getSearchCapabilities(handle, searchcaps, error))
-		return false;
-
 	if (searchcaps.empty())
 		return true;
 
@@ -434,13 +428,10 @@ UpnpDatabase::ReadNode(const ContentDirectoryService &server,
 	if (!server.getMetadata(handle, objid, dirbuf, error))
 		return false;
 
-	if (dirbuf.objects.size() == 1) {
+	if (dirbuf.objects.size() != 1)
-		dirent = std::move(dirbuf.objects.front());
+		throw std::runtime_error("Bad resource");
-	} else {
-		error.Format(upnp_domain, "Bad resource");
-		return false;
-	}
 
+	dirent = std::move(dirbuf.objects.front());
 	return true;
 }
 
@@ -495,24 +486,18 @@ UpnpDatabase::Namei(const ContentDirectoryService &server,
 
 		// Look for the name in the sub-container list
 		UPnPDirObject *child = dirbuf.FindObject(i->c_str());
-		if (child == nullptr) {
+		if (child == nullptr)
-			error.Format(db_domain,
+			throw DatabaseError(DatabaseErrorCode::NOT_FOUND,
-				     (int)DatabaseErrorCode::NOT_FOUND,
+					    "No such object");
-				     "No such object");
-			return false;
-		}
 
 		if (i == last) {
 			odirent = std::move(*child);
 			return true;
 		}
 
-		if (child->type != UPnPDirObject::Type::CONTAINER) {
+		if (child->type != UPnPDirObject::Type::CONTAINER)
-			error.Format(db_domain,
+			throw DatabaseError(DatabaseErrorCode::NOT_FOUND,
-				     (int)DatabaseErrorCode::NOT_FOUND,
+					    "Not a container");
-				     "Not a container");
-			return false;
-		}
 
 		objid = std::move(child->id);
 	}
@@ -607,10 +592,8 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server,
 			break;
 
 		default:
-			error.Format(db_domain,
+			throw DatabaseError(DatabaseErrorCode::NOT_FOUND,
-				     (int)DatabaseErrorCode::NOT_FOUND,
+					    "Not found");
-				     "Not found");
-			return false;
 		}
 
 		if (visit_song) {
@@ -620,12 +603,9 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server,
 				return false;
 
 			if (dirent.type != UPnPDirObject::Type::ITEM ||
-			    dirent.item_class != UPnPDirObject::ItemClass::MUSIC) {
+			    dirent.item_class != UPnPDirObject::ItemClass::MUSIC)
-				error.Format(db_domain,
+				throw DatabaseError(DatabaseErrorCode::NOT_FOUND,
-					     (int)DatabaseErrorCode::NOT_FOUND,
+						    "Not found");
-					     "Not found");
-				return false;
-			}
 
 			std::string path = songPath(server.getFriendlyName(),
 						    dirent.id);
@@ -693,11 +673,7 @@ UpnpDatabase::Visit(const DatabaseSelection &selection,
 {
 	auto vpath = stringToTokens(selection.uri, "/", true);
 	if (vpath.empty()) {
-		std::vector<ContentDirectoryService> servers;
+		for (const auto &server : discovery->GetDirectories()) {
-		if (!discovery->GetDirectories(servers, error))
-			return false;
-
-		for (const auto &server : servers) {
 			if (visit_directory) {
 				const LightDirectory d(server.getFriendlyName(), 0);
 				if (!visit_directory(d, error))
@@ -718,10 +694,7 @@ UpnpDatabase::Visit(const DatabaseSelection &selection,
 	std::string servername(std::move(vpath.front()));
 	vpath.pop_front();
 
-	ContentDirectoryService server;
+	auto server = discovery->GetServer(servername.c_str());
-	if (!discovery->GetServer(servername.c_str(), server, error))
-		return false;
-
 	return VisitServer(server, vpath, selection,
 			   visit_directory, visit_song, visit_playlist, error);
 }
@@ -737,12 +710,8 @@ UpnpDatabase::VisitUniqueTags(const DatabaseSelection &selection,
 	if (!visit_tag)
 		return true;
 
-	std::vector<ContentDirectoryService> servers;
-	if (!discovery->GetDirectories(servers, error))
-		return false;
-
 	std::set<std::string> values;
-	for (auto& server : servers) {
+	for (auto& server : discovery->GetDirectories()) {
 		UPnPDirContent dirbuf;
 		if (!SearchSongs(server, rootid, selection, dirbuf, error))
 			return false;

src/lib/upnp/ClientInit.cxx

@@ -23,10 +23,12 @@
 #include "Callback.hxx"
 #include "Domain.hxx"
 #include "thread/Mutex.hxx"
-#include "util/Error.hxx"
+#include "util/RuntimeError.hxx"
 
 #include <upnp/upnptools.h>
 
+#include <assert.h>
+
 static Mutex upnp_client_init_mutex;
 static unsigned upnp_client_ref;
 static UpnpClient_Handle upnp_client_handle;
@@ -44,40 +46,32 @@ UpnpClientCallback(Upnp_EventType et, void *evp, void *cookie)
 	return callback.Invoke(et, evp);
 }
 
-static bool
+static void
-DoInit(Error &error)
+DoInit()
 {
 	auto code = UpnpRegisterClient(UpnpClientCallback, nullptr,
 				       &upnp_client_handle);
-	if (code != UPNP_E_SUCCESS) {
+	if (code != UPNP_E_SUCCESS)
-		error.Format(upnp_domain, code,
+		throw FormatRuntimeError("UpnpRegisterClient() failed: %s",
-			     "UpnpRegisterClient() failed: %s",
+					 UpnpGetErrorMessage(code));
-			     UpnpGetErrorMessage(code));
-		return false;
-	}
-
-	return true;
 }
 
-bool
+void
-UpnpClientGlobalInit(UpnpClient_Handle &handle, Error &error)
+UpnpClientGlobalInit(UpnpClient_Handle &handle)
 {
-	if (!UpnpGlobalInit(error))
+	UpnpGlobalInit();
-		return false;
 
-	bool success;
+	try {
-	{
 		const ScopeLock protect(upnp_client_init_mutex);
-		success = upnp_client_ref > 0 || DoInit(error);
+		if (upnp_client_ref == 0)
+			DoInit();
+	} catch (...) {
+		UpnpGlobalFinish();
+		throw;
 	}
 
-	if (success) {
+	++upnp_client_ref;
-		++upnp_client_ref;
+	handle = upnp_client_handle;
-		handle = upnp_client_handle;
-	} else
-		UpnpGlobalFinish();
-
-	return success;
 }
 
 void

src/lib/upnp/ClientInit.hxx

@@ -24,10 +24,8 @@
 
 #include <upnp/upnp.h>
 
-class Error;
+void
-
+UpnpClientGlobalInit(UpnpClient_Handle &handle);
-bool
-UpnpClientGlobalInit(UpnpClient_Handle &handle, Error &error);
 
 void
 UpnpClientGlobalFinish();

src/lib/upnp/ContentDirectoryService.cxx

@@ -26,7 +26,7 @@
 #include "Util.hxx"
 #include "Action.hxx"
 #include "util/UriUtil.hxx"
-#include "util/Error.hxx"
+#include "util/RuntimeError.hxx"
 
 ContentDirectoryService::ContentDirectoryService(const UPnPDevice &device,
 						 const UPnPService &service)
@@ -50,43 +50,34 @@ ContentDirectoryService::~ContentDirectoryService()
 	/* this destructor exists here just so it won't get inlined */
 }
 
-bool
+std::list<std::string>
-ContentDirectoryService::getSearchCapabilities(UpnpClient_Handle hdl,
+ContentDirectoryService::getSearchCapabilities(UpnpClient_Handle hdl) const
-					       std::list<std::string> &result,
-					       Error &error) const
 {
-	assert(result.empty());
-
 	UniqueIxmlDocument request(UpnpMakeAction("GetSearchCapabilities", m_serviceType.c_str(),
 						  0,
 						  nullptr, nullptr));
-	if (!request) {
+	if (!request)
-		error.Set(upnp_domain, "UpnpMakeAction() failed");
+		throw std::runtime_error("UpnpMakeAction() failed");
-		return false;
-	}
 
 	IXML_Document *_response;
 	auto code = UpnpSendAction(hdl, m_actionURL.c_str(),
 				   m_serviceType.c_str(),
 				   0 /*devUDN*/, request.get(), &_response);
-	if (code != UPNP_E_SUCCESS) {
+	if (code != UPNP_E_SUCCESS)
-		error.Format(upnp_domain, code,
+		throw FormatRuntimeError("UpnpSendAction() failed: %s",
-			     "UpnpSendAction() failed: %s",
+					 UpnpGetErrorMessage(code));
-			     UpnpGetErrorMessage(code));
-		return false;
-	}
 
 	UniqueIxmlDocument response(_response);
 
+	std::list<std::string> result;
+
 	const char *s = ixmlwrap::getFirstElementValue(response.get(),
 						       "SearchCaps");
 	if (s == nullptr || *s == 0)
-		return true;
+		return result;
 
-	if (!csvToStrings(s, result)) {
+	if (!csvToStrings(s, result))
-		error.Set(upnp_domain, "Bad response");
+		throw std::runtime_error("Bad response");
-		return false;
-	}
 
-	return true;
+	return result;
 }

src/lib/upnp/ContentDirectoryService.hxx

@@ -114,13 +114,13 @@ public:
 			 Error &error) const;
 
 	/** Retrieve search capabilities
+	 *
+	 * Throws std::runtime_error on error.
 	 *
 	 * @param[out] result an empty vector: no search, or a single '*' element:
 	 *     any tag can be used in a search, or a list of usable tag names.
 	 */
-	bool getSearchCapabilities(UpnpClient_Handle handle,
+	std::list<std::string> getSearchCapabilities(UpnpClient_Handle handle) const;
-				   std::list<std::string> &result,
-				   Error &error) const;
 
 	gcc_pure
 	std::string GetURI() const {

src/lib/upnp/Discovery.cxx

@@ -24,6 +24,7 @@
 #include "system/Clock.hxx"
 #include "Log.hxx"
 #include "util/ScopeExit.hxx"
+#include "util/RuntimeError.hxx"
 
 #include <upnp/upnptools.h>
 
@@ -209,8 +210,8 @@ UPnPDeviceDirectory::Invoke(Upnp_EventType et, void *evp)
 	return UPNP_E_SUCCESS;
 }
 
-bool
+void
-UPnPDeviceDirectory::ExpireDevices(Error &error)
+UPnPDeviceDirectory::ExpireDevices()
 {
 	const unsigned now = MonotonicClockS();
 	bool didsomething = false;
@@ -226,9 +227,7 @@ UPnPDeviceDirectory::ExpireDevices(Error &error)
 	}
 
 	if (didsomething)
-		return Search(error);
+		Search();
-
-	return true;
 }
 
 UPnPDeviceDirectory::UPnPDeviceDirectory(UpnpClient_Handle _handle,
@@ -245,56 +244,45 @@ UPnPDeviceDirectory::~UPnPDeviceDirectory()
 	/* this destructor exists here just so it won't get inlined */
 }
 
-bool
+void
-UPnPDeviceDirectory::Start(Error &error)
+UPnPDeviceDirectory::Start()
 {
-	if (!queue.start(1, Explore, this)) {
+	if (!queue.start(1, Explore, this))
-		error.Set(upnp_domain, "Discover work queue start failed");
+		throw std::runtime_error("Discover work queue start failed");
-		return false;
-	}
 
-	return Search(error);
+	Search();
 }
 
-bool
+void
-UPnPDeviceDirectory::Search(Error &error)
+UPnPDeviceDirectory::Search()
 {
 	const unsigned now = MonotonicClockS();
 	if (now - last_search < 10)
-		return true;
+		return;
 	last_search = now;
 
 	// We search both for device and service just in case.
 	int code = UpnpSearchAsync(handle, search_timeout,
 				   ContentDirectorySType, GetUpnpCookie());
-	if (code != UPNP_E_SUCCESS) {
+	if (code != UPNP_E_SUCCESS)
-		error.Format(upnp_domain, code,
+		throw FormatRuntimeError("UpnpSearchAsync() failed: %s",
-			     "UpnpSearchAsync() failed: %s",
+					 UpnpGetErrorMessage(code));
-			     UpnpGetErrorMessage(code));
-		return false;
-	}
 
 	code = UpnpSearchAsync(handle, search_timeout,
 			       MediaServerDType, GetUpnpCookie());
-	if (code != UPNP_E_SUCCESS) {
+	if (code != UPNP_E_SUCCESS)
-		error.Format(upnp_domain, code,
+		throw FormatRuntimeError("UpnpSearchAsync() failed: %s",
-			     "UpnpSearchAsync() failed: %s",
+					 UpnpGetErrorMessage(code));
-			     UpnpGetErrorMessage(code));
-		return false;
-	}
-
-	return true;
 }
 
-bool
+std::vector<ContentDirectoryService>
-UPnPDeviceDirectory::GetDirectories(std::vector<ContentDirectoryService> &out,
+UPnPDeviceDirectory::GetDirectories()
-				    Error &error)
 {
 	const ScopeLock protect(mutex);
 
-	if (!ExpireDevices(error))
+	ExpireDevices();
-		return false;
 
+	std::vector<ContentDirectoryService> out;
 	for (auto dit = directories.begin();
 	     dit != directories.end(); dit++) {
 		for (const auto &service : dit->device.services) {
@@ -304,18 +292,15 @@ UPnPDeviceDirectory::GetDirectories(std::vector<ContentDirectoryService> &out,
 		}
 	}
 
-	return true;
+	return out;
 }
 
-bool
+ContentDirectoryService
-UPnPDeviceDirectory::GetServer(const char *friendly_name,
+UPnPDeviceDirectory::GetServer(const char *friendly_name)
-			       ContentDirectoryService &server,
-			       Error &error)
 {
 	const ScopeLock protect(mutex);
 
-	if (!ExpireDevices(error))
+	ExpireDevices();
-		return false;
 
 	for (const auto &i : directories) {
 		const auto &device = i.device;
@@ -323,15 +308,11 @@ UPnPDeviceDirectory::GetServer(const char *friendly_name,
 		if (device.friendlyName != friendly_name)
 			continue;
 
-		for (const auto &service : device.services) {
+		for (const auto &service : device.services)
-			if (isCDService(service.serviceType.c_str())) {
+			if (isCDService(service.serviceType.c_str()))
-				server = ContentDirectoryService(device,
+				return ContentDirectoryService(device,
-								 service);
+							       service);
-				return true;
-			}
-		}
 	}
 
-	error.Set(upnp_domain, "Server not found");
+	throw std::runtime_error("Server not found");
-	return false;
 }

src/lib/upnp/Discovery.hxx

@@ -120,20 +120,18 @@ public:
 	UPnPDeviceDirectory(const UPnPDeviceDirectory &) = delete;
 	UPnPDeviceDirectory& operator=(const UPnPDeviceDirectory &) = delete;
 
-	bool Start(Error &error);
+	void Start();
 
 	/** Retrieve the directory services currently seen on the network */
-	bool GetDirectories(std::vector<ContentDirectoryService> &, Error &);
+	std::vector<ContentDirectoryService> GetDirectories();
 
 	/**
 	 * Get server by friendly name.
 	 */
-	bool GetServer(const char *friendly_name,
+	ContentDirectoryService GetServer(const char *friendly_name);
-		       ContentDirectoryService &server,
-		       Error &error);
 
 private:
-	bool Search(Error &error);
+	void Search();
 
 	/**
 	 * Look at the devices and get rid of those which have not
@@ -142,7 +140,7 @@ private:
 	 *
 	 * Caller must lock #mutex.
 	 */
-	bool ExpireDevices(Error &error);
+	void ExpireDevices();
 
 	void LockAdd(ContentDirectoryDescriptor &&d);
 	void LockRemove(const std::string &id);

src/lib/upnp/Init.cxx

@@ -21,44 +21,40 @@
 #include "Init.hxx"
 #include "Domain.hxx"
 #include "thread/Mutex.hxx"
-#include "util/Error.hxx"
+#include "util/RuntimeError.hxx"
 
 #include <upnp/upnp.h>
 #include <upnp/upnptools.h>
 #include <upnp/ixml.h>
 
+#include <assert.h>
+
 static Mutex upnp_init_mutex;
 static unsigned upnp_ref;
 
-static bool
+static void
-DoInit(Error &error)
+DoInit()
 {
 	auto code = UpnpInit(0, 0);
-	if (code != UPNP_E_SUCCESS) {
+	if (code != UPNP_E_SUCCESS)
-		error.Format(upnp_domain, code,
+		throw FormatRuntimeError("UpnpInit() failed: %s",
-			     "UpnpInit() failed: %s",
+					 UpnpGetErrorMessage(code));
-			     UpnpGetErrorMessage(code));
-		return false;
-	}
 
 	UpnpSetMaxContentLength(2000*1024);
 
 	// Servers sometimes make error (e.g.: minidlna returns bad utf-8)
 	ixmlRelaxParser(1);
-
-	return true;
 }
 
-bool
+void
-UpnpGlobalInit(Error &error)
+UpnpGlobalInit()
 {
 	const ScopeLock protect(upnp_init_mutex);
 
-	if (upnp_ref == 0 && !DoInit(error))
+	if (upnp_ref == 0)
-		return false;
+		DoInit();
 
 	++upnp_ref;
-	return true;
 }
 
 void

src/lib/upnp/Init.hxx

@@ -22,10 +22,8 @@
 
 #include "check.h"
 
-class Error;
+void
-
+UpnpGlobalInit();
-bool
-UpnpGlobalInit(Error &error);
 
 void
 UpnpGlobalFinish();

src/neighbor/plugins/UpnpNeighborPlugin.cxx

@@ -70,17 +70,19 @@ private:
 };
 
 bool
-UpnpNeighborExplorer::Open(Error &error)
+UpnpNeighborExplorer::Open(gcc_unused Error &error)
 {
 	UpnpClient_Handle handle;
-	if (!UpnpClientGlobalInit(handle, error))
+	UpnpClientGlobalInit(handle);
-		return false;
 
 	discovery = new UPnPDeviceDirectory(handle, this);
-	if (!discovery->Start(error)) {
+
+	try {
+		discovery->Start();
+	} catch (...) {
 		delete discovery;
 		UpnpClientGlobalFinish();
-		return false;
+		throw;
 	}
 
 	return true;
@@ -98,10 +100,10 @@ UpnpNeighborExplorer::GetList() const
 {
 	std::vector<ContentDirectoryService> tmp;
 
-	{
+	try {
-		Error error;
+		tmp = discovery->GetDirectories();
-		if (!discovery->GetDirectories(tmp, error))
+	} catch (const std::runtime_error &e) {
-			LogError(error);
+		LogError(e);
 	}
 
 	List result;