lib/sqlite, sticker: migrate from class Error to C++ exceptions

Makefile.am

@@ -380,7 +380,7 @@ endif
 if ENABLE_SQLITE
 libmpd_a_SOURCES += \
 	src/command/StickerCommands.cxx src/command/StickerCommands.hxx \
-	src/lib/sqlite/Domain.cxx src/lib/sqlite/Domain.hxx \
+	src/lib/sqlite/Error.cxx src/lib/sqlite/Error.hxx \
 	src/lib/sqlite/Util.hxx \
 	src/sticker/Match.hxx \
 	src/sticker/StickerDatabase.cxx src/sticker/StickerDatabase.hxx \

src/Instance.cxx

@@ -22,7 +22,6 @@
 #include "Partition.hxx"
 #include "Idle.hxx"
 #include "Stats.hxx"
-#include "util/Error.hxx"
 
 #ifdef ENABLE_DATABASE
 #include "db/DatabaseError.hxx"
@@ -33,6 +32,8 @@
 #endif
 #endif
 
+#include <stdexcept>
+
 #ifdef ENABLE_DATABASE
 
 const Database &
@@ -63,8 +64,12 @@ Instance::OnDatabaseSongRemoved(const char *uri)
 
 #ifdef ENABLE_SQLITE
 	/* if the song has a sticker, remove it */
-	if (sticker_enabled())
-		sticker_song_delete(uri, IgnoreError());
+	if (sticker_enabled()) {
+		try {
+			sticker_song_delete(uri);
+		} catch (const std::runtime_error &) {
+		}
+	}
 #endif
 
 	partition->StaleSong(uri);

src/Main.cxx

@@ -252,8 +252,7 @@ glue_sticker_init(void)
 		return;
 	}
 
-	if (!sticker_global_init(std::move(sticker_file), error))
-		FatalError(error);
+	sticker_global_init(std::move(sticker_file));
 #endif
 }
 

src/command/StickerCommands.cxx

@@ -25,13 +25,11 @@
 #include "sticker/SongSticker.hxx"
 #include "sticker/StickerPrint.hxx"
 #include "sticker/StickerDatabase.hxx"
-#include "CommandError.hxx"
 #include "client/Client.hxx"
 #include "client/Response.hxx"
 #include "Partition.hxx"
-#include "util/Error.hxx"
-#include "util/ConstBuffer.hxx"
 #include "util/StringAPI.hxx"
