From 69391dadda9b759772650b9df62a04b972c8ba5b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Albin=20Eldst=C3=A5l-Damlin?= <laeder.keps@gmail.com>
Date: Mon, 14 Dec 2009 21:46:45 +0100
Subject: [PATCH] Proper error reporting from filter_config

---
 src/filter_config.c | 52 ++++++++++++++++++++++++++++-----------------
 src/filter_config.h |  3 ++-
 src/output_init.c   | 11 +++++++++-
 3 files changed, 45 insertions(+), 21 deletions(-)

diff --git a/src/filter_config.c b/src/filter_config.c
index feca10951..6eecdbfb1 100644
--- a/src/filter_config.c
+++ b/src/filter_config.c
@@ -27,28 +27,43 @@
 
 #include <string.h>
 
+
+static GQuark
+filter_quark(void)
+{
+	return g_quark_from_static_string("filter");
+}
+
 /**
  * Find the "filter" configuration block for the specified name.
  *
  * @param filter_template_name the name of the filter template
+ * @param error_r space to return an error description
  * @return the configuration block, or NULL if none was configured
  */
 static const struct config_param *
-filter_plugin_config(const char *filter_template_name)
+filter_plugin_config(const char *filter_template_name, GError **error_r)
 {
 	const struct config_param *param = NULL;
 
 	while ((param = config_get_next_param(CONF_AUDIO_FILTER, param)) != NULL) {
 		const char *name =
 			config_get_block_string(param, "name", NULL);
-		if (name == NULL)
-			g_error("filter configuration without 'name' name in line %d",
-				param->line);
+		if (name == NULL) {
+			g_set_error(error_r, filter_quark(), 1,
+				    "filter configuration without 'name' name in line %d",
+				    param->line);
+			return NULL;
+		}
 
 		if (strcmp(name, filter_template_name) == 0)
 			return param;
 	}
 
+	g_set_error(error_r, filter_quark(), 1,
+		    "filter template not found: %s",
+		    filter_template_name);
+
 	return NULL;
 }
 
@@ -58,10 +73,11 @@ filter_plugin_config(const char *filter_template_name)
  * configured filter sections.
  * @param chain the chain to append filters on
  * @param spec the filter chain specification
+ * @param error_r space to return an error description
  * @return the number of filters which were successfully added
  */
 unsigned int
-filter_chain_parse(struct filter *chain, const char *spec)
+filter_chain_parse(struct filter *chain, const char *spec, GError **error_r)
 {
 
 	// Split on comma
@@ -80,38 +96,36 @@ filter_chain_parse(struct filter *chain, const char *spec)
 		// Squeeze whitespace
 		g_strstrip(*template_names);
 
-		cfg = filter_plugin_config(*template_names);
+		cfg = filter_plugin_config(*template_names, error_r);
 		if (cfg == NULL) {
-			g_error("Unable to locate filter template %s",
-				*template_names);
-			++template_names;
-			continue;
+			// The error has already been set, just stop.
+			break;
 		}
 
 		// Figure out what kind of a plugin that is
 		plugin_name = config_get_block_string(cfg, "plugin", NULL);
 		if (plugin_name == NULL) {
-			g_error("filter configuration without 'plugin' at line %d",
+			g_set_error(error_r, filter_quark(), 1,
+				    "filter configuration without 'plugin' at line %d",
 				cfg->line);
-			++template_names;
-			continue;
+			break;
 		}
 
 		// Instantiate one of those filter plugins with the template name as a hint
 		fp = filter_plugin_by_name(plugin_name);
 		if (fp == NULL) {
-			g_error("filter plugin not found: %s",
+			g_set_error(error_r, filter_quark(), 2,
+				    "filter plugin not found: %s",
 				plugin_name);
-			++template_names;
-			continue;
+			break;
 		}
 
 		f = filter_new(fp, cfg, NULL);
 		if (f == NULL) {
-			g_error("filter plugin initialization failed: %s",
+			g_set_error(error_r, filter_quark(), 3,
+				    "filter plugin initialization failed: %s",
 				plugin_name);
-			++template_names;
-			continue;
+			break;
 		}
 
 		filter_chain_append(chain, f);
diff --git a/src/filter_config.h b/src/filter_config.h
index eadaf24ac..8e20320ff 100644
--- a/src/filter_config.h
+++ b/src/filter_config.h
@@ -38,9 +38,10 @@
  * configured filter sections.
  * @param chain the chain to append filters on
  * @param spec the filter chain specification
+ * @param error_r space to return an error description
  * @return the number of filters which were successfully added
  */
 unsigned int
-filter_chain_parse(struct filter *chain, const char *spec);
+filter_chain_parse(struct filter *chain, const char *spec, GError **error_r);
 
 #endif
diff --git a/src/output_init.c b/src/output_init.c
index f4025fbfe..ab5257829 100644
--- a/src/output_init.c
+++ b/src/output_init.c
@@ -193,9 +193,18 @@ audio_output_init(struct audio_output *ao, const struct config_param *param,
 	ao->filter = filter_chain_new();
 	assert(ao->filter != NULL);
 	filter_chain_parse(ao->filter,
-	                   config_get_block_string(param, AUDIO_FILTERS, "")
+	                   config_get_block_string(param, AUDIO_FILTERS, ""),
+	                   &error
 	);
 
+	// It's not really fatal - Part of the filter chain has been set up already
+	// and even an empty one will work (if only with unexpected behaviour)
+	if (error != NULL) {
+		g_warning("Failed to initialize filter chain for '%s': %s",
+			  ao->name, error->message);
+		g_error_free(error);
+	}
+
 	ao->thread = NULL;
 	ao->command = AO_COMMAND_NONE;
 	ao->mutex = g_mutex_new();