Filter/Plugin: migrate from class Error to C++ exceptions

src/filter/FilterConfig.cxx

@@ -26,39 +26,31 @@
 #include "config/ConfigGlobal.hxx"
 #include "config/ConfigError.hxx"
 #include "config/Block.hxx"
-#include "util/Error.hxx"
+#include "util/RuntimeError.hxx"
 
 #include <algorithm>
 
 #include <string.h>
 
-static bool
+static void
-filter_chain_append_new(PreparedFilter &chain, const char *template_name, Error &error)
+filter_chain_append_new(PreparedFilter &chain, const char *template_name)
 {
 	const auto *cfg = config_find_block(ConfigBlockOption::AUDIO_FILTER,
 					    "name", template_name);
-	if (cfg == nullptr) {
+	if (cfg == nullptr)
-		error.Format(config_domain,
+		throw FormatRuntimeError("Filter template not found: %s",
-			     "filter template not found: %s",
+					 template_name);
-			     template_name);
-		return false;
-	}
 
 	// Instantiate one of those filter plugins with the template name as a hint
-	PreparedFilter *f = filter_configured_new(*cfg, error);
+	PreparedFilter *f = filter_configured_new(*cfg);
-	if (f == nullptr)
-		// The error has already been set, just stop.
-		return false;
 
 	const char *plugin_name = cfg->GetBlockValue("plugin",
 						     "unknown");
 	filter_chain_append(chain, plugin_name, f);
-
-	return true;
 }
 
-bool
+void
-filter_chain_parse(PreparedFilter &chain, const char *spec, Error &error)
+filter_chain_parse(PreparedFilter &chain, const char *spec)
 {
 	const char *const end = spec + strlen(spec);
 
@@ -66,9 +58,7 @@ filter_chain_parse(PreparedFilter &chain, const char *spec, Error &error)
 		const char *comma = std::find(spec, end, ',');
 		if (comma > spec) {
 			const std::string name(spec, comma);
-			if (!filter_chain_append_new(chain, name.c_str(),
+			filter_chain_append_new(chain, name.c_str());
-						     error))
-				return false;
 		}
 
 		if (comma == end)
@@ -76,6 +66,4 @@ filter_chain_parse(PreparedFilter &chain, const char *spec, Error &error)
 
 		spec = comma + 1;
 	}
-
-	return true;
 }

src/filter/FilterConfig.hxx

@@ -26,18 +26,18 @@
 #define MPD_FILTER_CONFIG_HXX
 
 class PreparedFilter;
-class Error;
 
 /**
  * Builds a filter chain from a configuration string on the form
  * "name1, name2, name3, ..." by looking up each name among the
  * configured filter sections.
+ *
+ * Throws std::runtime_error on error.
+ *
  * @param chain the chain to append filters on
  * @param spec the filter chain specification
- * @param error space to return an error description
- * @return true on success
  */
-bool
+void
-filter_chain_parse(PreparedFilter &chain, const char *spec, Error &error);
+filter_chain_parse(PreparedFilter &chain, const char *spec);
 
 #endif

src/filter/FilterPlugin.cxx

@@ -22,37 +22,29 @@
 #include "FilterRegistry.hxx"
 #include "config/Block.hxx"
 #include "config/ConfigError.hxx"
-#include "util/Error.hxx"
+#include "util/RuntimeError.hxx"
 
 #include <assert.h>
 
 PreparedFilter *
-filter_new(const struct filter_plugin *plugin,
+filter_new(const struct filter_plugin *plugin, const ConfigBlock &block)
-	   const ConfigBlock &block, Error &error)
 {
 	assert(plugin != nullptr);
-	assert(!error.IsDefined());
 
-	return plugin->init(block, error);
+	return plugin->init(block);
 }
 
 PreparedFilter *