+#include "util/ScopeExit.hxx"
 
 struct sticker_song_find_data {
 	Response &r;
@@ -53,7 +51,6 @@ sticker_song_find_print_cb(const LightSong &song, const char *value,
 static CommandResult
 handle_sticker_song(Response &r, Partition &partition, Request args)
 {
-	Error error;
 	const Database &db = partition.GetDatabaseOrThrow();
 
 	const char *const cmd = args.front();
@@ -61,14 +58,11 @@ handle_sticker_song(Response &r, Partition &partition, Request args)
 	/* get song song_id key */
 	if (args.size == 4 && StringIsEqual(cmd, "get")) {
 		const LightSong *song = db.GetSong(args[2]);
+		assert(song != nullptr);
+		AtScopeExit(&db, song) { db.ReturnSong(song); };
 
-		const auto value = sticker_song_get_value(*song, args[3],
-							  error);
-		db.ReturnSong(song);
+		const auto value = sticker_song_get_value(*song, args[3]);
 		if (value.empty()) {
-			if (error.IsDefined())
-				return print_error(r, error);
-
 			r.Error(ACK_ERROR_NO_EXIST, "no such sticker");
 			return CommandResult::ERROR;
 		}
@@ -80,48 +74,34 @@ handle_sticker_song(Response &r, Partition &partition, Request args)
 	} else if (args.size == 3 && StringIsEqual(cmd, "list")) {
 		const LightSong *song = db.GetSong(args[2]);
 		assert(song != nullptr);
+		AtScopeExit(&db, song) { db.ReturnSong(song); };
 
-		Sticker *sticker = sticker_song_get(*song, error);
-		db.ReturnSong(song);
+		Sticker *sticker = sticker_song_get(*song);
 		if (sticker) {
 			sticker_print(r, *sticker);
 			sticker_free(sticker);
-		} else if (error.IsDefined())
-			return print_error(r, error);
+		}
 
 		return CommandResult::OK;
 	/* set song song_id id key */
 	} else if (args.size == 5 && StringIsEqual(cmd, "set")) {
 		const LightSong *song = db.GetSong(args[2]);
 		assert(song != nullptr);
+		AtScopeExit(&db, song) { db.ReturnSong(song); };
 
-		bool ret = sticker_song_set_value(*song, args[3], args[4],
-						  error);
-		db.ReturnSong(song);
-		if (!ret) {
-			if (error.IsDefined())
-				return print_error(r, error);
-
-			r.Error(ACK_ERROR_SYSTEM,
-				"failed to set sticker value");
-			return CommandResult::ERROR;
-		}
-
+		sticker_song_set_value(*song, args[3], args[4]);
 		return CommandResult::OK;
 	/* delete song song_id [key] */
 	} else if ((args.size == 3 || args.size == 4) &&
 		   StringIsEqual(cmd, "delete")) {
 		const LightSong *song = db.GetSong(args[2]);
 		assert(song != nullptr);
+		AtScopeExit(&db, song) { db.ReturnSong(song); };
 
 		bool ret = args.size == 3
-			? sticker_song_delete(*song, error)
-			: sticker_song_delete_value(*song, args[3], error);
-		db.ReturnSong(song);
+			? sticker_song_delete(*song)
+			: sticker_song_delete_value(*song, args[3]);
 		if (!ret) {
-			if (error.IsDefined())
-				return print_error(r, error);
-
 			r.Error(ACK_ERROR_SYSTEM, "no such sticker");
 			return CommandResult::ERROR;
 		}
@@ -161,17 +141,9 @@ handle_sticker_song(Response &r, Partition &partition, Request args)
 			args[3],
 		};
 
-		if (!sticker_song_find(db, base_uri, data.name,
-				       op, value,
-				       sticker_song_find_print_cb, &data,
-				       error)) {
-			if (error.IsDefined())
-				return print_error(r, error);
-
-			r.Error(ACK_ERROR_SYSTEM,
-				"failed to set search sticker database");
-			return CommandResult::ERROR;
-		}
+		sticker_song_find(db, base_uri, data.name,
+				  op, value,
+				  sticker_song_find_print_cb, &data);
 
 		return CommandResult::OK;
 	} else {

src/lib/sqlite/Domain.cxx → src/lib/sqlite/Error.cxx

@@ -17,8 +17,18 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-#include "config.h"
-#include "Domain.hxx"
-#include "util/Domain.hxx"
+#include "Error.hxx"
 
-const Domain sqlite_domain("sqlite");
+#include <sqlite3.h>
+
+static std::string
+MakeSqliteErrorMessage(sqlite3 *db, const char *msg)
+{
+	return std::string(msg) + ": " + sqlite3_errmsg(db);
+}
+
+SqliteError::SqliteError(sqlite3 *db, int _code, const char *msg)
+	:std::runtime_error(MakeSqliteErrorMessage(db, msg)), code(_code) {}
+
+SqliteError::SqliteError(sqlite3_stmt *stmt, int _code, const char *msg)
+	:SqliteError(sqlite3_db_handle(stmt), _code, msg) {}

src/lib/sqlite/Domain.hxx → src/lib/sqlite/Error.hxx

@@ -17,11 +17,24 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-#ifndef MPD_SQLITE_DOMAIN_HXX
-#define MPD_SQLITE_DOMAIN_HXX
+#ifndef MPD_SQLITE_ERROR_HXX
+#define MPD_SQLITE_ERROR_HXX
 
-class Domain;
+#include <stdexcept>
 
-extern const Domain sqlite_domain;
+struct sqlite3;
+struct sqlite3_stmt;
+
+class SqliteError final : public std::runtime_error {
+	int code;
+
+public:
+	SqliteError(sqlite3 *db, int _code, const char *msg);
+	SqliteError(sqlite3_stmt *stmt, int _code, const char *msg);
+
+	int GetCode() const {
+		return code;
+	}
+};
 
 #endif

src/lib/sqlite/Util.hxx

@@ -20,77 +20,49 @@
 #ifndef MPD_SQLITE_UTIL_HXX
 #define MPD_SQLITE_UTIL_HXX
 
-#include "Domain.hxx"
-#include "util/Error.hxx"
+#include "Error.hxx"
 
 #include <sqlite3.h>
 
 #include <assert.h>
 
+/**
+ * Throws #SqliteError on error.
+ */
 static void
-SetError(Error &error, sqlite3 *db, int code, const char *msg)
-{
-	error.Format(sqlite_domain, code, "%s: %s",
-		     msg, sqlite3_errmsg(db));
-}
-
-static void
-SetError(Error &error, sqlite3_stmt *stmt, int code, const char *msg)
-{
-	SetError(error, sqlite3_db_handle(stmt), code, msg);
-}
-
-static bool
-Bind(sqlite3_stmt *stmt, unsigned i, const char *value, Error &error)
+Bind(sqlite3_stmt *stmt, unsigned i, const char *value)
 {
 	int result = sqlite3_bind_text(stmt, i, value, -1, nullptr);
-	if (result != SQLITE_OK) {
-		SetError(error, stmt, result, "sqlite3_bind_text() failed");
-		return false;
-	}
-
-	return true;
+	if (result != SQLITE_OK)
+		throw SqliteError(stmt, result, "sqlite3_bind_text() failed");
 }
 
 template<typename... Args>
-static bool
-BindAll2(gcc_unused Error &error, gcc_unused sqlite3_stmt *stmt,
-	 gcc_unused unsigned i)
+static void
+BindAll2(gcc_unused sqlite3_stmt *stmt, gcc_unused unsigned i)
 {
 	assert(int(i - 1) == sqlite3_bind_parameter_count(stmt));
-
-	return true;
 }
 
 template<typename... Args>
-static bool
-BindAll2(Error &error, sqlite3_stmt *stmt, unsigned i,
+static void
+BindAll2(sqlite3_stmt *stmt, unsigned i,
 	 const char *value, Args&&... args)
 {
-	return Bind(stmt, i, value, error) &&
-		BindAll2(error, stmt, i + 1, std::forward<Args>(args)...);
-}
-
-template<typename... Args>
-static bool
-BindAll(Error &error, sqlite3_stmt *stmt, Args&&... args)
-{
-	assert(int(sizeof...(args)) == sqlite3_bind_parameter_count(stmt));
-
-	return BindAll2(error, stmt, 1, std::forward<Args>(args)...);
+	Bind(stmt, i, value);
+	BindAll2(stmt, i + 1, std::forward<Args>(args)...);
 }
 
 /**
- * Wrapper for BindAll() that returns the specified sqlite3_stmt* on
- * success and nullptr on error.
+ * Throws #SqliteError on error.
  */
 template<typename... Args>
-static sqlite3_stmt *
-BindAllOrNull(Error &error, sqlite3_stmt *stmt, Args&&... args)
+static void
+BindAll(sqlite3_stmt *stmt, Args&&... args)
 {
-	return BindAll(error, stmt, std::forward<Args>(args)...)
-		? stmt
-		: nullptr;
+	assert(int(sizeof...(args)) == sqlite3_bind_parameter_count(stmt));
+
+	BindAll2(stmt, 1, std::forward<Args>(args)...);
 }
 
 /**
@@ -110,16 +82,18 @@ ExecuteBusy(sqlite3_stmt *stmt)
 
 /**
  * Wrapper for ExecuteBusy() that returns true on SQLITE_ROW.
+ *
+ * Throws #SqliteError on error.
  */
 static bool
-ExecuteRow(sqlite3_stmt *stmt, Error &error)
+ExecuteRow(sqlite3_stmt *stmt)
 {
 	int result = ExecuteBusy(stmt);
 	if (result == SQLITE_ROW)
 		return true;
 
 	if (result != SQLITE_DONE)
-		SetError(error, stmt, result, "sqlite3_step() failed");
+		throw SqliteError(stmt, result, "sqlite3_step() failed");
 
 	return false;
 }
@@ -127,46 +101,46 @@ ExecuteRow(sqlite3_stmt *stmt, Error &error)
 /**
  * Wrapper for ExecuteBusy() that interprets everything other than
  * SQLITE_DONE as error.
+ *
+ * Throws #SqliteError on error.
  */
-static bool
-ExecuteCommand(sqlite3_stmt *stmt, Error &error)
+static void
+ExecuteCommand(sqlite3_stmt *stmt)
 {
 	int result = ExecuteBusy(stmt);
-	if (result != SQLITE_DONE) {
-		SetError(error, stmt, result, "sqlite3_step() failed");
-		return false;
-	}
-
-	return true;
+	if (result != SQLITE_DONE)
+		throw SqliteError(stmt, result, "sqlite3_step() failed");
 }
 
 /**
  * Wrapper for ExecuteCommand() that returns the number of rows
- * modified via sqlite3_changes().  Returns -1 on error.
+ * modified via sqlite3_changes().
+ *
+ * Throws #SqliteError on error.
  */
-static inline int
-ExecuteChanges(sqlite3_stmt *stmt, Error &error)
+static inline unsigned
+ExecuteChanges(sqlite3_stmt *stmt)
 {
-	if (!ExecuteCommand(stmt, error))
-		return -1;
+	ExecuteCommand(stmt);
 
 	return sqlite3_changes(sqlite3_db_handle(stmt));
 }
 
 /**
  * Wrapper for ExecuteChanges() that returns true if at least one row
- * was modified.  Returns false if nothing was modified or if an error
- * occurred.
+ * was modified.  Returns false if nothing was modified.
+ *
+ * Throws #SqliteError on error.
  */
 static inline bool
-ExecuteModified(sqlite3_stmt *stmt, Error &error)
+ExecuteModified(sqlite3_stmt *stmt)
 {
-	return ExecuteChanges(stmt, error) > 0;
+	return ExecuteChanges(stmt) > 0;
 }
 
 template<typename F>
-static inline bool
-ExecuteForEach(sqlite3_stmt *stmt, Error &error, F &&f)
+static inline void
+ExecuteForEach(sqlite3_stmt *stmt, F &&f)
 {
 	while (true) {
 		int result = ExecuteBusy(stmt);
@@ -176,11 +150,10 @@ ExecuteForEach(sqlite3_stmt *stmt, Error &error, F &&f)
 			break;
 
 		case SQLITE_DONE:
-			return true;
+			return;
 
 		default:
-			SetError(error, stmt, result, "sqlite3_step() failed");
-			return false;
+			throw SqliteError(stmt, result, "sqlite3_step() failed");
 		}
 	}
 }

src/sticker/SongSticker.cxx

@@ -22,8 +22,8 @@
 #include "StickerDatabase.hxx"
 #include "db/LightSong.hxx"
 #include "db/Interface.hxx"
-#include "util/Error.hxx"
 #include "util/Alloc.hxx"
+#include "util/ScopeExit.hxx"
 
 #include <stdexcept>
 
@@ -31,46 +31,44 @@
 #include <stdlib.h>
 
 std::string
-sticker_song_get_value(const LightSong &song, const char *name, Error &error)
+sticker_song_get_value(const LightSong &song, const char *name)
 {
 	const auto uri = song.GetURI();
-	return sticker_load_value("song", uri.c_str(), name, error);
+	return sticker_load_value("song", uri.c_str(), name);
 }
 
-bool
+void
 sticker_song_set_value(const LightSong &song,
-		       const char *name, const char *value,
-		       Error &error)
+		       const char *name, const char *value)
 {
 	const auto uri = song.GetURI();
-	return sticker_store_value("song", uri.c_str(), name, value, error);
+	sticker_store_value("song", uri.c_str(), name, value);
 }
 
 bool
-sticker_song_delete(const char *uri, Error &error)
+sticker_song_delete(const char *uri)
 {
-	return sticker_delete("song", uri, error);
+	return sticker_delete("song", uri);
 }
 
 bool
-sticker_song_delete(const LightSong &song, Error &error)
+sticker_song_delete(const LightSong &song)
 {
-	return sticker_song_delete(song.GetURI().c_str(), error);
+	return sticker_song_delete(song.GetURI().c_str());
 }
 
 bool
-sticker_song_delete_value(const LightSong &song, const char *name,
-			  Error &error)
+sticker_song_delete_value(const LightSong &song, const char *name)
 {
 	const auto uri = song.GetURI();
-	return sticker_delete_value("song", uri.c_str(), name, error);
+	return sticker_delete_value("song", uri.c_str(), name);
 }
 
 Sticker *
-sticker_song_get(const LightSong &song, Error &error)
+sticker_song_get(const LightSong &song)
 {
 	const auto uri = song.GetURI();
-	return sticker_load("song", uri.c_str(), error);
+	return sticker_load("song", uri.c_str());
 }
 
 struct sticker_song_find_data {
@@ -102,13 +100,12 @@ sticker_song_find_cb(const char *uri, const char *value, void *user_data)
 	}
 }
 
