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

src/db/plugins/upnp/ContentDirectoryService.cxx

@@ -32,22 +32,22 @@
 
 #include <stdio.h>
 
-static bool
+static void
-ReadResultTag(UPnPDirContent &dirbuf, IXML_Document *response, Error &error)
+ReadResultTag(UPnPDirContent &dirbuf, IXML_Document *response)
 {
 	const char *p = ixmlwrap::getFirstElementValue(response, "Result");
 	if (p == nullptr)
 		p = "";
 
-	return dirbuf.parse(p, error);
+	dirbuf.Parse(p);
 }
 
-inline bool
+inline void
 ContentDirectoryService::readDirSlice(UpnpClient_Handle hdl,
 				      const char *objectId, unsigned offset,
 				      unsigned count, UPnPDirContent &dirbuf,
-				      unsigned &didreadp, unsigned &totalp,
+				      unsigned &didreadp,
-				      Error &error) const
+				      unsigned &totalp) const
 {
 	// Create request
 	char ofbuf[100], cntbuf[100];
@@ -85,35 +85,32 @@ ContentDirectoryService::readDirSlice(UpnpClient_Handle hdl,
 	if (value != nullptr)
 		totalp = ParseUnsigned(value);
 
-	return ReadResultTag(dirbuf, response, error);
+	ReadResultTag(dirbuf, response);
 }
 
-bool
+UPnPDirContent
 ContentDirectoryService::readDir(UpnpClient_Handle handle,
-				 const char *objectId,
+				 const char *objectId) const
-				 UPnPDirContent &dirbuf,
-				 Error &error) const
 {
+	UPnPDirContent dirbuf;
 	unsigned offset = 0, total = -1, count;
 
 	do {
-		if (!readDirSlice(handle, objectId, offset, m_rdreqcnt, dirbuf,
+		readDirSlice(handle, objectId, offset, m_rdreqcnt, dirbuf,
-				  count, total, error))
+			     count, total);
-			return false;
 
 		offset += count;
 	} while (count > 0 && offset < total);
 
-	return true;
+	return dirbuf;
 }
 
-bool
+UPnPDirContent
 ContentDirectoryService::search(UpnpClient_Handle hdl,
 				const char *objectId,
-				const char *ss,
+				const char *ss) const
-				UPnPDirContent &dirbuf,
-				Error &error) const
 {
+	UPnPDirContent dirbuf;
 	unsigned offset = 0, total = -1, count;
 
 	do {
@@ -155,18 +152,15 @@ ContentDirectoryService::search(UpnpClient_Handle hdl,
 		if (value != nullptr)
 			total = ParseUnsigned(value);
 
-		if (!ReadResultTag(dirbuf, response.get(), error))
+		ReadResultTag(dirbuf, response.get());
-			return false;
 	} while (count > 0 && offset < total);
 
-	return true;
+	return dirbuf;
 }
 
-bool
+UPnPDirContent
 ContentDirectoryService::getMetadata(UpnpClient_Handle hdl,
-				     const char *objectId,
+				     const char *objectId) const
-				     UPnPDirContent &dirbuf,
-				     Error &error) const
 {
 	// Create request
 	UniqueIxmlDocument request(MakeActionHelper("Browse", m_serviceType.c_str(),
@@ -188,5 +182,7 @@ ContentDirectoryService::getMetadata(UpnpClient_Handle hdl,
 					 UpnpGetErrorMessage(code));
 
 	UniqueIxmlDocument response(_response);
-	return ReadResultTag(dirbuf, response.get(), error);
+	UPnPDirContent dirbuf;
+	ReadResultTag(dirbuf, response.get());
+	return dirbuf;
 }

src/db/plugins/upnp/Directory.cxx

@@ -236,9 +236,9 @@ protected:
 	}
 };
 
-bool
+void
-UPnPDirContent::parse(const char *input, Error &error)
+UPnPDirContent::Parse(const char *input)
 {
 	UPnPDirParser parser(*this);
-	return parser.Parse(input, strlen(input), true, error);
+	parser.Parse(input, strlen(input), true);
 }

src/db/plugins/upnp/Directory.hxx