-filter_configured_new(const ConfigBlock &block, Error &error)
+filter_configured_new(const ConfigBlock &block)
 {
-	assert(!error.IsDefined());
-
 	const char *plugin_name = block.GetBlockValue("plugin");
-	if (plugin_name == nullptr) {
+	if (plugin_name == nullptr)
-		error.Set(config_domain, "No filter plugin specified");
+		throw std::runtime_error("No filter plugin specified");
-		return nullptr;
-	}
 
 	const filter_plugin *plugin = filter_plugin_by_name(plugin_name);
-	if (plugin == nullptr) {
+	if (plugin == nullptr)
-		error.Format(config_domain,
+		throw FormatRuntimeError("No such filter plugin: %s",
-			     "No such filter plugin: %s", plugin_name);
+					 plugin_name);
-		return nullptr;
-	}
 
-	return filter_new(plugin, block, error);
+	return filter_new(plugin, block);
 }

src/filter/FilterPlugin.hxx

@@ -28,7 +28,6 @@
 
 struct ConfigBlock;
 class PreparedFilter;
-class Error;
 
 struct filter_plugin {
 	const char *name;
@@ -36,32 +35,30 @@ struct filter_plugin {
 	/**
          * Allocates and configures a filter.
 	 */
-	PreparedFilter *(*init)(const ConfigBlock &block, Error &error);
+	PreparedFilter *(*init)(const ConfigBlock &block);
 };
 
 /**
  * Creates a new instance of the specified filter plugin.
  *
+ * Throws std::runtime_error on error.
+ *
  * @param plugin the filter plugin
  * @param block configuration section
- * @param error location to store the error occurring, or nullptr to
- * ignore errors.
- * @return a new filter object, or nullptr on error
  */
 PreparedFilter *
 filter_new(const struct filter_plugin *plugin,
-	   const ConfigBlock &block, Error &error);
+	   const ConfigBlock &block);
 
 /**
  * Creates a new filter, loads configuration and the plugin name from
  * the specified configuration section.
  *
+ * Throws std::runtime_error on error.
+ *
  * @param block the configuration section
- * @param error location to store the error occurring, or nullptr to
- * ignore errors.
- * @return a new filter object, or nullptr on error
  */
 PreparedFilter *
-filter_configured_new(const ConfigBlock &block, Error &error);
+filter_configured_new(const ConfigBlock &block);
 
 #endif

src/filter/plugins/ChainFilterPlugin.cxx

@@ -93,8 +93,7 @@ public:
 };
 
 static PreparedFilter *
-chain_filter_init(gcc_unused const ConfigBlock &block,
+chain_filter_init(gcc_unused const ConfigBlock &block)
-		  gcc_unused Error &error)
 {
 	return new PreparedChainFilter();
 }

src/filter/plugins/ConvertFilterPlugin.cxx

@@ -64,8 +64,7 @@ public:
 };
 
 static PreparedFilter *
-convert_filter_init(gcc_unused const ConfigBlock &block,
+convert_filter_init(gcc_unused const ConfigBlock &block)
-		    gcc_unused Error &error)
 {
 	return new PreparedConvertFilter();
 }

src/filter/plugins/NormalizeFilterPlugin.cxx

@@ -53,8 +53,7 @@ public:
 };
 
 static PreparedFilter *
-normalize_filter_init(gcc_unused const ConfigBlock &block,
+normalize_filter_init(gcc_unused const ConfigBlock &block)
-		      gcc_unused Error &error)
 {
 	return new PreparedNormalizeFilter();
 }

src/filter/plugins/NullFilterPlugin.cxx

@@ -49,8 +49,7 @@ public:
 };
 
 static PreparedFilter *
-null_filter_init(gcc_unused const ConfigBlock &block,
+null_filter_init(gcc_unused const ConfigBlock &block)
-		 gcc_unused Error &error)
 {
 	return new PreparedNullFilter();
 }

src/filter/plugins/ReplayGainFilterPlugin.cxx

@@ -168,8 +168,7 @@ ReplayGainFilter::Update()
 }
 
 static PreparedFilter *
-replay_gain_filter_init(gcc_unused const ConfigBlock &block,
+replay_gain_filter_init(gcc_unused const ConfigBlock &block)
-			gcc_unused Error &error)
 {
 	return new PreparedReplayGainFilter();
 }