-bool
+void
 sticker_song_find(const Database &db, const char *base_uri, const char *name,
 		  StickerOperator op, const char *value,
 		  void (*func)(const LightSong &song, const char *value,
 			       void *user_data),
-		  void *user_data,
-		  Error &error)
+		  void *user_data)
 {
 	struct sticker_song_find_data data;
 	data.db = &db;
@@ -125,12 +122,10 @@ sticker_song_find(const Database &db, const char *base_uri, const char *name,
 		/* searching in root directory - no trailing slash */
 		allocated = nullptr;
 
-	data.base_uri_length = strlen(data.base_uri);
+	AtScopeExit(allocated) { free(allocated); };
 
-	bool success = sticker_find("song", data.base_uri, name, op, value,
-				    sticker_song_find_cb, &data,
-				    error);
-	free(allocated);
+	data.base_uri_length = strlen(data.base_uri);
 
-	return success;
+	sticker_find("song", data.base_uri, name, op, value,
+		     sticker_song_find_cb, &data);
 }

src/sticker/SongSticker.hxx

@@ -28,50 +28,56 @@
 struct LightSong;
 struct Sticker;
 class Database;
-class Error;
 
 /**
- * Returns one value from a song's sticker record.  The caller must
- * free the return value with g_free().
+ * Returns one value from a song's sticker record.
+ *
+ * Throws #SqliteError on error.
  */
 gcc_pure
 std::string
-sticker_song_get_value(const LightSong &song, const char *name, Error &error);
+sticker_song_get_value(const LightSong &song, const char *name);
 
 /**
  * Sets a sticker value in the specified song.  Overwrites existing
  * values.
+ *
+ * Throws #SqliteError on error.
  */
-bool
+void
 sticker_song_set_value(const LightSong &song,
-		       const char *name, const char *value,
-		       Error &error);
+		       const char *name, const char *value);
 
 /**
  * Deletes a sticker from the database.  All values are deleted.
+ *
+ * Throws #SqliteError on error.
  */
 bool
