LocateUri: migrate from class Error to C++ exceptions

src/LocateUri.cxx

@@ -24,25 +24,19 @@
 #include "ls.hxx"
 #include "util/UriUtil.hxx"
 #include "util/StringCompare.hxx"
-#include "util/Error.hxx"
-#include "util/Domain.hxx"
 
 #ifdef ENABLE_DATABASE
 #include "storage/StorageInterface.hxx"
 #endif
 
-const Domain locate_uri_domain("locate_uri");
-
 static LocatedUri
-LocateFileUri(const char *uri, const Client *client,
+LocateFileUri(const char *uri, const Client *client
 #ifdef ENABLE_DATABASE
-	      const Storage *storage,
+	      , const Storage *storage
 #endif
-	      Error &error)
+	      )
 {
-	auto path = AllocatedPath::FromUTF8(uri, error);
+	auto path = AllocatedPath::FromUTF8Throw(uri);
-	if (path.IsNull())
-		return LocatedUri::Unknown();
 
 #ifdef ENABLE_DATABASE
 	if (storage != nullptr) {
@@ -54,23 +48,21 @@ LocateFileUri(const char *uri, const Client *client,
 	}
 #endif
 
-	if (client != nullptr && !client->AllowFile(path, error))
+	if (client != nullptr)
-		return LocatedUri::Unknown();
+		client->AllowFile(path);
 
 	return LocatedUri(LocatedUri::Type::PATH, uri, std::move(path));
 }
 
 static LocatedUri
-LocateAbsoluteUri(const char *uri,
+LocateAbsoluteUri(const char *uri
 #ifdef ENABLE_DATABASE
-		  const Storage *storage,
+		  , const Storage *storage
 #endif
-		  Error &error)
+		  )
 {
-	if (!uri_supported_scheme(uri)) {
+	if (!uri_supported_scheme(uri))
-		error.Set(locate_uri_domain, "Unsupported URI scheme");
+		throw std::runtime_error("Unsupported URI scheme");
-		return LocatedUri::Unknown();
-	}
 
 #ifdef ENABLE_DATABASE
 	if (storage != nullptr) {
@@ -84,37 +76,35 @@ LocateAbsoluteUri(const char *uri,
 }
 
 LocatedUri
-LocateUri(const char *uri, const Client *client,
+LocateUri(const char *uri, const Client *client
 #ifdef ENABLE_DATABASE
-	  const Storage *storage,
+	  , const Storage *storage
 #endif
-	  Error &error)
+	  )
 {
 	/* skip the obsolete "file://" prefix */
 	const char *path_utf8 = StringAfterPrefix(uri, "file://");
 	if (path_utf8 != nullptr) {
-		if (!PathTraitsUTF8::IsAbsolute(path_utf8)) {
+		if (!PathTraitsUTF8::IsAbsolute(path_utf8))
-			error.Set(locate_uri_domain, "Malformed file:// URI");
+			throw std::runtime_error("Malformed file:// URI");
-			return LocatedUri::Unknown();
-		}
 
-		return LocateFileUri(path_utf8, client,
+		return LocateFileUri(path_utf8, client
 #ifdef ENABLE_DATABASE
-				     storage,
+				     , storage
 #endif
-				     error);
+				     );
 	} else if (PathTraitsUTF8::IsAbsolute(uri))
-		return LocateFileUri(uri, client,
+		return LocateFileUri(uri, client
 #ifdef ENABLE_DATABASE
-				     storage,
+				     , storage
 #endif
-				     error);
+				     );
 	else if (uri_has_scheme(uri))
-		return LocateAbsoluteUri(uri,
+		return LocateAbsoluteUri(uri
 #ifdef ENABLE_DATABASE
-					 storage,
+					 , storage
 #endif
-					 error);
+					 );
 	else
 		return LocatedUri(LocatedUri::Type::RELATIVE, uri);
 }

src/LocateUri.hxx

@@ -35,8 +35,6 @@
 #endif
 #endif
 
-class Domain;
-class Error;
 class Client;
 
 #ifdef ENABLE_DATABASE
@@ -45,11 +43,6 @@ class Storage;
 
 struct LocatedUri {
 	enum class Type {
-		/**
-		 * Failed to parse the URI.
-		 */
-		UNKNOWN,
-
 		/**
 		 * An absolute URI with a supported scheme.
 		 */
@@ -76,22 +69,13 @@ struct LocatedUri {
 	LocatedUri(Type _type, const char *_uri,
 		   AllocatedPath &&_path=AllocatedPath::Null())
 		:type(_type), canonical_uri(_uri), path(std::move(_path)) {}
-
-	gcc_const
-	static LocatedUri Unknown() {
-		return LocatedUri(Type::UNKNOWN, nullptr);
-	}
-
-	bool IsUnknown() const {
-		return type == Type::UNKNOWN;
-	}
 };
 
-extern const Domain locate_uri_domain;
-
 /**
  * Classify a URI.
  *
+ * Throws #std::runtime_error on error.
+ *
  * @param client the #Client that is used to determine whether a local
  * file is allowed; nullptr disables the check and allows all local
  * files
@@ -100,10 +84,10 @@ extern const Domain locate_uri_domain;
  * that feature is disabled if this parameter is nullptr
  */
 LocatedUri
-LocateUri(const char *uri, const Client *client,
+LocateUri(const char *uri, const Client *client
 #ifdef ENABLE_DATABASE
-	  const Storage *storage,
+	  , const Storage *storage
 #endif
-	  Error &error);
+	  );
 
 #endif

src/Partition.hxx

@@ -68,9 +68,8 @@ struct Partition final : QueueListener, PlayerListener, MixerListener {
 	}
 
 	unsigned AppendURI(const SongLoader &loader,
-			   const char *uri_utf8,
+			   const char *uri_utf8) {
-			   Error &error) {
+		return playlist.AppendURI(pc, loader, uri_utf8);
-		return playlist.AppendURI(pc, loader, uri_utf8, error);
 	}
 
 	void DeletePosition(unsigned position) {

src/PlaylistFile.cxx

@@ -373,17 +373,12 @@ try {
 	throw;
 }
 
-bool
+void
 spl_append_uri(const char *utf8file,
-	       const SongLoader &loader, const char *url,
+	       const SongLoader &loader, const char *url)
-	       Error &error)
 {
-	std::unique_ptr<DetachedSong> song(loader.LoadSong(url, error));
+	std::unique_ptr<DetachedSong> song(loader.LoadSong(url));
-	if (song == nullptr)
-		return false;
-
 	spl_append_song(utf8file, *song);
-	return true;
 }
 
 static void

src/PlaylistFile.hxx

@@ -73,10 +73,12 @@ spl_remove_index(const char *utf8path, unsigned pos);
 void
 spl_append_song(const char *utf8path, const DetachedSong &song);
 
-bool
+/**
+ * Throws #std::runtime_error on error.
+ */
+void
 spl_append_uri(const char *path_utf8,
-	       const SongLoader &loader, const char *uri_utf8,
+	       const SongLoader &loader, const char *uri_utf8);
-	       Error &error);
 
 void
 spl_rename(const char *utf8from, const char *utf8to);

src/SongLoader.cxx

@@ -74,9 +74,6 @@ DetachedSong *
 SongLoader::LoadSong(const LocatedUri &located_uri) const
 {
 	switch (located_uri.type) {
-	case LocatedUri::Type::UNKNOWN:
-		gcc_unreachable();
-
 	case LocatedUri::Type::ABSOLUTE:
 		return new DetachedSong(located_uri.canonical_uri);
 
@@ -91,20 +88,17 @@ SongLoader::LoadSong(const LocatedUri &located_uri) const
 }
 
 DetachedSong *
-SongLoader::LoadSong(const char *uri_utf8, Error &error) const
+SongLoader::LoadSong(const char *uri_utf8) const
 {
 #if !CLANG_CHECK_VERSION(3,6)
 	/* disabled on clang due to -Wtautological-pointer-compare */
 	assert(uri_utf8 != nullptr);
 #endif
 
-	const auto located_uri = LocateUri(uri_utf8, client,
+	const auto located_uri = LocateUri(uri_utf8, client
 #ifdef ENABLE_DATABASE
-					   storage,
+					   , storage
 #endif
-					   error);
+					   );
-	if (located_uri.IsUnknown())
-		return nullptr;
-
 	return LoadSong(located_uri);
 }

src/SongLoader.hxx

@@ -70,8 +70,11 @@ public:
 
 	DetachedSong *LoadSong(const LocatedUri &uri) const;
 
+	/**
+	 * Throws #std::runtime_error on error.
+	 */
 	gcc_nonnull_all
-	DetachedSong *LoadSong(const char *uri_utf8, Error &error) const;
+	DetachedSong *LoadSong(const char *uri_utf8) const;
 
 private:
 	gcc_nonnull_all

src/command/CommandError.cxx

@@ -21,7 +21,6 @@
 #include "CommandError.hxx"
 #include "PlaylistError.hxx"
 #include "db/DatabaseError.hxx"
-#include "LocateUri.hxx"
 #include "client/Response.hxx"
 #include "util/Error.hxx"
 #include "Log.hxx"
@@ -89,8 +88,6 @@ ToAck(const Error &error)
 {
 	if (error.IsDomain(ack_domain)) {
 		return (enum ack)error.GetCode();
-	} else if (error.IsDomain(locate_uri_domain)) {
-		return ACK_ERROR_ARG;
 	} else if (error.IsDomain(errno_domain)) {
 		return ACK_ERROR_SYSTEM;
 	}

src/command/FileCommands.cxx

@@ -216,16 +216,12 @@ handle_read_comments(Client &client, Request args, Response &r)
 
 	const char *const uri = args.front();
 
-	Error error;
+	const auto located_uri = LocateUri(uri, &client
-	const auto located_uri = LocateUri(uri, &client,
 #ifdef ENABLE_DATABASE
-					   nullptr,
+					   , nullptr
 #endif
-					   error);
+					   );
 	switch (located_uri.type) {
-	case LocatedUri::Type::UNKNOWN:
-		return print_error(r, error);
-
 	case LocatedUri::Type::ABSOLUTE:
 		return read_stream_comments(r, located_uri.canonical_uri);
 

src/command/OtherCommands.cxx

@@ -123,17 +123,13 @@ handle_listfiles(Client &client, Request args, Response &r)
 	/* default is root directory */
 	const auto uri = args.GetOptional(0, "");
 
-	Error error;
+	const auto located_uri = LocateUri(uri, &client
-	const auto located_uri = LocateUri(uri, &client,
 #ifdef ENABLE_DATABASE
-					   nullptr,
+					   , nullptr
 #endif
-					   error);
+					   );
 
 	switch (located_uri.type) {
-	case LocatedUri::Type::UNKNOWN:
-		return print_error(r, error);
-
 	case LocatedUri::Type::ABSOLUTE:
 #ifdef ENABLE_DATABASE
 		/* use storage plugin to list remote directory */
@@ -238,17 +234,13 @@ handle_lsinfo(Client &client, Request args, Response &r)
 		   compatibility, work around this here */
 		uri = "";
 
-	Error error;
+	const auto located_uri = LocateUri(uri, &client
-	const auto located_uri = LocateUri(uri, &client,
 #ifdef ENABLE_DATABASE
-					   nullptr,
+					   , nullptr
 #endif
-					   error);
+					   );
 
 	switch (located_uri.type) {
-	case LocatedUri::Type::UNKNOWN:
-		return print_error(r, error);
-
 	case LocatedUri::Type::ABSOLUTE:
 		return handle_lsinfo_absolute(r, located_uri.canonical_uri);
 

src/command/PlaylistCommands.cxx

@@ -165,7 +165,8 @@ handle_playlistadd(Client &client, Request args, Response &r)
 	Error error;
 	if (uri_has_scheme(uri)) {
 		const SongLoader loader(client);
-		success = spl_append_uri(playlist, loader, uri, error);
+		spl_append_uri(playlist, loader, uri);
+		success = true;
 	} else {
 #ifdef ENABLE_DATABASE
 		const Database &db = client.GetDatabaseOrThrow();

src/command/QueueCommands.cxx

@@ -83,16 +83,12 @@ handle_add(Client &client, Request args, Response &r)
 		   here */
 		uri = "";
 
-	Error error;
+	const auto located_uri = LocateUri(uri, &client
-	const auto located_uri = LocateUri(uri, &client,
 #ifdef ENABLE_DATABASE
-					   nullptr,
+					   , nullptr
 #endif
-					   error);
+					   );
 	switch (located_uri.type) {
-	case LocatedUri::Type::UNKNOWN:
-		return print_error(r, error);
-
 	case LocatedUri::Type::ABSOLUTE:
 	case LocatedUri::Type::PATH:
 		AddUri(client, located_uri);
@@ -112,10 +108,7 @@ handle_addid(Client &client, Request args, Response &r)
 	const char *const uri = args.front();
 
 	const SongLoader loader(client);
-	Error error;
+	unsigned added_id = client.partition.AppendURI(loader, uri);
-	unsigned added_id = client.partition.AppendURI(loader, uri, error);
-	if (added_id == 0)
-		return print_error(r, error);
 
 	if (args.size == 2) {
 		unsigned to = args.ParseUnsigned(1);

src/playlist/PlaylistSong.cxx

@@ -24,7 +24,6 @@
 #include "tag/TagBuilder.hxx"
 #include "fs/Traits.hxx"
 #include "util/UriUtil.hxx"
-#include "util/Error.hxx"
 #include "DetachedSong.hxx"
 
 #include <stdexcept>
@@ -49,7 +48,7 @@ playlist_check_load_song(DetachedSong &song, const SongLoader &loader)
 	DetachedSong *tmp;
 
 	try {
-		tmp = loader.LoadSong(song.GetURI(), IgnoreError());
+		tmp = loader.LoadSong(song.GetURI());
 	} catch (const std::runtime_error &) {
 		return false;
 	}

src/queue/Playlist.hxx

@@ -209,12 +209,13 @@ public:
 	unsigned AppendSong(PlayerControl &pc, DetachedSong &&song);
 
 	/**
-	 * @return the new song id or 0 on error
+	 * Throws #std::runtime_error on error.
+	 *
+	 * @return the new song id
 	 */
 	unsigned AppendURI(PlayerControl &pc,
 			   const SongLoader &loader,
-			   const char *uri_utf8,
+			   const char *uri_utf8);
-			   Error &error);
 
 protected:
 	void DeleteInternal(PlayerControl &pc,

src/queue/PlaylistEdit.cxx

@@ -124,13 +124,9 @@ playlist::AppendSong(PlayerControl &pc, DetachedSong &&song)
 
 unsigned
 playlist::AppendURI(PlayerControl &pc, const SongLoader &loader,
-		    const char *uri,
+		    const char *uri)
-		    Error &error)
 {
-	std::unique_ptr<DetachedSong> song(loader.LoadSong(uri, error));
+	std::unique_ptr<DetachedSong> song(loader.LoadSong(uri));
-	if (song == nullptr)
-		return 0;
-
 	return AppendSong(pc, std::move(*song));
 }
 

test/test_translate_song.cxx

@@ -142,13 +142,13 @@ Client::GetStorage() const
 	return ::storage;
 }
 
-bool
+void
-Client::AllowFile(gcc_unused Path path_fs, gcc_unused Error &error) const
+Client::AllowFile(gcc_unused Path path_fs) const
 {
-	/* always return false, so a SongLoader with a non-nullptr
+	/* always fail, so a SongLoader with a non-nullptr
 	   Client pointer will be regarded "insecure", while one with
 	   client==nullptr will allow all files */
-	return false;
+	throw std::runtime_error("foo");
 }
 
 static std::string