@@ -36,6 +36,9 @@ class UPnPDirContent {
 public:
 	std::vector<UPnPDirObject> objects;
 
+	UPnPDirContent() = default;
+	UPnPDirContent(UPnPDirContent &&) = default;
+
 	~UPnPDirContent();
 
 	gcc_pure
@@ -60,7 +63,7 @@ public:
 	 * actually global, nothing really bad will happen if you mix
 	 * up...
 	 */
-	bool parse(const char *didltext, Error &error);
+	void Parse(const char *didltext);
 };
 
 #endif /* _UPNPDIRCONTENT_H_X_INCLUDED_ */

src/db/plugins/upnp/UpnpDatabasePlugin.cxx

@@ -127,32 +127,26 @@ private:
 			 VisitSong visit_song,
 			 Error &error) const;
 
-	bool SearchSongs(const ContentDirectoryService &server,
+	UPnPDirContent SearchSongs(const ContentDirectoryService &server,
-			 const char *objid,
+				   const char *objid,
-			 const DatabaseSelection &selection,
+				   const DatabaseSelection &selection) const;
-			 UPnPDirContent& dirbuf,
-			 Error &error) const;
 
-	bool Namei(const ContentDirectoryService &server,
+	UPnPDirObject Namei(const ContentDirectoryService &server,
-		   const std::list<std::string> &vpath,
+			    const std::list<std::string> &vpath) const;
-		   UPnPDirObject &dirent,
-		   Error &error) const;
 
 	/**
 	 * Take server and objid, return metadata.
 	 */
-	bool ReadNode(const ContentDirectoryService &server,
+	UPnPDirObject ReadNode(const ContentDirectoryService &server,
-		      const char *objid, UPnPDirObject& dirent,
+			       const char *objid) const;
-		      Error &error) const;
 
 	/**
 	 * Get the path for an object Id. This works much like pwd,
 	 * except easier cause our inodes have a parent id. Not used
 	 * any more actually (see comments in SearchSongs).
 	 */
-	bool BuildPath(const ContentDirectoryService &server,
+	std::string BuildPath(const ContentDirectoryService &server,
-		       const UPnPDirObject& dirent, std::string &idpath,
+			      const UPnPDirObject& dirent) const;
-		       Error &error) const;
 };
 
 Database *
@@ -214,7 +208,7 @@ UpnpDatabase::ReturnSong(const LightSong *_song) const
 // Get song info by path. We can receive either the id path, or the titles
 // one
 const LightSong *
-UpnpDatabase::GetSong(const char *uri, Error &error) const
+UpnpDatabase::GetSong(const char *uri, gcc_unused Error &error) const
 {
 	auto vpath = stringToTokens(uri, "/", true);
 	if (vpath.size() < 2)
@@ -227,12 +221,9 @@ UpnpDatabase::GetSong(const char *uri, Error &error) const
 
 	UPnPDirObject dirent;
 	if (vpath.front() != rootid) {
-		if (!Namei(server, vpath, dirent, error))
+		dirent = Namei(server, vpath);
-			return nullptr;
 	} else {
-		if (!ReadNode(server, vpath.back().c_str(), dirent,
+		dirent = ReadNode(server, vpath.back().c_str());
-			      error))
-			return nullptr;
 	}
 
 	return new UpnpSong(std::move(dirent), uri);
@@ -262,20 +253,18 @@ dquote(std::string &out, const char *in)
 
 // Run an UPnP search, according to MPD parameters. Return results as
 // UPnP items
-bool
+UPnPDirContent
 UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 			  const char *objid,
-			  const DatabaseSelection &selection,
+			  const DatabaseSelection &selection) const
-			  UPnPDirContent &dirbuf,
-			  Error &error) const
 {
 	const SongFilter *filter = selection.filter;
 	if (selection.filter == nullptr)
-		return true;
+		return UPnPDirContent();
 
 	const auto searchcaps = server.getSearchCapabilities(handle);
 	if (searchcaps.empty())
-		return true;
+		return UPnPDirContent();
 
 	std::string cond;
 	for (const auto &item : filter->GetItems()) {
@@ -338,9 +327,7 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 		}
 	}
 
-	return server.search(handle,
+	return server.search(handle, objid, cond.c_str());
-			     objid, cond.c_str(), dirbuf,
-			     error);
 }
 
 static bool