-sticker_song_delete(const char *uri, Error &error);
+sticker_song_delete(const char *uri);
 
 bool
-sticker_song_delete(const LightSong &song, Error &error);
+sticker_song_delete(const LightSong &song);
 
 /**
  * Deletes a sticker value.  Does nothing if the sticker did not
  * exist.
+ *
+ * Throws #SqliteError on error.
  */
 bool
-sticker_song_delete_value(const LightSong &song, const char *name,
-			  Error &error);
+sticker_song_delete_value(const LightSong &song, const char *name);
 
 /**
  * Loads the sticker for the specified song.
  *
+ * Throws #SqliteError on error.
+ *
  * @param song the song object
- * @return a sticker object, or NULL on error or if there is no sticker
+ * @return a sticker object, or nullptr if there is no sticker
  */
 Sticker *
-sticker_song_get(const LightSong &song, Error &error);
+sticker_song_get(const LightSong &song);
 
 /**
  * Finds stickers with the specified name below the specified
@@ -79,17 +85,16 @@ sticker_song_get(const LightSong &song, Error &error);
  *
  * Caller must lock the #db_mutex.
  *
+ * Throws #SqliteError on error.
+ *
  * @param base_uri the base directory to search in
  * @param name the name of the sticker
- * @return true on success (even if no sticker was found), false on
- * failure
  */
