PlaylistFile: throw exception on spl_map_to_fs() failure

src/PlaylistFile.cxx

@@ -45,6 +45,8 @@
 #include "util/UriUtil.hxx"
 #include "util/Error.hxx"
 
+#include <memory>
+
 #include <assert.h>
 #include <sys/stat.h>
 #include <string.h>
@@ -91,37 +93,34 @@ spl_valid_name(const char *name_utf8)
 }
 
 static const AllocatedPath &
-spl_map(Error &error)
+spl_map()
 {
 	const AllocatedPath &path_fs = map_spl_path();
 	if (path_fs.IsNull())
-		error.Set(playlist_domain, int(PlaylistResult::DISABLED),
+		throw PlaylistError(PlaylistResult::DISABLED,
-			  "Stored playlists are disabled");
+				    "Stored playlists are disabled");
+
 	return path_fs;
 }
 
-static bool
+static void
-spl_check_name(const char *name_utf8, Error &error)
+spl_check_name(const char *name_utf8)
 {
-	if (!spl_valid_name(name_utf8)) {
+	if (!spl_valid_name(name_utf8))
-		error.Set(playlist_domain, int(PlaylistResult::BAD_NAME),
+		throw PlaylistError(PlaylistResult::BAD_NAME,
 				    "Bad playlist name");
-		return false;
-	}
-
-	return true;
 }
 
 AllocatedPath
-spl_map_to_fs(const char *name_utf8, Error &error)
+spl_map_to_fs(const char *name_utf8)
 {
-	if (spl_map(error).IsNull() || !spl_check_name(name_utf8, error))
+	spl_map();
-		return AllocatedPath::Null();
+	spl_check_name(name_utf8);
 
 	auto path_fs = map_spl_utf8_to_fs(name_utf8);
 	if (path_fs.IsNull())
-		error.Set(playlist_domain, int(PlaylistResult::BAD_NAME),
+		throw PlaylistError(PlaylistResult::BAD_NAME,
-			  "Bad playlist name");
+				    "Bad playlist name");
 
 	return path_fs;
 }
@@ -179,9 +178,8 @@ ListPlaylistFiles(Error &error)
 {
 	PlaylistVector list;
 
-	const auto &parent_path_fs = spl_map(error);
+	const auto &parent_path_fs = spl_map();
-	if (parent_path_fs.IsNull())
+	assert(!parent_path_fs.IsNull());
-		return list;
 
 	DirectoryReader reader(parent_path_fs);
 	if (reader.HasFailed()) {
@@ -199,15 +197,13 @@ ListPlaylistFiles(Error &error)
 	return list;
 }
 
-static bool
+static void
-SavePlaylistFile(const PlaylistFileContents &contents, const char *utf8path,
+SavePlaylistFile(const PlaylistFileContents &contents, const char *utf8path)
-		 Error &error)
 {
 	assert(utf8path != nullptr);
 
-	const auto path_fs = spl_map_to_fs(utf8path, error);
+	const auto path_fs = spl_map_to_fs(utf8path);
-	if (path_fs.IsNull())
+	assert(!path_fs.IsNull());
-		return false;
 
 	FileOutputStream fos(path_fs);
 
@@ -219,17 +215,15 @@ SavePlaylistFile(const PlaylistFileContents &contents, const char *utf8path,
 	bos.Flush();
 
 	fos.Commit();
-	return true;
 }
 
 PlaylistFileContents
-LoadPlaylistFile(const char *utf8path, Error &error)
+LoadPlaylistFile(const char *utf8path)
 try {
 	PlaylistFileContents contents;
 
-	const auto path_fs = spl_map_to_fs(utf8path, error);
+	const auto path_fs = spl_map_to_fs(utf8path);
-	if (path_fs.IsNull())
+	assert(!path_fs.IsNull());
-		return contents;
 
 	TextFile file(path_fs);
 
@@ -284,24 +278,18 @@ try {
 	throw;
 }
 
-bool
+void
-spl_move_index(const char *utf8path, unsigned src, unsigned dest,
+spl_move_index(const char *utf8path, unsigned src, unsigned dest)
-	       Error &error)
 {
 	if (src == dest)
 		/* this doesn't check whether the playlist exists, but
 		   what the hell.. */
-		return true;
+		return;
 
-	auto contents = LoadPlaylistFile(utf8path, error);
+	auto contents = LoadPlaylistFile(utf8path);
-	if (contents.empty() && error.IsDefined())
-		return false;
 
-	if (src >= contents.size() || dest >= contents.size()) {
+	if (src >= contents.size() || dest >= contents.size())
-		error.Set(playlist_domain, int(PlaylistResult::BAD_RANGE),
+		throw PlaylistError(PlaylistResult::BAD_RANGE, "Bad range");
-			  "Bad range");
-		return false;
-	}
 
 	const auto src_i = std::next(contents.begin(), src);
 	auto value = std::move(*src_i);
@@ -310,18 +298,16 @@ spl_move_index(const char *utf8path, unsigned src, unsigned dest,
 	const auto dest_i = std::next(contents.begin(), dest);
 	contents.insert(dest_i, std::move(value));
 
-	bool result = SavePlaylistFile(contents, utf8path, error);
+	SavePlaylistFile(contents, utf8path);
 
 	idle_add(IDLE_STORED_PLAYLIST);
-	return result;
 }
 
 bool
 spl_clear(const char *utf8path, Error &error)
 {
-	const auto path_fs = spl_map_to_fs(utf8path, error);
+	const auto path_fs = spl_map_to_fs(utf8path);
-	if (path_fs.IsNull())
+	assert(!path_fs.IsNull());
-		return false;
 
 	FILE *file = FOpen(path_fs, FOpenMode::WriteText);
 	if (file == nullptr) {
@@ -338,9 +324,8 @@ spl_clear(const char *utf8path, Error &error)
 bool
 spl_delete(const char *name_utf8, Error &error)
 {
-	const auto path_fs = spl_map_to_fs(name_utf8, error);
+	const auto path_fs = spl_map_to_fs(name_utf8);
-	if (path_fs.IsNull())
+	assert(!path_fs.IsNull());
-		return false;
 
 	if (!RemoveFile(path_fs)) {
 		playlist_errno(error);
@@ -351,41 +336,31 @@ spl_delete(const char *name_utf8, Error &error)
 	return true;
 }
 
-bool
+void
-spl_remove_index(const char *utf8path, unsigned pos, Error &error)
+spl_remove_index(const char *utf8path, unsigned pos)
 {
-	auto contents = LoadPlaylistFile(utf8path, error);
+	auto contents = LoadPlaylistFile(utf8path);
-	if (contents.empty() && error.IsDefined())
-		return false;
 
-	if (pos >= contents.size()) {
+	if (pos >= contents.size())
-		error.Set(playlist_domain, int(PlaylistResult::BAD_RANGE),
+		throw PlaylistError(PlaylistResult::BAD_RANGE, "Bad range");
-			  "Bad range");
-		return false;
-	}
 
 	contents.erase(std::next(contents.begin(), pos));
 
-	bool result = SavePlaylistFile(contents, utf8path, error);
+	SavePlaylistFile(contents, utf8path);
-
 	idle_add(IDLE_STORED_PLAYLIST);
-	return result;
 }
 
-bool
+void
-spl_append_song(const char *utf8path, const DetachedSong &song, Error &error)
+spl_append_song(const char *utf8path, const DetachedSong &song)
 try {
-	const auto path_fs = spl_map_to_fs(utf8path, error);
+	const auto path_fs = spl_map_to_fs(utf8path);
-	if (path_fs.IsNull())
+	assert(!path_fs.IsNull());
-		return false;
 
 	AppendFileOutputStream fos(path_fs);
 
-	if (fos.Tell() / (MPD_PATH_MAX + 1) >= playlist_max_length) {
+	if (fos.Tell() / (MPD_PATH_MAX + 1) >= playlist_max_length)
-		error.Set(playlist_domain, int(PlaylistResult::TOO_LARGE),
+		throw PlaylistError(PlaylistResult::TOO_LARGE,
-			  "Stored playlist is too large");
+				    "Stored playlist is too large");
-		return false;
-	}
 
 	BufferedOutputStream bos(fos);
 
@@ -395,7 +370,6 @@ try {
 	fos.Commit();
 
 	idle_add(IDLE_STORED_PLAYLIST);
-	return true;
 } catch (const std::system_error &e) {
 	if (IsFileNotFound(e))
 		throw PlaylistError::NoSuchList();
@@ -407,13 +381,12 @@ spl_append_uri(const char *utf8file,
 	       const SongLoader &loader, const char *url,
 	       Error &error)
 {
-	DetachedSong *song = loader.LoadSong(url, error);
+	std::unique_ptr<DetachedSong> song(loader.LoadSong(url, error));
 	if (song == nullptr)
 		return false;
 
-	bool success = spl_append_song(utf8file, *song, error);
+	spl_append_song(utf8file, *song);
-	delete song;
+	return true;
-	return success;
 }
 
 static bool
@@ -444,13 +417,11 @@ spl_rename_internal(Path from_path_fs, Path to_path_fs,
 bool
 spl_rename(const char *utf8from, const char *utf8to, Error &error)
 {
-	const auto from_path_fs = spl_map_to_fs(utf8from, error);
+	const auto from_path_fs = spl_map_to_fs(utf8from);
-	if (from_path_fs.IsNull())
+	assert(!from_path_fs.IsNull());
-		return false;
 
-	const auto to_path_fs = spl_map_to_fs(utf8to, error);
+	const auto to_path_fs = spl_map_to_fs(utf8to);
-	if (to_path_fs.IsNull())
+	assert(!to_path_fs.IsNull());
-		return false;
 
 	return spl_rename_internal(from_path_fs, to_path_fs, error);
 }

src/PlaylistFile.hxx

@@ -47,7 +47,7 @@ bool
 spl_valid_name(const char *name_utf8);
 
 AllocatedPath
-spl_map_to_fs(const char *name_utf8, Error &error);
+spl_map_to_fs(const char *name_utf8);
 
 /**
  * Returns a list of stored_playlist_info struct pointers.  Returns
@@ -57,11 +57,10 @@ PlaylistVector
 ListPlaylistFiles(Error &error);
 
 PlaylistFileContents
-LoadPlaylistFile(const char *utf8path, Error &error);
+LoadPlaylistFile(const char *utf8path);
 
-bool
+void
-spl_move_index(const char *utf8path, unsigned src, unsigned dest,
+spl_move_index(const char *utf8path, unsigned src, unsigned dest);
-	       Error &error);
 
 bool
 spl_clear(const char *utf8path, Error &error);
@@ -69,11 +68,11 @@ spl_clear(const char *utf8path, Error &error);
 bool
 spl_delete(const char *name_utf8, Error &error);
 
-bool
+void
-spl_remove_index(const char *utf8path, unsigned pos, Error &error);
+spl_remove_index(const char *utf8path, unsigned pos);
 
-bool
+void
-spl_append_song(const char *utf8path, const DetachedSong &song, Error &error);
+spl_append_song(const char *utf8path, const DetachedSong &song);
 
 bool
 spl_append_uri(const char *path_utf8,

src/PlaylistPrint.cxx

@@ -141,19 +141,16 @@ PrintSongDetails(Response &r, Partition &partition, const char *uri_utf8)
 
 #endif
 
-bool
+void
 spl_print(Response &r, Partition &partition,
-	  const char *name_utf8, bool detail,
+	  const char *name_utf8, bool detail)
-	  Error &error)
 {
 #ifndef ENABLE_DATABASE
 	(void)partition;
 	(void)detail;
 #endif
 
-	PlaylistFileContents contents = LoadPlaylistFile(name_utf8, error);
+	PlaylistFileContents contents = LoadPlaylistFile(name_utf8);
-	if (contents.empty() && error.IsDefined())
-		return false;
 
 	for (const auto &uri_utf8 : contents) {
 #ifdef ENABLE_DATABASE
@@ -162,6 +159,4 @@ spl_print(Response &r, Partition &partition,
 #endif
 			r.Format(SONG_FILE "%s\n", uri_utf8.c_str());
 	}
-
-	return true;
 }

src/PlaylistPrint.hxx

@@ -97,11 +97,9 @@ playlist_print_changes_position(Response &r,
  * @param client the client which requested the playlist
  * @param name_utf8 the name of the stored playlist in UTF-8 encoding
  * @param detail true if all details should be printed
- * @return true on success, false if the playlist does not exist
  */
-bool
+void
 spl_print(Response &r, Partition &partition,
-	  const char *name_utf8, bool detail,
+	  const char *name_utf8, bool detail);
-	  Error &error);
 
 #endif

src/PlaylistSave.cxx

@@ -67,18 +67,15 @@ playlist_print_uri(BufferedOutputStream &os, const char *uri)
 		os.Format("%s\n", NarrowPath(path).c_str());
 }
 
-bool
+void
-spl_save_queue(const char *name_utf8, const Queue &queue, Error &error)
+spl_save_queue(const char *name_utf8, const Queue &queue)
 {
-	const auto path_fs = spl_map_to_fs(name_utf8, error);
+	const auto path_fs = spl_map_to_fs(name_utf8);
-	if (path_fs.IsNull())
+	assert(!path_fs.IsNull());
-		return false;
 
-	if (FileExists(path_fs)) {
+	if (FileExists(path_fs))
-		error.Set(playlist_domain, int(PlaylistResult::LIST_EXISTS),
+		throw PlaylistError(PlaylistResult::LIST_EXISTS,
-			  "Playlist already exists");
+				    "Playlist already exists");
-		return false;
-	}
 
 	FileOutputStream fos(path_fs);
 	BufferedOutputStream bos(fos);
@@ -90,12 +87,10 @@ spl_save_queue(const char *name_utf8, const Queue &queue, Error &error)
 	fos.Commit();
 
 	idle_add(IDLE_STORED_PLAYLIST);
-	return true;
 }
 
-bool
+void
-spl_save_playlist(const char *name_utf8, const playlist &playlist,
+spl_save_playlist(const char *name_utf8, const playlist &playlist)
-		  Error &error)
 {
-	return spl_save_queue(name_utf8, playlist.queue, error);
+	spl_save_queue(name_utf8, playlist.queue);
 }

src/PlaylistSave.hxx

@@ -24,7 +24,6 @@ struct Queue;
 struct playlist;
 class BufferedOutputStream;
 class DetachedSong;
-class Error;
 
 void
 playlist_print_song(BufferedOutputStream &os, const DetachedSong &song);
@@ -35,14 +34,13 @@ playlist_print_uri(BufferedOutputStream &os, const char *uri);
 /**
  * Saves a queue object into a stored playlist file.
  */
-bool
+void
-spl_save_queue(const char *name_utf8, const Queue &queue, Error &error);
+spl_save_queue(const char *name_utf8, const Queue &queue);
 
 /**
  * Saves a playlist object into a stored playlist file.
  */
-bool
+void
-spl_save_playlist(const char *name_utf8, const playlist &playlist,
+spl_save_playlist(const char *name_utf8, const playlist &playlist);
-		  Error &error);
 
 #endif

src/command/PlaylistCommands.cxx

@@ -59,12 +59,10 @@ print_spl_list(Response &r, const PlaylistVector &list)
 }
 
 CommandResult
-handle_save(Client &client, Request args, Response &r)
+handle_save(Client &client, Request args, gcc_unused Response &r)
 {
-	Error error;
+	spl_save_playlist(args.front(), client.playlist);
-	return spl_save_playlist(args.front(), client.playlist, error)
+	return CommandResult::OK;
-		? CommandResult::OK
-		: print_error(r, error);
 }
 
 CommandResult
@@ -94,10 +92,8 @@ handle_listplaylist(Client &client, Request args, Response &r)
 				name, false))
 		return CommandResult::OK;
 
-	Error error;
+	spl_print(r, client.partition, name, false);
-	return spl_print(r, client.partition, name, false, error)
+	return CommandResult::OK;
-		? CommandResult::OK
-		: print_error(r, error);
 }
 
 CommandResult
@@ -109,10 +105,8 @@ handle_listplaylistinfo(Client &client, Request args, Response &r)
 				name, true))
 		return CommandResult::OK;
 
-	Error error;
+	spl_print(r, client.partition, name, true);
-	return spl_print(r, client.partition, name, true, error)
+	return CommandResult::OK;
-		? CommandResult::OK
-		: print_error(r, error);
 }
 
 CommandResult
@@ -139,28 +133,26 @@ handle_rename(gcc_unused Client &client, Request args, Response &r)
 }
 
 CommandResult
-handle_playlistdelete(gcc_unused Client &client, Request args, Response &r)
+handle_playlistdelete(gcc_unused Client &client,
+		      Request args, gcc_unused Response &r)
 {
 	const char *const name = args[0];
 	unsigned from = args.ParseUnsigned(1);
 
-	Error error;
+	spl_remove_index(name, from);
-	return spl_remove_index(name, from, error)
+	return CommandResult::OK;
-		? CommandResult::OK
-		: print_error(r, error);
 }
 
 CommandResult
-handle_playlistmove(gcc_unused Client &client, Request args, Response &r)
+handle_playlistmove(gcc_unused Client &client,
+		    Request args, gcc_unused Response &r)
 {
 	const char *const name = args.front();
 	unsigned from = args.ParseUnsigned(1);
 	unsigned to = args.ParseUnsigned(2);
 
-	Error error;
+	spl_move_index(name, from, to);
-	return spl_move_index(name, from, to, error)
+	return CommandResult::OK;
-		? CommandResult::OK
-		: print_error(r, error);
 }
 
 CommandResult

src/db/DatabasePlaylist.cxx

@@ -30,11 +30,11 @@
 
 static bool
 AddSong(const Storage &storage, const char *playlist_path_utf8,
-	const LightSong &song, Error &error)
+	const LightSong &song)
 {
-	return spl_append_song(playlist_path_utf8,
+	spl_append_song(playlist_path_utf8,
-			       DatabaseDetachSong(storage, song),
+			DatabaseDetachSong(storage, song));
-			       error);
+	return true;
 }
 
 bool
@@ -47,6 +47,6 @@ search_add_to_playlist(const Database &db, const Storage &storage,
 
 	using namespace std::placeholders;
 	const auto f = std::bind(AddSong, std::ref(storage),
-				 playlist_path_utf8, _1, _2);
+				 playlist_path_utf8, _1);
 	return db.Visit(selection, f, error);
 }