@@ -381,13 +368,10 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 			  VisitSong visit_song,
 			  Error &error) const
 {
-	UPnPDirContent dirbuf;
 	if (!visit_song)
 		return true;
-	if (!SearchSongs(server, objid, selection, dirbuf, error))
-		return false;
 
-	for (auto &dirent : dirbuf.objects) {
+	for (auto &dirent : SearchSongs(server, objid, selection).objects) {
 		if (dirent.type != UPnPDirObject::Type::ITEM ||
 		    dirent.item_class != UPnPDirObject::ItemClass::MUSIC)
 			continue;
@@ -419,34 +403,25 @@ UpnpDatabase::SearchSongs(const ContentDirectoryService &server,
 	return true;
 }
 
-bool
+UPnPDirObject
 UpnpDatabase::ReadNode(const ContentDirectoryService &server,
-		       const char *objid, UPnPDirObject &dirent,
+		       const char *objid) const
-		       Error &error) const
 {
-	UPnPDirContent dirbuf;
+	auto dirbuf = server.getMetadata(handle, objid);
-	if (!server.getMetadata(handle, objid, dirbuf, error))
-		return false;
-
 	if (dirbuf.objects.size() != 1)
 		throw std::runtime_error("Bad resource");
 
-	dirent = std::move(dirbuf.objects.front());
+	return std::move(dirbuf.objects.front());
-	return true;
 }
 
-bool
+std::string
 UpnpDatabase::BuildPath(const ContentDirectoryService &server,
-			const UPnPDirObject& idirent,
+			const UPnPDirObject& idirent) const
-			std::string &path,
-			Error &error) const
 {
 	const char *pid = idirent.id.c_str();
-	path.clear();
+	std::string path;
-	UPnPDirObject dirent;
 	while (strcmp(pid, rootid) != 0) {
-		if (!ReadNode(server, pid, dirent, error))
+		auto dirent = ReadNode(server, pid);
-			return false;
 		pid = dirent.parent_id.c_str();
 
 		if (path.empty())
@@ -456,33 +431,24 @@ UpnpDatabase::BuildPath(const ContentDirectoryService &server,
 						     path.c_str());
 	}
 
-	path = PathTraitsUTF8::Build(server.getFriendlyName(),
+	return PathTraitsUTF8::Build(server.getFriendlyName(),
 				     path.c_str());
-	return true;
 }
 
 // Take server and internal title pathname and return objid and metadata.
-bool
+UPnPDirObject
 UpnpDatabase::Namei(const ContentDirectoryService &server,
-		    const std::list<std::string> &vpath,
+		    const std::list<std::string> &vpath) const
-		    UPnPDirObject &odirent,
-		    Error &error) const
 {
-	if (vpath.empty()) {
+	if (vpath.empty())
 		// looking for root info
-		if (!ReadNode(server, rootid, odirent, error))
+		return ReadNode(server, rootid);
-			return false;
-
-		return true;
-	}
 
 	std::string objid(rootid);
 
 	// Walk the path elements, read each directory and try to find the next one
 	for (auto i = vpath.begin(), last = std::prev(vpath.end());; ++i) {
-		UPnPDirContent dirbuf;
+		auto dirbuf = server.readDir(handle, objid.c_str());
-		if (!server.readDir(handle, objid.c_str(), dirbuf, error))
-			return false;
 
 		// Look for the name in the sub-container list
 		UPnPDirObject *child = dirbuf.FindObject(i->c_str());
@@ -490,10 +456,8 @@ UpnpDatabase::Namei(const ContentDirectoryService &server,
 			throw DatabaseError(DatabaseErrorCode::NOT_FOUND,
 					    "No such object");
 
-		if (i == last) {
+		if (i == last)
-			odirent = std::move(*child);
+			return std::move(*child);
-			return true;
-		}
 
 		if (child->type != UPnPDirObject::Type::CONTAINER)
 			throw DatabaseError(DatabaseErrorCode::NOT_FOUND,
@@ -597,10 +561,7 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server,
 		}
 
 		if (visit_song) {
-			UPnPDirObject dirent;
+			auto dirent = ReadNode(server, vpath.back().c_str());
-			if (!ReadNode(server, vpath.back().c_str(), dirent,
-				      error))
-				return false;
 
 			if (dirent.type != UPnPDirObject::Type::ITEM ||
 			    dirent.item_class != UPnPDirObject::ItemClass::MUSIC)
@@ -618,9 +579,7 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server,
 	}
 
 	// Translate the target path into an object id and the associated metadata.
-	UPnPDirObject tdirent;
+	const auto tdirent = Namei(server, vpath);
-	if (!Namei(server, vpath, tdirent, error))
-		return false;
 
 	/* If recursive is set, this is a search... No use sending it
 	   if the filter is empty. In this case, we implement limited
@@ -644,12 +603,7 @@ UpnpDatabase::VisitServer(const ContentDirectoryService &server,
 	/* Target was a a container. Visit it. We could read slices
 	   and loop here, but it's not useful as mpd will only return
 	   data to the client when we're done anyway. */
-	UPnPDirContent dirbuf;
+	for (const auto &dirent : server.readDir(handle, tdirent.id.c_str()).objects) {
-	if (!server.readDir(handle, tdirent.id.c_str(), dirbuf,
-			    error))
-		return false;
-
-	for (auto &dirent : dirbuf.objects) {
 		const std::string uri = PathTraitsUTF8::Build(base_uri,
 							      dirent.name.c_str());
 		if (!VisitObject(dirent, uri.c_str(),
@@ -712,9 +666,7 @@ UpnpDatabase::VisitUniqueTags(const DatabaseSelection &selection,
 
 	std::set<std::string> values;
 	for (auto& server : discovery->GetDirectories()) {
-		UPnPDirContent dirbuf;
+		const auto dirbuf = SearchSongs(server, rootid, selection);
-		if (!SearchSongs(server, rootid, selection, dirbuf, error))
-			return false;
 
 		for (const auto &dirent : dirbuf.objects) {
 			if (dirent.type != UPnPDirObject::Type::ITEM ||

src/lib/expat/ExpatParser.cxx

@@ -36,16 +36,11 @@ ExpatParser::SetError(Error &error)
 		     XML_ErrorString(code));
 }
 
-bool
+void
-ExpatParser::Parse(const char *data, size_t length, bool is_final,
+ExpatParser::Parse(const char *data, size_t length, bool is_final)
-		   Error &error)
 {
-	bool success = XML_Parse(parser, data, length,
+	if (XML_Parse(parser, data, length, is_final) != XML_STATUS_OK)
-				 is_final) == XML_STATUS_OK;
+		throw ExpatError(parser);
-	if (!success)
-		SetError(error);
-
-	return success;
 }
 
 bool
@@ -59,14 +54,14 @@ ExpatParser::Parse(InputStream &is, Error &error)
 		if (nbytes == 0)
 			break;
 
-		if (!Parse(buffer, nbytes, false, error))
+		Parse(buffer, nbytes, false);
-			return false;
 	}
 
 	if (error.IsDefined())
 		return false;
 
-	return Parse("", 0, true, error);
+	Parse("", 0, true);
+	return true;
 }
 
 const char *

src/lib/expat/ExpatParser.hxx

@@ -25,9 +25,20 @@
 
 #include <expat.h>
 
+#include <stdexcept>
+
 class InputStream;
 class Error;
 
+class ExpatError final : public std::runtime_error {
+public:
+	ExpatError(XML_Error code)
+		:std::runtime_error(XML_ErrorString(code)) {}
+
+	ExpatError(XML_Parser parser)
+		:ExpatError(XML_GetErrorCode(parser)) {}
+};
+
 class ExpatParser final {
 	const XML_Parser parser;
 
@@ -53,8 +64,7 @@ public:
 		XML_SetCharacterDataHandler(parser, charhndl);
 	}
 
-	bool Parse(const char *data, size_t length, bool is_final,
+	void Parse(const char *data, size_t length, bool is_final);
-		   Error &error);
 
 	bool Parse(InputStream &is, Error &error);
 
@@ -83,9 +93,8 @@ public:
 		parser.SetCharacterDataHandler(CharacterData);
 	}
 
-	bool Parse(const char *data, size_t length, bool is_final,
+	void Parse(const char *data, size_t length, bool is_final) {
-		   Error &error) {
+		parser.Parse(data, length, is_final);
-		return parser.Parse(data, length, is_final, error);
 	}
 
 	bool Parse(InputStream &is, Error &error) {

src/lib/upnp/ContentDirectoryService.hxx

@@ -76,17 +76,14 @@ public:
 	/** Read a container's children list into dirbuf.
 	 *
 	 * @param objectId the UPnP object Id for the container. Root has Id "0"
-	 * @param[out] dirbuf stores the entries we read.
 	 */
-	bool readDir(UpnpClient_Handle handle,
+	UPnPDirContent readDir(UpnpClient_Handle handle,
-		     const char *objectId, UPnPDirContent &dirbuf,
+			       const char *objectId) const;
-		     Error &error) const;
 
-	bool readDirSlice(UpnpClient_Handle handle,
+	void readDirSlice(UpnpClient_Handle handle,
 			  const char *objectId, unsigned offset,
 			  unsigned count, UPnPDirContent& dirbuf,
-			  unsigned &didread, unsigned &total,
+			  unsigned &didread, unsigned &total) const;
-			  Error &error) const;
 
 	/** Search the content directory service.
 	 *
@@ -96,22 +93,17 @@ public:
 	 * @param searchstring an UPnP searchcriteria string. Check the
 	 * UPnP document: UPnP-av-ContentDirectory-v1-Service-20020625.pdf
 	 * section 2.5.5. Maybe we'll provide an easier way some day...
-	 * @param[out] dirbuf stores the entries we read.
 	 */
-	bool search(UpnpClient_Handle handle,
+	UPnPDirContent search(UpnpClient_Handle handle,
-		    const char *objectId, const char *searchstring,
+			      const char *objectId,
-		    UPnPDirContent &dirbuf,
+			      const char *searchstring) const;
-		    Error &error) const;
 
 	/** Read metadata for a given node.
 	 *
 	 * @param objectId the UPnP object Id. Root has Id "0"
-	 * @param[out] dirbuf stores the entries we read. At most one entry will be
-	 *   returned.
 	 */
-	bool getMetadata(UpnpClient_Handle handle,
+	UPnPDirContent getMetadata(UpnpClient_Handle handle,
-			 const char *objectId, UPnPDirContent &dirbuf,
+				   const char *objectId) const;
-			 Error &error) const;
 
 	/** Retrieve search capabilities
 	 *

src/lib/upnp/Device.cxx

@@ -21,7 +21,6 @@
 #include "Device.hxx"
 #include "Util.hxx"
 #include "lib/expat/ExpatParser.hxx"
-#include "util/Error.hxx"
 
 #include <stdlib.h>
 
@@ -100,15 +99,12 @@ protected:
 	}
 };
 
-bool
+void
-UPnPDevice::Parse(const std::string &url, const char *description,
+UPnPDevice::Parse(const std::string &url, const char *description)
-		  Error &error)
 {
 	{
 		UPnPDeviceParser mparser(*this);
-		if (!mparser.Parse(description, strlen(description),
+		mparser.Parse(description, strlen(description), true);
-				   true, error))
-			return false;
 	}
 
 	if (URLBase.empty()) {
@@ -129,6 +125,4 @@ UPnPDevice::Parse(const std::string &url, const char *description,
 			}
 		}
 	}
-
-	return true;
 }

src/lib/upnp/Device.hxx

@@ -23,8 +23,6 @@
 #include <vector>
 #include <string>
 
-class Error;
-
 /**
  * UPnP Description phase: interpreting the device description which we
  * downloaded from the URL obtained by the discovery phase.
@@ -81,8 +79,7 @@ public:
 	 * @param url where the description came from
 	 * @param description the xml device description
 	 */
-	bool Parse(const std::string &url, const char *description,
+	void Parse(const std::string &url, const char *description);
-		   Error &error);
 };
 
 #endif /* _UPNPDEV_HXX_INCLUDED_ */

src/lib/upnp/Discovery.cxx

@@ -135,13 +135,10 @@ UPnPDeviceDirectory::Explore()
 		ContentDirectoryDescriptor d(std::move(tsk->device_id),
 					     MonotonicClockS(), tsk->expires);
 
-		{
+		try {
-			Error error2;
+			d.Parse(tsk->url, buf);
-			bool success = d.Parse(tsk->url, buf, error2);
+		} catch (const std::exception &e) {
-			if (!success) {
+			LogError(e);
-				LogError(error2);
-				continue;
-			}
 		}
 
 		LockAdd(std::move(d));

src/lib/upnp/Discovery.hxx

@@ -87,9 +87,8 @@ class UPnPDeviceDirectory final : UpnpCallback {
 					   unsigned last, int exp)
 			:id(std::move(_id)), expires(last + exp + 20) {}
 
-		bool Parse(const std::string &url, const char *description,
+		void Parse(const std::string &url, const char *description) {
-			   Error &_error) {
+			device.Parse(url, description);
-			return device.Parse(url, description, _error);
 		}
 	};