-bool
+void
 sticker_song_find(const Database &db, const char *base_uri, const char *name,
 		  StickerOperator op, const char *value,
 		  void (*func)(const LightSong &song, const char *value,
 			       void *user_data),
-		  void *user_data,
-		  Error &error);
+		  void *user_data);
 
 #endif

src/sticker/StickerDatabase.hxx

@@ -54,10 +54,10 @@ struct Sticker;
 /**
  * Opens the sticker database.
  *
- * @return true on success, false on error
+ * Throws std::runtime_error on error.
  */
-bool
-sticker_global_init(Path path, Error &error);
+void
+sticker_global_init(Path path);
 
 /**
  * Close the sticker database.
@@ -75,35 +75,39 @@ sticker_enabled();
 /**
  * Returns one value from an object's sticker record.  Returns an
  * empty string if the value doesn't exist.
+ *
+ * Throws #SqliteError on error.
  */
 std::string
-sticker_load_value(const char *type, const char *uri, const char *name,
-		   Error &error);
+sticker_load_value(const char *type, const char *uri, const char *name);
 
 /**
  * Sets a sticker value in the specified object.  Overwrites existing
  * values.
+ *
+ * Throws #SqliteError on error.
  */
-bool
+void
 sticker_store_value(const char *type, const char *uri,
-		    const char *name, const char *value,
-		    Error &error);
+		    const char *name, const char *value);
 
 /**
  * Deletes a sticker from the database.  All sticker values of the
  * specified object are deleted.
+ *
+ * Throws #SqliteError on error.
  */
 bool