src/filter/plugins/RouteFilterPlugin.cxx

@@ -49,7 +49,7 @@
 #include "pcm/PcmBuffer.hxx"
 #include "pcm/Silence.hxx"
 #include "util/StringUtil.hxx"
-#include "util/Error.hxx"
+#include "util/RuntimeError.hxx"
 #include "util/ConstBuffer.hxx"
 #include "util/WritableBuffer.hxx"
 
@@ -130,16 +130,14 @@ public:
 	 * and input channel a gets copied to output channel b, etc.
 	 * @param block the configuration block to read
 	 * @param filter a route_filter whose min_channels and sources[] to set
-	 * @return true on success, false on error
 	 */
-	bool Configure(const ConfigBlock &block, Error &error);
+	PreparedRouteFilter(const ConfigBlock &block);
 
 	/* virtual methods from class PreparedFilter */
 	Filter *Open(AudioFormat &af) override;
 };
 
-bool
+PreparedRouteFilter::PreparedRouteFilter(const ConfigBlock &block)
-PreparedRouteFilter::Configure(const ConfigBlock &block, Error &error)
 {
 	/* TODO:
 	 * With a more clever way of marking "don't copy to output N",
@@ -160,18 +158,12 @@ PreparedRouteFilter::Configure(const ConfigBlock &block, Error &error)
 		char *endptr;
 		const unsigned source = strtoul(routes, &endptr, 10);
 		endptr = StripLeft(endptr);
-		if (endptr == routes || *endptr != '>') {
+		if (endptr == routes || *endptr != '>')
-			error.Set(config_domain,
+			throw std::runtime_error("Malformed 'routes' specification");
-				  "Malformed 'routes' specification");
-			return false;
-		}
 
-		if (source >= MAX_CHANNELS) {
+		if (source >= MAX_CHANNELS)
-			error.Format(config_domain,
+			throw FormatRuntimeError("Invalid source channel number: %u",
-				     "Invalid source channel number: %u",
+						 source);
-				     source);
-			return false;
-		}
 
 		if (source >= min_input_channels)
 			min_input_channels = source + 1;
@@ -180,18 +172,12 @@ PreparedRouteFilter::Configure(const ConfigBlock &block, Error &error)
 
 		unsigned dest = strtoul(routes, &endptr, 10);
 		endptr = StripLeft(endptr);
-		if (endptr == routes) {
+		if (endptr == routes)
-			error.Set(config_domain,
+			throw std::runtime_error("Malformed 'routes' specification");
-				  "Malformed 'routes' specification");
-			return false;
-		}
 
-		if (dest >= MAX_CHANNELS) {
+		if (dest >= MAX_CHANNELS)
-			error.Format(config_domain,
+			throw FormatRuntimeError("Invalid destination channel number: %u",
-				     "Invalid destination channel number: %u",
+						 dest);
-				     dest);
-			return false;
-		}
 
 		if (dest >= min_output_channels)
 			min_output_channels = dest + 1;
@@ -203,28 +189,17 @@ PreparedRouteFilter::Configure(const ConfigBlock &block, Error &error)
 		if (*routes == 0)
 			break;
 
-		if (*routes != ',') {
+		if (*routes != ',')
-			error.Set(config_domain,
+			throw std::runtime_error("Malformed 'routes' specification");
-				  "Malformed 'routes' specification");
-			return false;
-		}
 
 		++routes;
 	}
-
-	return true;
 }
 
 static PreparedFilter *
-route_filter_init(const ConfigBlock &block, Error &error)
+route_filter_init(const ConfigBlock &block)
 {
-	auto *filter = new PreparedRouteFilter();
+	return new PreparedRouteFilter(block);
-	if (!filter->Configure(block, error)) {
-		delete filter;
-		return nullptr;
-	}
-
-	return filter;
 }
 
 RouteFilter::RouteFilter(const AudioFormat &audio_format,

src/filter/plugins/VolumeFilterPlugin.cxx

@@ -61,8 +61,7 @@ public:
 };
 
 static PreparedFilter *
-volume_filter_init(gcc_unused const ConfigBlock &block,
+volume_filter_init(gcc_unused const ConfigBlock &block)
-		   gcc_unused Error &error)
 {
 	return new PreparedVolumeFilter();
 }

src/output/Init.cxx

@@ -39,6 +39,8 @@
 #include "util/Error.hxx"
 #include "Log.hxx"
 
+#include <stdexcept>
+
 #include <assert.h>
 #include <string.h>
 
@@ -105,8 +107,7 @@ audio_output_mixer_type(const ConfigBlock &block)
 static PreparedFilter *
 CreateVolumeFilter()
 {
-	return filter_new(&volume_filter_plugin, ConfigBlock(),
+	return filter_new(&volume_filter_plugin, ConfigBlock());
-			  IgnoreError());
 }
 
 static Mixer *
@@ -190,25 +191,24 @@ AudioOutput::Configure(const ConfigBlock &block, Error &error)
 
 	if (config_get_bool(ConfigOption::VOLUME_NORMALIZATION, false)) {
 		auto *normalize_filter =
-			filter_new(&normalize_filter_plugin, ConfigBlock(),
+			filter_new(&normalize_filter_plugin, ConfigBlock());
-				   IgnoreError());
 		assert(normalize_filter != nullptr);
 
 		filter_chain_append(*prepared_filter, "normalize",
 				    autoconvert_filter_new(normalize_filter));
 	}
 
-	Error filter_error;
+	try {
-	filter_chain_parse(*prepared_filter,
+		filter_chain_parse(*prepared_filter,
-			   block.GetBlockValue(AUDIO_FILTERS, ""),
+				   block.GetBlockValue(AUDIO_FILTERS, ""));
-			   filter_error);
+	} catch (const std::runtime_error &e) {
-
+		/* It's not really fatal - Part of the filter chain
-	// It's not really fatal - Part of the filter chain has been set up already
+		   has been set up already and even an empty one will
-	// and even an empty one will work (if only with unexpected behaviour)
+		   work (if only with unexpected behaviour) */
-	if (filter_error.IsDefined())
+		FormatError(e,
-		FormatError(filter_error,
 			    "Failed to initialize filter chain for '%s'",
 			    name);
+	}
 
 	/* done */
 
@@ -229,14 +229,13 @@ audio_output_setup(EventLoop &event_loop, AudioOutput &ao,
 
 	if (strcmp(replay_gain_handler, "none") != 0) {
 		ao.prepared_replay_gain_filter = filter_new(&replay_gain_filter_plugin,
-							    block, IgnoreError());
+							    block);
 		assert(ao.prepared_replay_gain_filter != nullptr);
 
 		ao.replay_gain_serial = 0;
 
 		ao.prepared_other_replay_gain_filter = filter_new(&replay_gain_filter_plugin,
-								  block,
+								  block);
-								  IgnoreError());
 		assert(ao.prepared_other_replay_gain_filter != nullptr);
 
 		ao.other_replay_gain_serial = 0;
@@ -276,8 +275,7 @@ audio_output_setup(EventLoop &event_loop, AudioOutput &ao,
 
 	/* the "convert" filter must be the last one in the chain */
 
-	auto *f = filter_new(&convert_filter_plugin, ConfigBlock(),
+	auto *f = filter_new(&convert_filter_plugin, ConfigBlock());
-			     IgnoreError());
 	assert(f != nullptr);
 
 	filter_chain_append(*ao.prepared_filter, "convert",

test/run_filter.cxx

@@ -58,14 +58,7 @@ load_filter(const char *name)
 		return nullptr;
 	}
 
-	Error error;
+	return filter_configured_new(*param);
-	auto *filter = filter_configured_new(*param, error);
-	if (filter == NULL) {
-		LogError(error, "Failed to load filter");
-		return NULL;
-	}
-
-	return filter;
 }
 
 int main(int argc, char **argv)