-sticker_delete(const char *type, const char *uri,
-	       Error &error);
+sticker_delete(const char *type, const char *uri);
 
 /**
  * Deletes a sticker value.  Fails if no sticker with this name
  * exists.
+ *
+ * Throws #SqliteError on error.
  */
 bool
-sticker_delete_value(const char *type, const char *uri, const char *name,
-		     Error &error);
+sticker_delete_value(const char *type, const char *uri, const char *name);
 
 /**
  * Frees resources held by the sticker object.
@@ -140,13 +144,14 @@ sticker_foreach(const Sticker &sticker,
 /**
  * Loads the sticker for the specified resource.
  *
+ * Throws #SqliteError on error.
+ *
  * @param type the resource type, e.g. "song"
  * @param uri the URI of the resource, e.g. the song path
- * @return a sticker object, or nullptr on error or if there is no sticker
+ * @return a sticker object, or nullptr if there is no sticker
  */
 Sticker *
-sticker_load(const char *type, const char *uri,
-	     Error &error);
+sticker_load(const char *type, const char *uri);
 
 /**
  * Finds stickers with the specified name below the specified URI.
@@ -157,15 +162,12 @@ sticker_load(const char *type, const char *uri,
  * @param name the name of the sticker
  * @param op the comparison operator
  * @param value the operand
- * @return true on success (even if no sticker was found), false on
- * failure
  */
-bool
+void
 sticker_find(const char *type, const char *base_uri, const char *name,
 	     StickerOperator op, const char *value,
 	     void (*func)(const char *uri, const char *value,
 			  void *user_data),
-	     void *user_data,
-	     Error &error);
+	     void *user_data);
 
 #endif